[io] Fix Mac OS X file watcher
Fixes bug in commit 25df2b3f111bc7975776226183e1bb07cfa8ae08.
The code for converting FSEventStream notification into Dart format
was checking file existence using relative path instead of absolute
path. This caused it to incorrectly classify rename events which
were creations as deletions.
This was not caught by testing because this was not well covered
by the test and the test closest to checking this was even
disabled on Mac. CL updates updates the test to cover this better.
Fixes https://github.com/dart-lang/sdk/issues/61693
TEST=standalone/io/file_system_watcher_test
Change-Id: I2a7c984dfcec0be74c974f13b6ead7d86a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/453880
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
diff --git a/runtime/bin/file_system_watcher_macos.cc b/runtime/bin/file_system_watcher_macos.cc
index ee8c84c..c63f633 100644
--- a/runtime/bin/file_system_watcher_macos.cc
+++ b/runtime/bin/file_system_watcher_macos.cc
@@ -212,25 +212,25 @@
events.value.as_array.length = 0;
for (size_t i = 0; i < num_events; i++) {
char* path = reinterpret_cast<char**>(event_paths)[i];
- path += node->base_path_length();
- // If path is longer the base, skip next character ('/').
- if (path[0] != '\0') {
- path += 1;
+ char* relative_path = path;
+
+ relative_path += node->base_path_length();
+ // If path is longer than the base, skip next character ('/').
+ if (relative_path[0] != '\0') {
+ relative_path += 1;
}
- if (!node->recursive() && (strstr(path, "/") != nullptr)) {
+ if (!node->recursive() && (strstr(relative_path, "/") != nullptr)) {
continue;
}
- const bool is_path_empty = path[0] == '\0';
+ const bool is_self = relative_path[0] == '\0';
const bool path_exists =
- !is_path_empty &&
File::GetType(nullptr, path, false) != File::kDoesNotExist;
events.value.as_array.values[events.value.as_array.length++] =
CreateCObjectArray(
- /*flags=*/ConvertEventFlags(event_flags[i], is_path_empty,
- path_exists),
- /*cookie=*/static_cast<int64_t>(0), path,
+ /*flags=*/ConvertEventFlags(event_flags[i], is_self, path_exists),
+ /*cookie=*/static_cast<int64_t>(0), relative_path,
/*path_id=*/reinterpret_cast<int64_t>(node));
}
@@ -245,11 +245,11 @@
}
static int64_t ConvertEventFlags(FSEventStreamEventFlags flags,
- bool is_path_empty,
+ bool is_self,
bool path_exists) {
int64_t mask = 0;
if ((flags & kFSEventStreamEventFlagItemRenamed) != 0) {
- if (is_path_empty) {
+ if (is_self) {
// The moved path is the path being watched.
mask |= FileSystemWatcher::kDeleteSelf;
} else if (path_exists) {
@@ -271,7 +271,7 @@
mask |= FileSystemWatcher::kIsDir;
}
if ((flags & kFSEventStreamEventFlagItemRemoved) != 0) {
- if (is_path_empty) {
+ if (is_self) {
// The removed path is the path being watched.
mask |= FileSystemWatcher::kDeleteSelf;
} else {
diff --git a/tests/standalone/io/file_system_watcher_test.dart b/tests/standalone/io/file_system_watcher_test.dart
index 7b30c29..0a57870 100644
--- a/tests/standalone/io/file_system_watcher_test.dart
+++ b/tests/standalone/io/file_system_watcher_test.dart
@@ -227,47 +227,74 @@
file.writeAsStringSync('a');
}
-void testMultipleEvents() {
+enum FileState { none, created, modified, moved, deleted }
+
+void testMultipleEvents() async {
var dir = Directory.systemTemp.createTempSync('dart_file_system_watcher');
var file = new File(join(dir.path, 'file'));
var file2 = new File(join(dir.path, 'file2'));
+ final fileDeletionObserved = Completer<void>();
var watcher = dir.watch();
asyncStart();
- int state = 0;
+ final state = <String, FileState>{};
+ var seenMove = false;
var sub;
- sub = watcher.listen((event) {
- int newState = 0;
- switch (event.type) {
- case FileSystemEvent.create:
- newState = 1;
- break;
- case FileSystemEvent.modify:
- newState = 2;
- break;
+ void updateState(FileSystemEvent event, String path, FileState newState) {
+ final currentState = state[path] ?? FileState.none;
- case FileSystemEvent.move:
- newState = 3;
- break;
+ // We expect that states change monotonically (e.g. we don't get creation
+ // after deletion).
+ Expect.isTrue(
+ newState.index >= currentState.index,
+ 'unexpected state transition $currentState -> $newState ($event)',
+ );
- case FileSystemEvent.delete:
- newState = 4;
+ if (newState == FileState.moved) {
+ // On Windows and Linux rename should generate a move event.
+ updateState(event, path, FileState.deleted);
+ updateState(
+ event,
+ (event as FileSystemMoveEvent).destination!,
+ FileState.created,
+ );
+ seenMove = true;
+ return;
+ } else if (newState == FileState.deleted) {
+ // We expect to observe creation or modification before deletion.
+ Expect.isTrue(
+ currentState.index <= FileState.modified.index,
+ 'file creation not observed',
+ );
+
+ if (event.path.endsWith('file')) {
+ fileDeletionObserved.complete();
+ } else if (event.path.endsWith('file2')) {
sub.cancel();
asyncEnd();
dir.deleteSync();
- break;
+ }
}
- if (!Platform.isMacOS) {
- if (newState < state) throw "Bad state";
- }
- state = newState;
+ state[event.path] = newState;
+ }
+
+ sub = watcher.listen((event) {
+ updateState(event, event.path, switch (event.type) {
+ FileSystemEvent.create => FileState.created,
+ FileSystemEvent.modify => FileState.modified,
+ FileSystemEvent.move => FileState.moved,
+ FileSystemEvent.delete => FileState.deleted,
+ _ => throw 'Unexpected event: ${event}',
+ });
});
file.createSync();
file.writeAsStringSync('a');
file.renameSync(file2.path);
+ await fileDeletionObserved.future;
+ Expect.isTrue(Platform.isMacOS || seenMove);
file2.deleteSync();
}
@@ -520,17 +547,14 @@
}
void watcher(SendPort sendPort) async {
- runZonedGuarded(
- () {
- var watcher = Directory.systemTemp.watch(recursive: true);
- watcher.listen((data) async {});
- sendPort.send('start');
- },
- (error, stack) {
- print(error);
+ var watcher = Directory.systemTemp.watch(recursive: true);
+ watcher.listen(
+ (data) async {},
+ onError: (error, stack) {
sendPort.send('end');
},
);
+ sendPort.send('start');
}
void main() {
