From 4e594c3542076e491455690ed204b0327a9c4e13 Mon Sep 17 00:00:00 2001 From: CJ van den Berg Date: Fri, 27 Mar 2026 22:07:17 +0100 Subject: [PATCH] fix: correct directory event types and stale paths after rename (inotify) --- src/backend/INotify.zig | 95 +++++++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 13 deletions(-) diff --git a/src/backend/INotify.zig b/src/backend/INotify.zig index db2864d..eb71d23 100644 --- a/src/backend/INotify.zig +++ b/src/backend/INotify.zig @@ -14,7 +14,7 @@ pub fn Create(comptime variant: InterfaceType) type { return struct { handler: *Handler, inotify_fd: std.posix.fd_t, - watches: std.AutoHashMapUnmanaged(i32, []u8), // wd -> owned path + watches: std.AutoHashMapUnmanaged(i32, WatchEntry), // wd -> {owned path, is dir} // Protects `watches` against concurrent access by the background thread // (handle_read_ready / has_watch_for_path) and the main thread // (add_watch / remove_watch). Void for the polling variant, which is @@ -37,6 +37,8 @@ pub fn Create(comptime variant: InterfaceType) type { pub const detects_file_modifications = true; pub const polling = variant == .polling; + const WatchEntry = struct { path: []u8, is_dir: bool }; + const Handler = switch (variant) { .threaded => types.Handler, .polling => types.PollingHandler, @@ -89,7 +91,7 @@ pub fn Create(comptime variant: InterfaceType) type { std.posix.close(self.stop_pipe[1]); } var it = self.watches.iterator(); - while (it.next()) |entry| allocator.free(entry.value_ptr.*); + while (it.next()) |entry| allocator.free(entry.value_ptr.*.path); self.watches.deinit(allocator); for (self.pending_renames.items) |r| allocator.free(r.path); self.pending_renames.deinit(allocator); @@ -143,13 +145,18 @@ pub fn Create(comptime variant: InterfaceType) type { return error.WatchFailed; }, } + const is_dir = blk: { + var d = std.fs.openDirAbsolute(path, .{}) catch break :blk false; + defer d.close(); + break :blk true; + }; const owned_path = try allocator.dupe(u8, path); errdefer allocator.free(owned_path); if (comptime variant == .threaded) self.watches_mutex.lock(); defer if (comptime variant == .threaded) self.watches_mutex.unlock(); const result = try self.watches.getOrPut(allocator, @intCast(wd)); - if (result.found_existing) allocator.free(result.value_ptr.*); - result.value_ptr.* = owned_path; + if (result.found_existing) allocator.free(result.value_ptr.*.path); + result.value_ptr.* = .{ .path = owned_path, .is_dir = is_dir }; } pub fn remove_watch(self: *@This(), allocator: std.mem.Allocator, path: []const u8) void { @@ -157,9 +164,9 @@ pub fn Create(comptime variant: InterfaceType) type { defer if (comptime variant == .threaded) self.watches_mutex.unlock(); var it = self.watches.iterator(); while (it.next()) |entry| { - if (!std.mem.eql(u8, entry.value_ptr.*, path)) continue; + if (!std.mem.eql(u8, entry.value_ptr.*.path, path)) continue; _ = std.os.linux.inotify_rm_watch(self.inotify_fd, entry.key_ptr.*); - allocator.free(entry.value_ptr.*); + allocator.free(entry.value_ptr.*.path); self.watches.removeByPtr(entry.key_ptr); return; } @@ -170,11 +177,37 @@ pub fn Create(comptime variant: InterfaceType) type { defer if (comptime variant == .threaded) self.watches_mutex.unlock(); var it = self.watches.iterator(); while (it.next()) |entry| { - if (std.mem.eql(u8, entry.value_ptr.*, path)) return true; + if (std.mem.eql(u8, entry.value_ptr.*.path, path)) return true; } return false; } + // Rewrite stored watch paths after a directory rename. + // Any entry equal to old_path is updated to new_path; any entry whose + // path begins with old_path + sep has its prefix replaced with new_path. + fn rename_watch_paths(self: *@This(), allocator: std.mem.Allocator, old_path: []const u8, new_path: []const u8) void { + if (comptime variant == .threaded) self.watches_mutex.lock(); + defer if (comptime variant == .threaded) self.watches_mutex.unlock(); + var it = self.watches.valueIterator(); + while (it.next()) |entry| { + if (std.mem.eql(u8, entry.path, old_path)) { + const owned = allocator.dupe(u8, new_path) catch continue; + allocator.free(entry.path); + entry.path = owned; + } else if (std.mem.startsWith(u8, entry.path, old_path) and + entry.path.len > old_path.len and + entry.path[old_path.len] == std.fs.path.sep) + { + const suffix = entry.path[old_path.len..]; // includes leading sep + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const new_sub = std.fmt.bufPrint(&path_buf, "{s}{s}", .{ new_path, suffix }) catch continue; + const owned = allocator.dupe(u8, new_sub) catch continue; + allocator.free(entry.path); + entry.path = owned; + } + } + } + pub fn handle_read_ready(self: *@This(), allocator: std.mem.Allocator) (std.posix.ReadError || error{ NoSpaceLeft, OutOfMemory, HandlerFailed })!void { const InotifyEvent = extern struct { wd: i32, @@ -184,6 +217,13 @@ pub fn Create(comptime variant: InterfaceType) type { }; var buf: [65536]u8 align(@alignOf(InotifyEvent)) = undefined; + // Src paths for which we already emitted a paired atomic rename this + // read, so IN_MOVE_SELF for the same inode can be suppressed. + var paired_src_paths: std.ArrayListUnmanaged([]u8) = .empty; + defer { + for (paired_src_paths.items) |p| allocator.free(p); + paired_src_paths.deinit(allocator); + } defer { // Any unpaired MOVED_FROM means the file was moved out of the watched tree. for (self.pending_renames.items) |r| { @@ -212,10 +252,12 @@ pub fn Create(comptime variant: InterfaceType) type { // cannot free the slice while we are still reading from it. var watched_buf: [std.fs.max_path_bytes]u8 = undefined; var watched_len: usize = 0; + var watched_is_dir = false; if (comptime variant == .threaded) self.watches_mutex.lock(); - if (self.watches.get(ev.wd)) |p| { - @memcpy(watched_buf[0..p.len], p); - watched_len = p.len; + if (self.watches.get(ev.wd)) |e| { + @memcpy(watched_buf[0..e.path.len], e.path); + watched_len = e.path.len; + watched_is_dir = e.is_dir; } if (comptime variant == .threaded) self.watches_mutex.unlock(); if (watched_len == 0) continue; @@ -252,6 +294,16 @@ pub fn Create(comptime variant: InterfaceType) type { // Complete rename pair: emit a single atomic rename message. const r = self.pending_renames.swapRemove(i); defer allocator.free(r.path); + // Rewrite any watch entries whose stored path starts with the + // old directory path so subsequent events use the new name. + if (r.object_type == .dir) + self.rename_watch_paths(allocator, r.path, full_path); + // Track the path MOVE_SELF will see so it can be suppressed. + // For directory renames the watch path has just been rewritten + // to full_path (new name); for file renames it stays r.path. + const move_self_path = if (r.object_type == .dir) full_path else r.path; + if (allocator.dupe(u8, move_self_path) catch null) |copy| + paired_src_paths.append(allocator, copy) catch allocator.free(copy); try self.handler.rename(r.path, full_path, r.object_type); } else { // No paired MOVED_FROM, file was moved in from outside the watched tree. @@ -259,14 +311,31 @@ pub fn Create(comptime variant: InterfaceType) type { try self.handler.change(full_path, EventType.created, ot); } } else if (ev.mask & IN.MOVE_SELF != 0) { - // The watched directory itself was renamed/moved away. - try self.handler.change(full_path, EventType.deleted, .dir); + // Suppress if the rename was already delivered as a paired + // MOVED_FROM/MOVED_TO event, or if it is still in pending_renames + // as an unpaired MOVED_FROM (the defer above will emit 'deleted'). + // Only emit here when the watched root was moved and its parent + // directory is not itself watched (no MOVED_FROM was generated). + const already_handled = + (for (paired_src_paths.items) |p| { + if (std.mem.eql(u8, p, full_path)) break true; + } else false) or + (for (self.pending_renames.items) |r| { + if (std.mem.eql(u8, r.path, full_path)) break true; + } else false); + if (!already_handled) + try self.handler.change(full_path, EventType.deleted, .dir); + } else if (ev.mask & IN.DELETE_SELF != 0) { + // The watched path itself was deleted. IN_DELETE_SELF does not + // set IN_ISDIR, so use the is_dir recorded at watch registration. + const object_type: ObjectType = if (watched_is_dir) .dir else .file; + try self.handler.change(full_path, EventType.deleted, object_type); } else { const is_dir = ev.mask & IN.ISDIR != 0; const object_type: ObjectType = if (is_dir) .dir else .file; const event_type: EventType = if (ev.mask & IN.CREATE != 0) .created - else if (ev.mask & (IN.DELETE | IN.DELETE_SELF) != 0) blk: { + else if (ev.mask & IN.DELETE != 0) blk: { // Suppress IN_DELETE|IN_ISDIR for subdirs that have their // own watch: IN_DELETE_SELF on that watch will fire the // same path without duplication.