Fix a race condition in file watcher.
It was possible for events to be added to the events controller after it
had been closed if the call to close() came in while a method was
waiting on an asynchronous callback.
R=cbracken@google.com
Review URL: https://codereview.chromium.org//1228703007 .
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ff6cfa..30762a0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+# 0.9.7
+
+* Fix a bug in `FileWatcher` where events could be added after watchers were
+ closed.
+
# 0.9.6
* Add a `Watcher` interface that encompasses watching both files and
diff --git a/lib/src/file_watcher/native.dart b/lib/src/file_watcher/native.dart
index f5699bb..1862e7b 100644
--- a/lib/src/file_watcher/native.dart
+++ b/lib/src/file_watcher/native.dart
@@ -60,10 +60,17 @@
}
_onDone() async {
- // If the file exists now, it was probably removed and quickly replaced;
- // this can happen for example when another file is moved on top of it.
- // Re-subscribe and report a modify event.
- if (await new File(path).exists()) {
+ var fileExists = await new File(path).exists();
+
+ // Check for this after checking whether the file exists because it's
+ // possible that [close] was called between [File.exists] being called and
+ // it completing.
+ if (_eventsController.isClosed) return;
+
+ if (fileExists) {
+ // If the file exists now, it was probably removed and quickly replaced;
+ // this can happen for example when another file is moved on top of it.
+ // Re-subscribe and report a modify event.
_eventsController.add(new WatchEvent(ChangeType.MODIFY, path));
_listen();
} else {
diff --git a/lib/src/file_watcher/polling.dart b/lib/src/file_watcher/polling.dart
index a44f80c..3480ae2 100644
--- a/lib/src/file_watcher/polling.dart
+++ b/lib/src/file_watcher/polling.dart
@@ -51,7 +51,10 @@
// We don't mark the file as removed if this is the first poll (indicated by
// [_lastModified] being null). Instead, below we forward the dart:io error
// that comes from trying to read the mtime below.
- if (_lastModified != null && !await new File(path).exists()) {
+ var pathExists = await new File(path).exists();
+ if (_eventsController.isClosed) return;
+
+ if (_lastModified != null && !pathExists) {
_eventsController.add(new WatchEvent(ChangeType.REMOVE, path));
close();
return;
@@ -59,14 +62,17 @@
var modified;
try {
- modified = await getModificationTime(path);
+ try {
+ modified = await getModificationTime(path);
+ } finally {
+ if (_eventsController.isClosed) return;
+ }
} on FileSystemException catch (error, stackTrace) {
_eventsController.addError(error, stackTrace);
close();
return;
}
- if (_eventsController.isClosed) return;
if (_lastModified == modified) return;
if (_lastModified == null) {
diff --git a/pubspec.yaml b/pubspec.yaml
index f62248d..2e033f1 100644
--- a/pubspec.yaml
+++ b/pubspec.yaml
@@ -1,5 +1,5 @@
name: watcher
-version: 0.9.6
+version: 0.9.7
author: Dart Team <misc@dartlang.org>
homepage: http://github.com/dart-lang/watcher
description: >
diff --git a/test/file_watcher/shared.dart b/test/file_watcher/shared.dart
index 2931d80..9a4965c 100644
--- a/test/file_watcher/shared.dart
+++ b/test/file_watcher/shared.dart
@@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
+import 'dart:async';
+
import 'package:scheduled_test/scheduled_test.dart';
import 'package:watcher/src/utils.dart';
@@ -55,4 +57,19 @@
renameFile("old.txt", "file.txt");
expectModifyEvent("file.txt");
});
+
+ // Regression test for a race condition.
+ test("closes the watcher immediately after deleting the file", () {
+ writeFile("old.txt");
+ var watcher = createWatcher(path: "file.txt", waitForReady: false);
+ var sub = schedule(() => watcher.events.listen(null));
+
+ deleteFile("file.txt");
+ schedule(() async {
+ // Reproducing the race condition will always be flaky, but this sleep
+ // helped it reproduce more consistently on my machine.
+ await new Future.delayed(new Duration(milliseconds: 10));
+ (await sub).cancel();
+ });
+ });
}
