authorgravatar for meyerja.2013@gmail.comJarrod Meyer <meyerja.2013@gmail.com> 2024-08-01 22:28:06-04:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-08-14 13:21:01-07:00
log9be10ea964f010099124f7aed8e71b0eb1c5d983
treeb54e0925201b37895f9779ab8471f352d8da224f
parent78fb9c0a177eeb8f5a103eb54c46b367082cfc75

Watch.zig: fixes for windows implementation

Using --watch I noticed a couple of issues with my initial attempt. 1) The index I used as 'completion key' was not stable over time, when directories are being added/removed the key no longer corresponds with the intended dir. 2) There exists a race condition in which we receive a completion notification for a directory that was removed. My solution is to generate a key value and associate it with each Directory.

1 files changed, 20 insertions(+), 9 deletions(-)

lib/std/Build/Watch.zig+20-9
...@@ -242,8 +242,9 @@ const Os = switch (builtin.os.tag) {...@@ -242,8 +242,9 @@ const Os = switch (builtin.os.tag) {
242242
243 /// Keyed differently but indexes correspond 1:1 with `dir_table`.243 /// Keyed differently but indexes correspond 1:1 with `dir_table`.
244 handle_table: HandleTable,244 handle_table: HandleTable,
245 dir_list: std.ArrayListUnmanaged(*Directory),245 dir_list: std.AutoArrayHashMapUnmanaged(usize, *Directory),
246 io_cp: ?windows.HANDLE,246 io_cp: ?windows.HANDLE,
247 counter: usize = 0,
247248
248 const HandleTable = std.AutoArrayHashMapUnmanaged(FileId, ReactionSet);249 const HandleTable = std.AutoArrayHashMapUnmanaged(FileId, ReactionSet);
249250
...@@ -260,7 +261,8 @@ const Os = switch (builtin.os.tag) {...@@ -260,7 +261,8 @@ const Os = switch (builtin.os.tag) {
260 // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw#remarks261 // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-readdirectorychangesw#remarks
261 buffer: [64 * 1024]u8 align(@alignOf(windows.FILE_NOTIFY_INFORMATION)) = undefined,262 buffer: [64 * 1024]u8 align(@alignOf(windows.FILE_NOTIFY_INFORMATION)) = undefined,
262263
263 fn readChanges(self: *@This()) !void {264 /// Start listening for events, buffer field will be overwritten eventually.
265 fn startListening(self: *@This()) !void {
264 const r = windows.kernel32.ReadDirectoryChangesW(266 const r = windows.kernel32.ReadDirectoryChangesW(
265 self.handle,267 self.handle,
266 @ptrCast(&self.buffer),268 @ptrCast(&self.buffer),
...@@ -394,6 +396,7 @@ const Os = switch (builtin.os.tag) {...@@ -394,6 +396,7 @@ const Os = switch (builtin.os.tag) {
394 if (bytes_returned == 0) {396 if (bytes_returned == 0) {
395 std.log.warn("file system watch queue overflowed; falling back to fstat", .{});397 std.log.warn("file system watch queue overflowed; falling back to fstat", .{});
396 markAllFilesDirty(w, gpa);398 markAllFilesDirty(w, gpa);
399 try dir.startListening();
397 return true;400 return true;
398 }401 }
399 var file_name_buf: [std.fs.max_path_bytes]u8 = undefined;402 var file_name_buf: [std.fs.max_path_bytes]u8 = undefined;
...@@ -418,7 +421,8 @@ const Os = switch (builtin.os.tag) {...@@ -418,7 +421,8 @@ const Os = switch (builtin.os.tag) {
418 offset += notify.NextEntryOffset;421 offset += notify.NextEntryOffset;
419 }422 }
420423
421 try dir.readChanges();424 // We call this now since at this point we have finished reading dir.buffer.
425 try dir.startListening();
422 return any_dirty;426 return any_dirty;
423 }427 }
424428
...@@ -444,12 +448,14 @@ const Os = switch (builtin.os.tag) {...@@ -444,12 +448,14 @@ const Os = switch (builtin.os.tag) {
444 } else {448 } else {
445 assert(dh_gop.index == gop.index);449 assert(dh_gop.index == gop.index);
446 dh_gop.value_ptr.* = .{};450 dh_gop.value_ptr.* = .{};
447 try dir.readChanges();451 try dir.startListening();
448 try w.os.dir_list.insert(gpa, dh_gop.index, dir);452 const key = w.os.counter;
453 w.os.counter +%= 1;
454 try w.os.dir_list.put(gpa, key, dir);
449 w.os.io_cp = try windows.CreateIoCompletionPort(455 w.os.io_cp = try windows.CreateIoCompletionPort(
450 dir.handle,456 dir.handle,
451 w.os.io_cp,457 w.os.io_cp,
452 dh_gop.index,458 key,
453 0,459 0,
454 );460 );
455 }461 }
...@@ -495,8 +501,8 @@ const Os = switch (builtin.os.tag) {...@@ -495,8 +501,8 @@ const Os = switch (builtin.os.tag) {
495 }501 }
496 }502 }
497503
498 w.os.dir_list.items[i].deinit(gpa);504 w.os.dir_list.values()[i].deinit(gpa);
499 _ = w.os.dir_list.swapRemove(i);505 w.os.dir_list.swapRemoveAt(i);
500 w.dir_table.swapRemoveAt(i);506 w.dir_table.swapRemoveAt(i);
501 w.os.handle_table.swapRemoveAt(i);507 w.os.handle_table.swapRemoveAt(i);
502 }508 }
...@@ -653,7 +659,12 @@ pub fn wait(w: *Watch, gpa: Allocator, timeout: Timeout) !WaitResult {...@@ -653,7 +659,12 @@ pub fn wait(w: *Watch, gpa: Allocator, timeout: Timeout) !WaitResult {
653 .Normal => {659 .Normal => {
654 if (bytes_transferred == 0)660 if (bytes_transferred == 0)
655 break error.Unexpected;661 break error.Unexpected;
656 break if (try Os.markDirtySteps(w, gpa, w.os.dir_list.items[key]))662
663 // This 'orelse' detects a race condition that happens when we receive a
664 // completion notification for a directory that no longer exists in our list.
665 const dir = w.os.dir_list.get(key) orelse break .clean;
666
667 break if (try Os.markDirtySteps(w, gpa, dir))
657 .dirty668 .dirty
658 else669 else
659 .clean;670 .clean;