Chromium Code Reviews| Index: pkg/watcher/lib/src/directory_watcher/mac_os.dart |
| diff --git a/pkg/watcher/lib/src/directory_watcher/mac_os.dart b/pkg/watcher/lib/src/directory_watcher/mac_os.dart |
| index bb82a4c3e1af4ed38f1a75e357d5f881913be94d..2c05c158554421add3d720d25157fc35bc33a1a9 100644 |
| --- a/pkg/watcher/lib/src/directory_watcher/mac_os.dart |
| +++ b/pkg/watcher/lib/src/directory_watcher/mac_os.dart |
| @@ -50,14 +50,6 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| Future get ready => _readyCompleter.future; |
| final _readyCompleter = new Completer(); |
| - /// The number of event batches that have been received from |
| - /// [Directory.watch]. |
| - /// |
| - /// This is used to determine if the [Directory.watch] stream was falsely |
| - /// closed due to issue 14849. A close caused by events in the past will only |
| - /// happen before or immediately after the first batch of events. |
| - int _batches = 0; |
| - |
| /// The set of files that are known to exist recursively within the watched |
| /// directory. |
| /// |
| @@ -73,11 +65,17 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| /// needs to be resubscribed in order to work around issue 14849. |
| StreamSubscription<FileSystemEvent> _watchSubscription; |
| - /// A set of subscriptions that this watcher subscribes to. |
| - /// |
| - /// These are gathered together so that they may all be canceled when the |
| - /// watcher is closed. This does not include [_watchSubscription]. |
| - final _subscriptions = new Set<StreamSubscription>(); |
| + /// The subscription to the [Directory.list] call for the initial listing of |
| + /// the directory to determine its initial state. |
| + StreamSubscription<FileSystemEntity> _initialListSubscription; |
| + |
| + /// The subscription to the [Directory.list] call for listing the contents of |
| + /// a subdirectory that was moved into the watched directory. |
| + StreamSubscription<FileSystemEntity> _listSubscription; |
| + |
| + /// The timer for tracking how long we wait for an initial batch of bogus |
| + /// events (see issue 14373). |
| + Timer _bogusEventTimer; |
| _MacOSDirectoryWatcher(String directory, int parentId) |
| : directory = directory, |
| @@ -85,12 +83,16 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| _id = "$parentId/${_count++}" { |
| _startWatch(); |
| - _listen(Chain.track(new Directory(directory).list(recursive: true)), |
| - (entity) { |
| - if (entity is! Directory) _files.add(entity.path); |
| - }, |
| - onError: _emitError, |
| - onDone: () { |
| + // Before we're ready to emit events, wait for [_listDir] to complete and |
| + // for enough time to elapse that if bogus events (issue 14373) would be |
| + // emitted, they will be. |
| + // |
| + // If we do receive a batch of events, [_onBatch] will ensure that these |
| + // futures don't fire and that the directory is re-listed. |
| + Future.wait([ |
| + _listDir().then((_) => print("[$_id] finished initial directory list")), |
|
Bob Nystrom
2014/01/15 21:32:31
Wrap this print in a debug check.
nweiz
2014/01/15 21:49:17
Done.
|
| + _waitForBogusEvents() |
| + ]).then((_) { |
| if (MacOSDirectoryWatcher.logDebugInfo) { |
| print("[$_id] watcher is ready, known files:"); |
| for (var file in _files.toSet()) { |
| @@ -98,20 +100,19 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| } |
| } |
| _readyCompleter.complete(); |
| - }, |
| - cancelOnError: true); |
| + }); |
| } |
| void close() { |
| if (MacOSDirectoryWatcher.logDebugInfo) { |
| print("[$_id] watcher is closed"); |
| } |
| - for (var subscription in _subscriptions) { |
| - subscription.cancel(); |
| - } |
| - _subscriptions.clear(); |
| if (_watchSubscription != null) _watchSubscription.cancel(); |
| + if (_initialListSubscription != null) _initialListSubscription.cancel(); |
| + if (_listSubscription != null) _listSubscription.cancel(); |
| _watchSubscription = null; |
| + _initialListSubscription = null; |
| + _listSubscription = null; |
| _eventsController.close(); |
| } |
| @@ -129,7 +130,28 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| } |
| } |
| - _batches++; |
| + // If we get a batch of events before we're ready to begin emitting events, |
| + // it's probable that it's a batch of pre-watcher events (see issue 14373). |
| + // Ignore those events and re-list the directory. |
| + if (!isReady) { |
| + if (MacOSDirectoryWatcher.logDebugInfo) { |
| + print("[$_id] not ready to emit events, re-listing directory"); |
| + } |
| + |
| + // Cancel the timer because this batch is the only batch of bogus events |
|
Bob Nystrom
2014/01/15 21:32:31
Reword more like "because bogus events only appear
nweiz
2014/01/15 21:49:17
Done.
|
| + // we'll receive. |
| + _bogusEventTimer.cancel(); |
| + _listDir().then((_) { |
| + if (MacOSDirectoryWatcher.logDebugInfo) { |
| + print("[$_id] watcher is ready, known files:"); |
| + for (var file in _files.toSet()) { |
| + print("[$_id] ${p.relative(file, from: directory)}"); |
| + } |
| + } |
| + _readyCompleter.complete(); |
| + }); |
| + return; |
| + } |
| _sortEvents(batch).forEach((path, events) { |
| var relativePath = p.relative(path, from: directory); |
| @@ -165,8 +187,8 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| if (_files.containsDir(path)) continue; |
| - _listen(Chain.track(new Directory(path).list(recursive: true)), |
| - (entity) { |
| + var stream = Chain.track(new Directory(path).list(recursive: true)); |
| + _listSubscription = stream.listen((entity) { |
| if (entity is Directory) return; |
| if (_files.contains(path)) return; |
| @@ -366,10 +388,10 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| _watchSubscription = null; |
| - // If the directory still exists and we haven't seen more than one batch, |
| + // If the directory still exists and we're still expecting bogus events, |
| // this is probably issue 14849 rather than a real close event. We should |
| // just restart the watcher. |
| - if (_batches < 2 && new Directory(directory).existsSync()) { |
| + if (!isReady && new Directory(directory).existsSync()) { |
| if (MacOSDirectoryWatcher.logDebugInfo) { |
| print("[$_id] fake closure (issue 14849), re-opening stream"); |
| } |
| @@ -398,6 +420,37 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| onDone: _onDone); |
| } |
| + /// Starts or restarts listing the watched directory to get an initial picture |
| + /// of its state. |
| + Future _listDir() { |
| + assert(!isReady); |
| + if (_initialListSubscription != null) _initialListSubscription.cancel(); |
| + |
| + _files.clear(); |
| + var completer = new Completer(); |
| + var stream = Chain.track(new Directory(directory).list(recursive: true)); |
| + _initialListSubscription = stream.listen((entity) { |
| + if (entity is! Directory) _files.add(entity.path); |
| + }, |
| + onError: _emitError, |
| + onDone: completer.complete, |
| + cancelOnError: true); |
| + return completer.future; |
| + } |
| + |
| + /// Wait 200ms for a batch of bogus events (issue 14373) to come in. |
| + /// |
| + /// 200ms is short in terms of human interaction, but longer than any Mac OS |
| + /// watcher tests take on the bots, so it should be safe to assume that any |
| + /// bogus events will be signaled in that time frame. |
| + Future _waitForBogusEvents() { |
| + var completer = new Completer(); |
| + _bogusEventTimer = new Timer( |
| + new Duration(milliseconds: 200), |
| + completer.complete); |
| + return completer.future; |
| + } |
| + |
| /// Emit an event with the given [type] and [path]. |
| void _emitEvent(ChangeType type, String path) { |
| if (!isReady) return; |
| @@ -415,18 +468,6 @@ class _MacOSDirectoryWatcher implements ManuallyClosedDirectoryWatcher { |
| close(); |
| } |
| - /// Like [Stream.listen], but automatically adds the subscription to |
| - /// [_subscriptions] so that it can be canceled when [close] is called. |
| - void _listen(Stream stream, void onData(event), {Function onError, |
| - void onDone(), bool cancelOnError}) { |
| - var subscription; |
| - subscription = stream.listen(onData, onError: onError, onDone: () { |
| - _subscriptions.remove(subscription); |
| - if (onDone != null) onDone(); |
| - }, cancelOnError: cancelOnError); |
| - _subscriptions.add(subscription); |
| - } |
| - |
| // TODO(nweiz): remove this when issue 15042 is fixed. |
| /// Return a human-friendly string representation of [event]. |
| String _formatEvent(FileSystemEvent event) { |