Skip to content

Commit 53a9f83

Browse files
authored
Windows DirectoryWatcher buffer exhaustion recovery workaround. (#2149)
1 parent 1195125 commit 53a9f83

File tree

5 files changed

+215
-90
lines changed

5 files changed

+215
-90
lines changed

pkgs/watcher/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
## 1.1.3-wip
2+
3+
- Improve handling of
4+
`FileSystemException: Directory watcher closed unexpectedly` on Windows. The
5+
watcher was already attempting to restart after this error and resume sending
6+
events. But, the restart would sometimes silently fail. Now, it is more
7+
reliable.
8+
19
## 1.1.2
210

311
- Fix a bug on Windows where a file creation event could be reported twice when creating

pkgs/watcher/lib/src/directory_watcher.dart

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ import 'directory_watcher/windows.dart';
1212

1313
/// Watches the contents of a directory and emits [WatchEvent]s when something
1414
/// in the directory has changed.
15+
///
16+
/// On Windows, the underlying SDK `Directory.watch` fails if too many events
17+
/// are received while Dart is busy, for example during a long-running
18+
/// synchronous operation. When this happens, some events are dropped.
19+
/// `DirectoryWatcher` restarts the watch and sends a `FileSystemException` with
20+
/// the message "Directory watcher closed unexpectedly" on the event stream. The
21+
/// code using the watcher needs to do additional work to account for the
22+
/// dropped events, for example by recomputing interesting files from scratch.
1523
abstract class DirectoryWatcher implements Watcher {
1624
/// The directory whose contents are being monitored.
1725
@Deprecated('Expires in 1.0.0. Use DirectoryWatcher.path instead.')
@@ -29,8 +37,10 @@ abstract class DirectoryWatcher implements Watcher {
2937
/// watchers.
3038
factory DirectoryWatcher(String directory, {Duration? pollingDelay}) {
3139
if (FileSystemEntity.isWatchSupported) {
32-
var customWatcher =
33-
createCustomDirectoryWatcher(directory, pollingDelay: pollingDelay);
40+
var customWatcher = createCustomDirectoryWatcher(
41+
directory,
42+
pollingDelay: pollingDelay,
43+
);
3444
if (customWatcher != null) return customWatcher;
3545
if (Platform.isLinux) return LinuxDirectoryWatcher(directory);
3646
if (Platform.isMacOS) return MacOSDirectoryWatcher(directory);

pkgs/watcher/lib/src/directory_watcher/windows.dart

Lines changed: 79 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -126,30 +126,34 @@ class _WindowsDirectoryWatcher
126126
// Check if [path] is already the root directory.
127127
if (FileSystemEntity.identicalSync(parent, path)) return;
128128
var parentStream = Directory(parent).watch(recursive: false);
129-
_parentWatchSubscription = parentStream.listen((event) {
130-
// Only look at events for 'directory'.
131-
if (p.basename(event.path) != p.basename(absoluteDir)) return;
132-
// Test if the directory is removed. FileSystemEntity.typeSync will
133-
// return NOT_FOUND if it's unable to decide upon the type, including
134-
// access denied issues, which may happen when the directory is deleted.
135-
// FileSystemMoveEvent and FileSystemDeleteEvent events will always mean
136-
// the directory is now gone.
137-
if (event is FileSystemMoveEvent ||
138-
event is FileSystemDeleteEvent ||
139-
(FileSystemEntity.typeSync(path) == FileSystemEntityType.notFound)) {
140-
for (var path in _files.paths) {
141-
_emitEvent(ChangeType.REMOVE, path);
129+
_parentWatchSubscription = parentStream.listen(
130+
(event) {
131+
// Only look at events for 'directory'.
132+
if (p.basename(event.path) != p.basename(absoluteDir)) return;
133+
// Test if the directory is removed. FileSystemEntity.typeSync will
134+
// return NOT_FOUND if it's unable to decide upon the type, including
135+
// access denied issues, which may happen when the directory is deleted.
136+
// FileSystemMoveEvent and FileSystemDeleteEvent events will always mean
137+
// the directory is now gone.
138+
if (event is FileSystemMoveEvent ||
139+
event is FileSystemDeleteEvent ||
140+
(FileSystemEntity.typeSync(path) ==
141+
FileSystemEntityType.notFound)) {
142+
for (var path in _files.paths) {
143+
_emitEvent(ChangeType.REMOVE, path);
144+
}
145+
_files.clear();
146+
close();
142147
}
143-
_files.clear();
144-
close();
145-
}
146-
}, onError: (error) {
147-
// Ignore errors, simply close the stream. The user listens on
148-
// [directory], and while it can fail to listen on the parent, we may
149-
// still be able to listen on the path requested.
150-
_parentWatchSubscription?.cancel();
151-
_parentWatchSubscription = null;
152-
});
148+
},
149+
onError: (error) {
150+
// Ignore errors, simply close the stream. The user listens on
151+
// [directory], and while it can fail to listen on the parent, we may
152+
// still be able to listen on the path requested.
153+
_parentWatchSubscription?.cancel();
154+
_parentWatchSubscription = null;
155+
},
156+
);
153157
}
154158

155159
void _onEvent(FileSystemEvent event) {
@@ -225,16 +229,18 @@ class _WindowsDirectoryWatcher
225229
// Events within directories that already have events are superfluous; the
226230
// directory's full contents will be examined anyway, so we ignore such
227231
// events. Emitting them could cause useless or out-of-order events.
228-
var directories = unionAll(batch.map((event) {
229-
if (!event.isDirectory) return <String>{};
230-
if (event is FileSystemMoveEvent) {
231-
var destination = event.destination;
232-
if (destination != null) {
233-
return {event.path, destination};
232+
var directories = unionAll(
233+
batch.map((event) {
234+
if (!event.isDirectory) return <String>{};
235+
if (event is FileSystemMoveEvent) {
236+
var destination = event.destination;
237+
if (destination != null) {
238+
return {event.path, destination};
239+
}
234240
}
235-
}
236-
return {event.path};
237-
}));
241+
return {event.path};
242+
}),
243+
);
238244

239245
bool isInModifiedDirectory(String path) =>
240246
directories.any((dir) => path != dir && p.isWithin(dir, path));
@@ -285,9 +291,11 @@ class _WindowsDirectoryWatcher
285291
// REMOVE; otherwise there will also be a REMOVE or CREATE event
286292
// (respectively) that will be contradictory.
287293
if (event is FileSystemModifyEvent) continue;
288-
assert(event is FileSystemCreateEvent ||
289-
event is FileSystemDeleteEvent ||
290-
event is FileSystemMoveEvent);
294+
assert(
295+
event is FileSystemCreateEvent ||
296+
event is FileSystemDeleteEvent ||
297+
event is FileSystemMoveEvent,
298+
);
291299

292300
// If we previously thought this was a MODIFY, we now consider it to be a
293301
// CREATE or REMOVE event. This is safe for the same reason as above.
@@ -297,9 +305,11 @@ class _WindowsDirectoryWatcher
297305
}
298306

299307
// A CREATE event contradicts a REMOVE event and vice versa.
300-
assert(type == FileSystemEvent.create ||
301-
type == FileSystemEvent.delete ||
302-
type == FileSystemEvent.move);
308+
assert(
309+
type == FileSystemEvent.create ||
310+
type == FileSystemEvent.delete ||
311+
type == FileSystemEvent.move,
312+
);
303313
if (type != event.type) return null;
304314
}
305315

@@ -383,21 +393,31 @@ class _WindowsDirectoryWatcher
383393
void _startWatch() {
384394
// Note: "watcher closed" exceptions do not get sent over the stream
385395
// returned by watch, and must be caught via a zone handler.
386-
runZonedGuarded(() {
387-
var innerStream = Directory(path).watch(recursive: true);
388-
_watchSubscription = innerStream.listen(_onEvent,
389-
onError: _eventsController.addError, onDone: _onDone);
390-
}, (error, stackTrace) {
391-
if (error is FileSystemException &&
392-
error.message.startsWith('Directory watcher closed unexpectedly')) {
393-
_watchSubscription?.cancel();
394-
_eventsController.addError(error, stackTrace);
395-
_startWatch();
396-
} else {
397-
// ignore: only_throw_errors
398-
throw error;
399-
}
400-
});
396+
runZonedGuarded(
397+
() {
398+
var innerStream = Directory(path).watch(recursive: true);
399+
_watchSubscription = innerStream.listen(
400+
_onEvent,
401+
onError: _eventsController.addError,
402+
onDone: _onDone,
403+
);
404+
},
405+
(error, stackTrace) async {
406+
if (error is FileSystemException &&
407+
error.message.startsWith('Directory watcher closed unexpectedly')) {
408+
// Wait to work around https://github.com/dart-lang/sdk/issues/61378.
409+
// Give the VM time to reset state after the error. See the issue for
410+
// more discussion of the workaround.
411+
await _watchSubscription?.cancel();
412+
await Future<void>.delayed(const Duration(milliseconds: 1));
413+
_eventsController.addError(error, stackTrace);
414+
_startWatch();
415+
} else {
416+
// ignore: only_throw_errors
417+
throw error;
418+
}
419+
},
420+
);
401421
}
402422

403423
/// Starts or restarts listing the watched directory to get an initial picture
@@ -413,8 +433,12 @@ class _WindowsDirectoryWatcher
413433
if (entity is! Directory) _files.add(entity.path);
414434
}
415435

416-
_initialListSubscription = stream.listen(handleEntity,
417-
onError: _emitError, onDone: completer.complete, cancelOnError: true);
436+
_initialListSubscription = stream.listen(
437+
handleEntity,
438+
onError: _emitError,
439+
onDone: completer.complete,
440+
cancelOnError: true,
441+
);
418442
return completer.future;
419443
}
420444

pkgs/watcher/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: watcher
2-
version: 1.1.2
2+
version: 1.1.3-wip
33
description: >-
44
A file system watcher. It monitors changes to contents of directories and
55
sends notifications when files have been added, removed, or modified.

pkgs/watcher/test/directory_watcher/windows_test.dart

Lines changed: 115 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,40 +25,123 @@ void main() {
2525
expect(DirectoryWatcher('.'), const TypeMatcher<WindowsDirectoryWatcher>());
2626
});
2727

28-
test('Regression test for https://github.com/dart-lang/tools/issues/2110',
29-
() async {
30-
late StreamSubscription<WatchEvent> sub;
31-
try {
32-
final temp = Directory.systemTemp.createTempSync();
28+
test(
29+
'Regression test for https://github.com/dart-lang/tools/issues/2110',
30+
() async {
31+
late StreamSubscription<WatchEvent> sub;
32+
try {
33+
final temp = Directory.systemTemp.createTempSync();
34+
final watcher = DirectoryWatcher(temp.path);
35+
final events = <WatchEvent>[];
36+
sub = watcher.events.listen(events.add);
37+
await watcher.ready;
38+
39+
// Create a file in a directory that doesn't exist. This forces the
40+
// directory to be created first before the child file.
41+
//
42+
// When directory creation is detected by the watcher, it calls
43+
// `Directory.list` on the directory to determine if there's files that
44+
// have been created or modified. It's possible that the watcher will
45+
// have already detected the file creation event before `Directory.list`
46+
// returns. Before https://github.com/dart-lang/tools/issues/2110 was
47+
// resolved, the check to ensure an event hadn't already been emitted
48+
// for the file creation was incorrect, leading to the event being
49+
// emitted again in some circumstances.
50+
final file = File(p.join(temp.path, 'foo', 'file.txt'))
51+
..createSync(recursive: true);
52+
53+
// Introduce a short delay to allow for the directory watcher to detect
54+
// the creation of foo/ and foo/file.txt.
55+
await Future<void>.delayed(const Duration(seconds: 1));
56+
57+
// There should only be a single file added event.
58+
expect(events, hasLength(1));
59+
expect(
60+
events.first.toString(),
61+
WatchEvent(ChangeType.ADD, file.path).toString(),
62+
);
63+
} finally {
64+
await sub.cancel();
65+
}
66+
},
67+
);
68+
69+
// The Windows native watcher has a buffer that gets exhausted if events are
70+
// not handled quickly enough. Then, it throws an error and stops watching.
71+
// The exhaustion is reliably triggered if enough events arrive during a sync
72+
// block. The `package:watcher` implementation tries to catch this and recover
73+
// by starting a new watcher.
74+
group('Buffer exhaustion', () {
75+
late StreamSubscription<Object> subscription;
76+
late Directory temp;
77+
late int eventsSeen;
78+
late int errorsSeen;
79+
80+
setUp(() async {
81+
temp = Directory.systemTemp.createTempSync();
3382
final watcher = DirectoryWatcher(temp.path);
34-
final events = <WatchEvent>[];
35-
sub = watcher.events.listen(events.add);
83+
84+
eventsSeen = 0;
85+
errorsSeen = 0;
86+
subscription = watcher.events.listen(
87+
(e) {
88+
++eventsSeen;
89+
},
90+
onError: (_, __) {
91+
++errorsSeen;
92+
},
93+
);
3694
await watcher.ready;
95+
});
96+
97+
tearDown(() {
98+
subscription.cancel();
99+
});
100+
101+
test('recovery', () async {
102+
// Use a long filename to fill the buffer.
103+
final file = File('${temp.path}\\file'.padRight(255, 'a'));
104+
105+
// Repeatedly trigger buffer exhaustion, to check that recovery is
106+
// reliable.
107+
for (var times = 0; times != 200; ++times) {
108+
errorsSeen = 0;
109+
eventsSeen = 0;
110+
111+
// Syncronously trigger 200 events. Because this is a sync block, the VM
112+
// won't handle the events, so this has a very high chance of triggering
113+
// a buffer exhaustion.
114+
//
115+
// If a buffer exhaustion happens, `package:watcher` turns this into an
116+
// error on the event stream, so `errorsSeen` will get incremented once.
117+
// The number of changes 200 is chosen so this is very likely to happen.
118+
// If there is _not_ an exhaustion, the 200 events will show on the
119+
// stream as a single event because they are changes of the same file.
120+
// So, `eventsSeen` will instead be incremented once.
121+
for (var i = 0; i != 200; ++i) {
122+
file.writeAsStringSync('');
123+
}
124+
125+
// Events only happen when there is an async gap, wait for such a gap.
126+
await Future<void>.delayed(const Duration(milliseconds: 10));
37127

38-
// Create a file in a directory that doesn't exist. This forces the
39-
// directory to be created first before the child file.
40-
//
41-
// When directory creation is detected by the watcher, it calls
42-
// `Directory.list` on the directory to determine if there's files that
43-
// have been created or modified. It's possible that the watcher will have
44-
// already detected the file creation event before `Directory.list`
45-
// returns. Before https://github.com/dart-lang/tools/issues/2110 was
46-
// resolved, the check to ensure an event hadn't already been emitted for
47-
// the file creation was incorrect, leading to the event being emitted
48-
// again in some circumstances.
49-
final file = File(p.join(temp.path, 'foo', 'file.txt'))
50-
..createSync(recursive: true);
51-
52-
// Introduce a short delay to allow for the directory watcher to detect
53-
// the creation of foo/ and foo/file.txt.
54-
await Future<void>.delayed(const Duration(seconds: 1));
55-
56-
// There should only be a single file added event.
57-
expect(events, hasLength(1));
58-
expect(events.first.toString(),
59-
WatchEvent(ChangeType.ADD, file.path).toString());
60-
} finally {
61-
await sub.cancel();
62-
}
128+
// If everything is going well, there should have been either one event
129+
// seen or one error seen.
130+
if (errorsSeen == 0 && eventsSeen == 0) {
131+
// It looks like the watcher is now broken: there were file changes
132+
// but no event and no error. Do some non-sync writes to confirm
133+
// whether the watcher really is now broken.
134+
for (var i = 0; i != 5; ++i) {
135+
await file.writeAsString('');
136+
}
137+
await Future<void>.delayed(const Duration(milliseconds: 10));
138+
fail(
139+
'On attempt ${times + 1}, watcher registered nothing. '
140+
'On retry, it registered: $errorsSeen error(s), $eventsSeen '
141+
'event(s).',
142+
);
143+
}
144+
}
145+
});
63146
});
64147
}

0 commit comments

Comments
 (0)