Take a broader approach to filtering out bogus Mac OS watcher events. With luck, this should unflake the test. R=rnystrom@google.com BUG=16079 Review URL: https://codereview.chromium.org//140013002 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart/pkg/watcher@31853 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/pkgs/watcher/lib/src/directory_watcher/mac_os.dart b/pkgs/watcher/lib/src/directory_watcher/mac_os.dart index bb82a4c..ba0ede4 100644 --- a/pkgs/watcher/lib/src/directory_watcher/mac_os.dart +++ b/pkgs/watcher/lib/src/directory_watcher/mac_os.dart
@@ -50,14 +50,6 @@ 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 @@ /// 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,20 @@ _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((_) { + if (MacOSDirectoryWatcher.logDebugInfo) { + print("[$_id] finished initial directory list"); + } + }), + _waitForBogusEvents() + ]).then((_) { if (MacOSDirectoryWatcher.logDebugInfo) { print("[$_id] watcher is ready, known files:"); for (var file in _files.toSet()) { @@ -98,20 +104,19 @@ } } _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 +134,28 @@ } } - _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 bogus events only occur in the first batch, so + // we can fire [ready] as soon as we're done listing the directory. + _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 +191,8 @@ 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 +392,10 @@ _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 +424,37 @@ 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 +472,6 @@ 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) {
diff --git a/pkgs/watcher/test/utils.dart b/pkgs/watcher/test/utils.dart index 885018b..b520bbd 100644 --- a/pkgs/watcher/test/utils.dart +++ b/pkgs/watcher/test/utils.dart
@@ -5,7 +5,6 @@ library watcher.test.utils; import 'dart:async'; -import 'dart:collection'; import 'dart:io'; import 'package:path/path.dart' as p;