[analysis_server] Remove pendingFilesToRemoveOverlay when cancelling timer
This prevents an issue where the overlay can be removed unexpectedly because the watch event sees the old (cancelled) timer, and believes there is a pending removal, so removes it.
Change-Id: Ib193ba4d6b4ba3d55a0d793554a19476c8f40e47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/238087
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index 0279042..2aec83b 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -569,10 +569,11 @@
'Invalid overlay change')));
}
} else if (change is RemoveContentOverlay) {
- _pendingFilesToRemoveOverlay[file]?.cancel();
+ _pendingFilesToRemoveOverlay.remove(file)?.cancel();
_pendingFilesToRemoveOverlay[file] = Timer(
pendingFilesRemoveOverlayDelay,
() {
+ _pendingFilesToRemoveOverlay.remove(file);
resourceProvider.removeOverlay(file);
_changeFileInDrivers(file);
},
@@ -583,7 +584,7 @@
throw AnalysisException('Illegal change type');
}
- _pendingFilesToRemoveOverlay[file]?.cancel();
+ _pendingFilesToRemoveOverlay.remove(file)?.cancel();
resourceProvider.setOverlay(
file,
content: newContents,
diff --git a/pkg/analysis_server/test/domain_analysis_test.dart b/pkg/analysis_server/test/domain_analysis_test.dart
index 776c72d..5c2c45e 100644
--- a/pkg/analysis_server/test/domain_analysis_test.dart
+++ b/pkg/analysis_server/test/domain_analysis_test.dart
@@ -884,6 +884,62 @@
assertNoErrorsNotification(a_path);
}
+ /// This test ensures that when an `addOverlay` cancels any pending
+ /// `removeOverlay` timer, it also removes it, so that a subsequent watch
+ /// event does not still try to process it.
+ Future<void>
+ test_fileSystem_removeOverlay_addOverlay_changeFile_changeOverlay() async {
+ // Use long delay, so that it does not happen.
+ server.pendingFilesRemoveOverlayDelay = const Duration(seconds: 300);
+
+ newFile2(testFilePath, '');
+
+ // Add an overlay without errors.
+ await handleSuccessfulRequest(
+ AnalysisUpdateContentParams({
+ testFilePathPlatform: AddContentOverlay(''),
+ }).toRequest('0'),
+ );
+
+ await setRoots(included: [workspaceRootPath], excluded: []);
+
+ // The test file (overlay) is analyzed, no errors.
+ await _waitAnalysisComplete();
+ _assertAnalyzedFiles(
+ hasErrors: [],
+ noErrors: [testFilePathPlatform],
+ notAnalyzed: [],
+ );
+
+ // Ask to remove the overlay, still active, start a timer.
+ await handleSuccessfulRequest(
+ AnalysisUpdateContentParams({
+ testFilePathPlatform: RemoveContentOverlay(),
+ }).toRequest('0'),
+ );
+
+ // Re-add an overlay. Should cancel the timer and replace the overlay.
+ await handleSuccessfulRequest(
+ AnalysisUpdateContentParams({
+ testFilePathPlatform: AddContentOverlay(''),
+ }).toRequest('0'),
+ );
+
+ // Change the file to trigger the watcher. Since the request above should
+ // have cancelled (and removed) the timer, this should not do anything
+ // (specifically, it should not remove the new overlay).
+ modifyFile(testFilePath, 'error');
+
+ // The overlay should still be present, so we should be able to change it.
+ await handleSuccessfulRequest(
+ AnalysisUpdateContentParams({
+ testFilePathPlatform: ChangeContentOverlay(
+ [SourceEdit(0, 0, '//')],
+ ),
+ }).toRequest('0'),
+ );
+ }
+
Future<void> test_setPriorityFiles() async {
var a = getFile('$workspaceRootPath/foo/lib/a.dart');
var b = getFile('$workspaceRootPath/foo/lib/b.dart');
