From 86633e0af47dc9ff62e9cef23962a855d061bde8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 26 Jul 2023 21:35:49 -0700 Subject: [PATCH] Start time performance improvements to build tools (#3797) * Make os.cpus() faster on Linux * Fix crash See https://github.com/ziglang/zig/issues/16540 * Handle watcher_count == 0 * Add assertion * Clean up lifetimes of fs watcher a little * :scissors: * Use `errdefer` * Make the error better * Make os.cpus() more lazy * Please don't translate-c on the entire C standard library * immediately closing works correctly is still bug * ops * fmt+fixeup * add back verbose * free instead of destroy * remove destroy option for watcher tasks * flush verbose and add debug log * fixup files * use log for debug --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Co-authored-by: cirospaciari --- src/bun.js/bindings/c-bindings.cpp | 8 + src/bun.js/node/fs_events.zig | 37 +- src/bun.js/node/node_fs.zig | 9 +- src/bun.js/node/node_fs_watcher.zig | 367 +++------------- src/bun.js/node/node_os.zig | 29 +- src/bun.js/node/path_watcher.zig | 560 ++++++++++++++++++++++++ src/js/node/os.js | 64 ++- src/js/out/modules/node/os.js | 55 ++- src/lock.zig | 6 + src/string_immutable.zig | 10 +- src/watcher.zig | 26 +- test/js/node/watch/fixtures/relative.js | 39 +- test/js/node/watch/fs.watch.test.ts | 9 +- 13 files changed, 862 insertions(+), 357 deletions(-) create mode 100644 src/bun.js/node/path_watcher.zig diff --git a/src/bun.js/bindings/c-bindings.cpp b/src/bun.js/bindings/c-bindings.cpp index 0e837d04f63b8..ff4c8c4e7cd04 100644 --- a/src/bun.js/bindings/c-bindings.cpp +++ b/src/bun.js/bindings/c-bindings.cpp @@ -40,4 +40,12 @@ extern "C" void bun_ignore_sigpipe() { // ignore SIGPIPE signal(SIGPIPE, SIG_IGN); +} +extern "C" ssize_t bun_sysconf__SC_CLK_TCK() +{ +#ifdef __APPLE__ + return sysconf(_SC_CLK_TCK); +#else + return 0; +#endif } \ No newline at end of file diff --git a/src/bun.js/node/fs_events.zig b/src/bun.js/node/fs_events.zig index 54c969d0f2d33..cfcd50993115b 100644 --- a/src/bun.js/node/fs_events.zig +++ b/src/bun.js/node/fs_events.zig @@ -267,10 +267,11 @@ pub const FSEventsLoop = struct { pub const Queue = UnboundedQueue(ConcurrentTask, .next); - pub fn from(this: *ConcurrentTask, task: Task) *ConcurrentTask { + pub fn from(this: *ConcurrentTask, task: Task, auto_delete: bool) *ConcurrentTask { this.* = .{ .task = task, .next = null, + .auto_delete = auto_delete, }; return this; } @@ -339,8 +340,7 @@ pub const FSEventsLoop = struct { fn enqueueTaskConcurrent(this: *FSEventsLoop, task: Task) void { const CF = CoreFoundation.get(); var concurrent = bun.default_allocator.create(ConcurrentTask) catch unreachable; - concurrent.auto_delete = true; - this.tasks.push(concurrent.from(task)); + this.tasks.push(concurrent.from(task, true)); CF.RunLoopSourceSignal(this.signal_source); CF.RunLoopWakeUp(this.loop); } @@ -403,8 +403,9 @@ pub const FSEventsLoop = struct { } } - handle.callback(handle.ctx, path, is_file, is_rename); + handle.emit(path, is_file, is_rename); } + handle.flush(); } } } @@ -414,6 +415,7 @@ pub const FSEventsLoop = struct { this.mutex.lock(); defer this.mutex.unlock(); this.has_scheduled_watchers = false; + const watcher_count = this.watcher_count; var watchers = this.watchers.slice(); @@ -432,14 +434,18 @@ pub const FSEventsLoop = struct { // clean old paths if (this.paths) |p| { this.paths = null; - bun.default_allocator.destroy(p); + bun.default_allocator.free(p); } if (this.cf_paths) |cf| { this.cf_paths = null; CF.Release(cf); } - const paths = bun.default_allocator.alloc(?*anyopaque, this.watcher_count) catch unreachable; + if (watcher_count == 0) { + return; + } + + const paths = bun.default_allocator.alloc(?*anyopaque, watcher_count) catch unreachable; var count: u32 = 0; for (watchers) |w| { if (w) |watcher| { @@ -567,17 +573,20 @@ pub const FSEventsLoop = struct { pub const FSEventsWatcher = struct { path: string, callback: Callback, + flushCallback: UpdateEndCallback, loop: ?*FSEventsLoop, recursive: bool, ctx: ?*anyopaque, const Callback = *const fn (ctx: ?*anyopaque, path: string, is_file: bool, is_rename: bool) void; + const UpdateEndCallback = *const fn (ctx: ?*anyopaque) void; - pub fn init(loop: *FSEventsLoop, path: string, recursive: bool, callback: Callback, ctx: ?*anyopaque) *FSEventsWatcher { + pub fn init(loop: *FSEventsLoop, path: string, recursive: bool, callback: Callback, updateEnd: UpdateEndCallback, ctx: ?*anyopaque) *FSEventsWatcher { var this = bun.default_allocator.create(FSEventsWatcher) catch unreachable; this.* = FSEventsWatcher{ .path = path, .callback = callback, + .flushCallback = updateEnd, .loop = loop, .recursive = recursive, .ctx = ctx, @@ -587,6 +596,14 @@ pub const FSEventsWatcher = struct { return this; } + pub fn emit(this: *FSEventsWatcher, path: string, is_file: bool, is_rename: bool) void { + this.callback(this.ctx, path, is_file, is_rename); + } + + pub fn flush(this: *FSEventsWatcher) void { + this.flushCallback(this.ctx); + } + pub fn deinit(this: *FSEventsWatcher) void { if (this.loop) |loop| { loop.unregisterWatcher(this); @@ -595,15 +612,15 @@ pub const FSEventsWatcher = struct { } }; -pub fn watch(path: string, recursive: bool, callback: FSEventsWatcher.Callback, ctx: ?*anyopaque) !*FSEventsWatcher { +pub fn watch(path: string, recursive: bool, callback: FSEventsWatcher.Callback, updateEnd: FSEventsWatcher.UpdateEndCallback, ctx: ?*anyopaque) !*FSEventsWatcher { if (fsevents_default_loop) |loop| { - return FSEventsWatcher.init(loop, path, recursive, callback, ctx); + return FSEventsWatcher.init(loop, path, recursive, callback, updateEnd, ctx); } else { fsevents_default_loop_mutex.lock(); defer fsevents_default_loop_mutex.unlock(); if (fsevents_default_loop == null) { fsevents_default_loop = try FSEventsLoop.init(); } - return FSEventsWatcher.init(fsevents_default_loop.?, path, recursive, callback, ctx); + return FSEventsWatcher.init(fsevents_default_loop.?, path, recursive, callback, updateEnd, ctx); } } diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 8fb769facace4..314cd44bdf1f3 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -4488,8 +4488,13 @@ pub const NodeFS = struct { } pub fn watch(_: *NodeFS, args: Arguments.Watch, comptime _: Flavor) Maybe(Return.Watch) { const watcher = args.createFSWatcher() catch |err| { - args.global_this.throwError(err, "Failed to watch filename"); - return Maybe(Return.Watch){ .result = JSC.JSValue.jsUndefined() }; + var buf = std.fmt.allocPrint(bun.default_allocator, "{s} watching {}", .{ @errorName(err), strings.QuotedFormatter{ .text = args.path.slice() } }) catch unreachable; + defer bun.default_allocator.free(buf); + args.global_this.throwValue((JSC.SystemError{ + .message = bun.String.init(buf), + .path = bun.String.init(args.path.slice()), + }).toErrorInstance(args.global_this)); + return Maybe(Return.Watch){ .result = JSC.JSValue.undefined }; }; return Maybe(Return.Watch){ .result = watcher }; } diff --git a/src/bun.js/node/node_fs_watcher.zig b/src/bun.js/node/node_fs_watcher.zig index d0af350c0d70f..3144f4f6bb08b 100644 --- a/src/bun.js/node/node_fs_watcher.zig +++ b/src/bun.js/node/node_fs_watcher.zig @@ -7,6 +7,7 @@ const Encoder = JSC.WebCore.Encoder; const Mutex = @import("../../lock.zig").Lock; const FSEvents = @import("./fs_events.zig"); +const PathWatcher = @import("./path_watcher.zig"); const VirtualMachine = JSC.VirtualMachine; const EventLoop = JSC.EventLoop; @@ -18,30 +19,16 @@ const StoredFileDescriptorType = bun.StoredFileDescriptorType; const Environment = bun.Environment; pub const FSWatcher = struct { - const watcher = @import("../../watcher.zig"); - const options = @import("../../options.zig"); - pub const Watcher = watcher.NewWatcher(*FSWatcher); - const log = Output.scoped(.FSWatcher, false); - - pub const ChangeEvent = struct { - hash: Watcher.HashType = 0, - event_type: FSWatchTask.EventType = .change, - time_stamp: i64 = 0, - }; - - onAccept: std.ArrayHashMapUnmanaged(FSWatcher.Watcher.HashType, bun.BabyList(OnAcceptCallback), bun.ArrayIdentityContext, false) = .{}, ctx: *VirtualMachine, verbose: bool = false, - file_paths: bun.BabyList(string) = .{}, entry_path: ?string = null, entry_dir: string = "", - last_change_event: ChangeEvent = .{}, // JSObject mutex: Mutex, signal: ?*JSC.AbortSignal, persistent: bool, - default_watcher: ?*FSWatcher.Watcher, + default_watcher: ?*PathWatcher.PathWatcher, fsevents_watcher: ?*FSEvents.FSEventsWatcher, poll_ref: JSC.PollRef = .{}, globalThis: *JSC.JSGlobalObject, @@ -52,6 +39,7 @@ pub const FSWatcher = struct { // counts pending tasks so we only deinit after all tasks are done task_count: u32, has_pending_activity: std.atomic.Atomic(bool), + current_task: FSWatchTask = undefined, pub usingnamespace JSC.Codegen.JSFSWatcher; pub fn eventLoop(this: FSWatcher) *EventLoop { @@ -65,14 +53,9 @@ pub const FSWatcher = struct { pub fn deinit(this: *FSWatcher) void { // stop all managers and signals this.detach(); - - while (this.file_paths.popOrNull()) |file_path| { - bun.default_allocator.destroy(file_path); - } - this.file_paths.deinitWithAllocator(bun.default_allocator); if (this.entry_path) |path| { this.entry_path = null; - bun.default_allocator.destroy(path); + bun.default_allocator.free(path); } bun.default_allocator.destroy(this); } @@ -91,19 +74,13 @@ pub const FSWatcher = struct { abort, }; - pub const EventFreeType = enum { - destroy, - free, - none, - }; - pub const Entry = struct { file_path: string, event_type: EventType, - free_type: EventFreeType, + needs_free: bool, }; - pub fn append(this: *FSWatchTask, file_path: string, event_type: EventType, free_type: EventFreeType) void { + pub fn append(this: *FSWatchTask, file_path: string, event_type: EventType, needs_free: bool) void { if (this.count == 8) { this.enqueue(); var ctx = this.ctx; @@ -116,7 +93,7 @@ pub const FSWatcher = struct { this.entries[this.count] = .{ .file_path = file_path, .event_type = event_type, - .free_type = free_type, + .needs_free = needs_free, }; this.count += 1; } @@ -163,14 +140,12 @@ pub const FSWatcher = struct { this.cleanEntries(); } pub fn cleanEntries(this: *FSWatchTask) void { - while (this.count > 0) { - this.count -= 1; - switch (this.entries[this.count].free_type) { - .destroy => bun.default_allocator.destroy(this.entries[this.count].file_path), - .free => bun.default_allocator.free(this.entries[this.count].file_path), - else => {}, + for (this.entries[0..this.count]) |entry| { + if (entry.needs_free) { + bun.default_allocator.free(entry.file_path); } } + this.count = 0; } pub fn deinit(this: *FSWatchTask) void { @@ -179,241 +154,66 @@ pub const FSWatcher = struct { } }; - fn NewCallback(comptime FunctionSignature: type) type { - return union(enum) { - javascript_callback: JSC.Strong, - zig_callback: struct { - ptr: *anyopaque, - function: *const FunctionSignature, - }, - }; - } - - pub const OnAcceptCallback = NewCallback(fn ( - vm: *JSC.VirtualMachine, - specifier: []const u8, - ) void); - - fn addDirectory(ctx: *FSWatcher, fs_watcher: *FSWatcher.Watcher, fd: StoredFileDescriptorType, file_path: string, recursive: bool, buf: *[bun.MAX_PATH_BYTES + 1]u8, is_entry_path: bool) !void { - var dir_path_clone = bun.default_allocator.dupeZ(u8, file_path) catch unreachable; - - if (is_entry_path) { - ctx.entry_path = dir_path_clone; - ctx.entry_dir = dir_path_clone; - } else { - ctx.file_paths.push(bun.default_allocator, dir_path_clone) catch unreachable; - } - fs_watcher.addDirectory(fd, dir_path_clone, FSWatcher.Watcher.getHash(file_path), false) catch |err| { - ctx.deinit(); - fs_watcher.deinit(true); - return err; - }; - - var iter = (std.fs.IterableDir{ .dir = std.fs.Dir{ - .fd = fd, - } }).iterate(); - - while (iter.next() catch |err| { - ctx.deinit(); - fs_watcher.deinit(true); - return err; - }) |entry| { - var parts = [2]string{ dir_path_clone, entry.name }; - var entry_path = Path.joinAbsStringBuf( - Fs.FileSystem.instance.topLevelDirWithoutTrailingSlash(), - buf, - &parts, - .auto, - ); - - buf[entry_path.len] = 0; - var entry_path_z = buf[0..entry_path.len :0]; - - var fs_info = fdFromAbsolutePathZ(entry_path_z) catch |err| { - ctx.deinit(); - fs_watcher.deinit(true); - return err; - }; - - if (fs_info.is_file) { - const file_path_clone = bun.default_allocator.dupeZ(u8, entry_path) catch unreachable; - - ctx.file_paths.push(bun.default_allocator, file_path_clone) catch unreachable; - - fs_watcher.addFile(fs_info.fd, file_path_clone, FSWatcher.Watcher.getHash(entry_path), options.Loader.file, 0, null, false) catch |err| { - ctx.deinit(); - fs_watcher.deinit(true); - return err; - }; - } else { - if (recursive) { - addDirectory(ctx, fs_watcher, fs_info.fd, entry_path, recursive, buf, false) catch |err| { - ctx.deinit(); - fs_watcher.deinit(true); - return err; - }; - } - } - } - } - - pub fn onError( - this: *FSWatcher, - err: anyerror, - ) void { - var current_task: FSWatchTask = .{ - .ctx = this, - }; - current_task.append(@errorName(err), .@"error", .none); - current_task.enqueue(); - } - pub fn onFSEventUpdate( ctx: ?*anyopaque, path: string, - _: bool, + is_file: bool, is_rename: bool, ) void { - const this = bun.cast(*FSWatcher, ctx.?); + // only called by FSEventUpdate - var current_task: FSWatchTask = .{ - .ctx = this, - }; - defer current_task.enqueue(); + const this = bun.cast(*FSWatcher, ctx.?); const relative_path = bun.default_allocator.dupe(u8, path) catch unreachable; const event_type: FSWatchTask.EventType = if (is_rename) .rename else .change; - current_task.append(relative_path, event_type, .destroy); - } - - pub fn onFileUpdate( - this: *FSWatcher, - events: []watcher.WatchEvent, - changed_files: []?[:0]u8, - watchlist: watcher.Watchlist, - ) void { - var slice = watchlist.slice(); - const file_paths = slice.items(.file_path); - - var counts = slice.items(.count); - const kinds = slice.items(.kind); - var _on_file_update_path_buf: [bun.MAX_PATH_BYTES]u8 = undefined; - - var ctx = this.default_watcher.?; - defer ctx.flushEvictions(); - defer Output.flush(); - - var bundler = if (@TypeOf(this.ctx.bundler) == *bun.Bundler) - this.ctx.bundler - else - &this.ctx.bundler; - - var fs: *Fs.FileSystem = bundler.fs; - - var current_task: FSWatchTask = .{ - .ctx = this, - }; - defer current_task.enqueue(); - - const time_stamp = std.time.milliTimestamp(); - const time_diff = time_stamp - this.last_change_event.time_stamp; - - for (events) |event| { - const file_path = file_paths[event.index]; - const update_count = counts[event.index] + 1; - counts[event.index] = update_count; - const kind = kinds[event.index]; - - if (comptime Environment.isDebug) { - if (this.verbose) { - Output.prettyErrorln("[watch] {s} ({s}, {})", .{ file_path, @tagName(kind), event.op }); - } + if (this.verbose) { + if (is_file) { + Output.prettyErrorln(" File changed: {s}", .{relative_path}); + } else { + Output.prettyErrorln(" Dir changed: {s}", .{relative_path}); } + } - switch (kind) { - .file => { - if (event.op.delete) { - ctx.removeAtIndex( - event.index, - 0, - &.{}, - .file, - ); - } - - var file_hash: FSWatcher.Watcher.HashType = FSWatcher.Watcher.getHash(file_path); - - if (event.op.write or event.op.delete or event.op.rename) { - const event_type: FSWatchTask.EventType = if (event.op.delete or event.op.rename or event.op.move_to) .rename else .change; - // skip consecutive duplicates - if ((this.last_change_event.time_stamp == 0 or time_diff > 1) or this.last_change_event.event_type != event_type and this.last_change_event.hash != file_hash) { - this.last_change_event.time_stamp = time_stamp; - this.last_change_event.event_type = event_type; - this.last_change_event.hash = file_hash; - - const relative_slice = fs.relative(this.entry_dir, file_path); - - if (this.verbose) - Output.prettyErrorln("File changed: {s}", .{relative_slice}); - - const relative_path = bun.default_allocator.dupe(u8, relative_slice) catch unreachable; - - current_task.append(relative_path, event_type, .destroy); - } - } - }, - .directory => { - // macOS should use FSEvents for directories - if (comptime Environment.isMac) { - @panic("Unexpected directory watch"); - } - - const affected = event.names(changed_files); - - for (affected) |changed_name_| { - const changed_name: []const u8 = bun.asByteSlice(changed_name_.?); - if (changed_name.len == 0 or changed_name[0] == '~' or changed_name[0] == '.') continue; - - var file_hash: FSWatcher.Watcher.HashType = 0; - const relative_slice: string = brk: { - var file_path_without_trailing_slash = std.mem.trimRight(u8, file_path, std.fs.path.sep_str); - - @memcpy(_on_file_update_path_buf[0..file_path_without_trailing_slash.len], file_path_without_trailing_slash); - - _on_file_update_path_buf[file_path_without_trailing_slash.len] = std.fs.path.sep; - - @memcpy(_on_file_update_path_buf[file_path_without_trailing_slash.len + 1 ..][0..changed_name.len], changed_name); - const path_slice = _on_file_update_path_buf[0 .. file_path_without_trailing_slash.len + changed_name.len + 1]; - file_hash = FSWatcher.Watcher.getHash(path_slice); - - const relative = fs.relative(this.entry_dir, path_slice); + this.current_task.append(relative_path, event_type, true); + } - break :brk relative; - }; + pub fn onPathUpdate(ctx: ?*anyopaque, path: string, is_file: bool, event_type: PathWatcher.PathWatcher.EventType) void { + // only called by PathWatcher - // skip consecutive duplicates - const event_type: FSWatchTask.EventType = .rename; // renaming folders, creating folder or files will be always be rename - if ((this.last_change_event.time_stamp == 0 or time_diff > 1) or this.last_change_event.event_type != event_type and this.last_change_event.hash != file_hash) { - const relative_path = bun.default_allocator.dupe(u8, relative_slice) catch unreachable; + const this = bun.cast(*FSWatcher, ctx.?); - this.last_change_event.time_stamp = time_stamp; - this.last_change_event.event_type = event_type; - this.last_change_event.hash = file_hash; + const relative_path = bun.default_allocator.dupe(u8, path) catch unreachable; - current_task.append(relative_path, event_type, .destroy); + if (this.verbose and event_type != .@"error") { + if (is_file) { + Output.prettyErrorln(" File changed: {s}", .{relative_path}); + } else { + Output.prettyErrorln(" Dir changed: {s}", .{relative_path}); + } + } - if (this.verbose) - Output.prettyErrorln(" Dir change: {s}", .{relative_path}); - } - } + switch (event_type) { + .rename => { + this.current_task.append(relative_path, .rename, true); + }, + .change => { + this.current_task.append(relative_path, .change, true); + }, + else => { + this.current_task.append(relative_path, .@"error", true); + }, + } + } - if (this.verbose and affected.len == 0) { - Output.prettyErrorln(" Dir change: {s}", .{fs.relative(this.entry_dir, file_path)}); - } - }, - } + pub fn onUpdateEnd(ctx: ?*anyopaque) void { + const this = bun.cast(*FSWatcher, ctx.?); + if (this.verbose) { + Output.flush(); } + // we only enqueue after all events are processed + // this is called by FSEventsWatcher or PathWatcher + this.current_task.enqueue(); } pub const Arguments = struct { @@ -589,7 +389,7 @@ pub const FSWatcher = struct { var current_task: FSWatchTask = .{ .ctx = this, }; - current_task.append("", .abort, .none); + current_task.append("", .abort, false); current_task.enqueue(); } else { // watch for abortion @@ -753,11 +553,6 @@ pub const FSWatcher = struct { // this can be called multiple times pub fn detach(this: *FSWatcher) void { - if (this.persistent) { - this.persistent = false; - this.poll_ref.unref(this.ctx); - } - if (this.signal) |signal| { this.signal = null; signal.detach(this); @@ -765,7 +560,7 @@ pub const FSWatcher = struct { if (this.default_watcher) |default_watcher| { this.default_watcher = null; - default_watcher.deinit(true); + default_watcher.deinit(); } if (this.fsevents_watcher) |fsevents_watcher| { @@ -773,6 +568,10 @@ pub const FSWatcher = struct { fsevents_watcher.deinit(); } + if (this.persistent) { + this.persistent = false; + this.poll_ref.unref(this.ctx); + } this.js_this = .zero; } @@ -846,6 +645,10 @@ pub const FSWatcher = struct { const vm = args.global_this.bunVM(); ctx.* = .{ .ctx = vm, + .current_task = .{ + .ctx = ctx, + .count = 0, + }, .mutex = Mutex.init(), .signal = if (args.signal) |s| s.ref() else null, .persistent = args.persistent, @@ -858,60 +661,28 @@ pub const FSWatcher = struct { .task_count = 0, .has_pending_activity = std.atomic.Atomic(bool).init(true), .verbose = args.verbose, - .file_paths = bun.BabyList(string).initCapacity(bun.default_allocator, 1) catch |err| { - ctx.deinit(); - return err; - }, }; + errdefer ctx.deinit(); + if (comptime Environment.isMac) { if (!fs_type.is_file) { var dir_path_clone = bun.default_allocator.dupeZ(u8, file_path) catch unreachable; ctx.entry_path = dir_path_clone; ctx.entry_dir = dir_path_clone; - ctx.fsevents_watcher = FSEvents.watch(dir_path_clone, args.recursive, onFSEventUpdate, bun.cast(*anyopaque, ctx)) catch |err| { - ctx.deinit(); - return err; - }; + ctx.fsevents_watcher = try FSEvents.watch(dir_path_clone, args.recursive, onFSEventUpdate, onUpdateEnd, bun.cast(*anyopaque, ctx)); ctx.initJS(args.listener); return ctx; } } - var default_watcher = FSWatcher.Watcher.init( - ctx, - vm.bundler.fs, - bun.default_allocator, - ) catch |err| { - ctx.deinit(); - return err; - }; - - ctx.default_watcher = default_watcher; - - if (fs_type.is_file) { - var file_path_clone = bun.default_allocator.dupeZ(u8, file_path) catch unreachable; - - ctx.entry_path = file_path_clone; - ctx.entry_dir = std.fs.path.dirname(file_path_clone) orelse file_path_clone; + var file_path_clone = bun.default_allocator.dupeZ(u8, file_path) catch unreachable; - default_watcher.addFile(fs_type.fd, file_path_clone, FSWatcher.Watcher.getHash(file_path), options.Loader.file, 0, null, false) catch |err| { - ctx.deinit(); - return err; - }; - } else { - addDirectory(ctx, default_watcher, fs_type.fd, file_path, args.recursive, &buf, true) catch |err| { - ctx.deinit(); - return err; - }; - } - - default_watcher.start() catch |err| { - ctx.deinit(); - return err; - }; + ctx.entry_path = file_path_clone; + ctx.entry_dir = std.fs.path.dirname(file_path_clone) orelse file_path_clone; + ctx.default_watcher = try PathWatcher.watch(vm, file_path_clone, args.recursive, onPathUpdate, onUpdateEnd, bun.cast(*anyopaque, ctx)); ctx.initJS(args.listener); return ctx; diff --git a/src/bun.js/node/node_os.zig b/src/bun.js/node/node_os.zig index 5710b5c212cd2..bd9844db0c985 100644 --- a/src/bun.js/node/node_os.zig +++ b/src/bun.js/node/node_os.zig @@ -110,9 +110,14 @@ pub const Os = struct { // Read /proc/stat to get number of CPUs and times if (std.fs.openFileAbsolute("/proc/stat", .{})) |file| { defer file.close(); - var reader = file.reader(); + // TODO: remove all usages of file.reader(). zig's std.io.Reader() + // is extremely slow and should rarely ever be used in Bun until + // that is fixed. + var buffered_reader = std.io.BufferedReader(8096, @TypeOf(file.reader())){ .unbuffered_reader = file.reader() }; + var reader = buffered_reader.reader(); // Skip the first line (aggregate of all CPUs) + // TODO: use indexOfNewline try reader.skipUntilDelimiterOrEof('\n'); // Read each CPU line @@ -148,18 +153,23 @@ pub const Os = struct { // Read /proc/cpuinfo to get model information (optional) if (std.fs.openFileAbsolute("/proc/cpuinfo", .{})) |file| { defer file.close(); - var reader = file.reader(); + // TODO: remove all usages of file.reader(). zig's std.io.Reader() + // is extremely slow and should rarely ever be used in Bun until + // that is fixed. + var buffered_reader = std.io.BufferedReader(8096, @TypeOf(file.reader())){ .unbuffered_reader = file.reader() }; + var reader = buffered_reader.reader(); + const key_processor = "processor\t: "; const key_model_name = "model name\t: "; var cpu_index: u32 = 0; while (try reader.readUntilDelimiterOrEof(&line_buffer, '\n')) |line| { - if (std.mem.startsWith(u8, line, key_processor)) { + if (strings.hasPrefixComptime(line, key_processor)) { // If this line starts a new processor, parse the index from the line const digits = std.mem.trim(u8, line[key_processor.len..], " \t\n"); cpu_index = try std.fmt.parseInt(u32, digits, 10); if (cpu_index >= num_cpus) return error.too_may_cpus; - } else if (std.mem.startsWith(u8, line, key_model_name)) { + } else if (strings.hasPrefixComptime(line, key_model_name)) { // If this is the model name, extract it and store on the current cpu const model_name = line[key_model_name.len..]; const cpu = JSC.JSObject.getIndex(values, globalThis, cpu_index); @@ -176,9 +186,8 @@ pub const Os = struct { } // Read /sys/devices/system/cpu/cpu{}/cpufreq/scaling_cur_freq to get current frequency (optional) - var cpu_index: u32 = 0; - while (cpu_index < num_cpus) : (cpu_index += 1) { - const cpu = JSC.JSObject.getIndex(values, globalThis, cpu_index); + for (0..num_cpus) |cpu_index| { + const cpu = JSC.JSObject.getIndex(values, globalThis, @truncate(cpu_index)); var path_buf: [128]u8 = undefined; const path = try std.fmt.bufPrint(&path_buf, "/sys/devices/system/cpu/cpu{}/cpufreq/scaling_cur_freq", .{cpu_index}); @@ -199,6 +208,7 @@ pub const Os = struct { return values; } + extern fn bun_sysconf__SC_CLK_TCK() isize; fn cpusImplDarwin(globalThis: *JSC.JSGlobalObject) !JSC.JSValue { const local_bindings = @import("../../darwin_c.zig"); const c = std.c; @@ -243,10 +253,7 @@ pub const Os = struct { } // Get the multiplier; this is the number of ms/tick - const unistd = @cImport({ - @cInclude("unistd.h"); - }); - const ticks: i64 = unistd.sysconf(unistd._SC_CLK_TCK); + const ticks: i64 = bun_sysconf__SC_CLK_TCK(); const multiplier = 1000 / @as(u64, @intCast(ticks)); // Set up each CPU value in the return diff --git a/src/bun.js/node/path_watcher.zig b/src/bun.js/node/path_watcher.zig new file mode 100644 index 0000000000000..5a22cb783c40e --- /dev/null +++ b/src/bun.js/node/path_watcher.zig @@ -0,0 +1,560 @@ +const std = @import("std"); + +const UnboundedQueue = @import("../unbounded_queue.zig").UnboundedQueue; +const Path = @import("../../resolver/resolve_path.zig"); +const Fs = @import("../../fs.zig"); +const Mutex = @import("../../lock.zig").Lock; + +const bun = @import("root").bun; +const Output = bun.Output; +const Environment = bun.Environment; +const StoredFileDescriptorType = bun.StoredFileDescriptorType; +const string = bun.string; +const JSC = bun.JSC; +const VirtualMachine = JSC.VirtualMachine; + +const sync = @import("../../sync.zig"); +const Semaphore = sync.Semaphore; + +var default_manager_mutex: Mutex = Mutex.init(); +var default_manager: ?*PathWatcherManager = null; + +pub const PathWatcherManager = struct { + const GenericWatcher = @import("../../watcher.zig"); + const options = @import("../../options.zig"); + pub const Watcher = GenericWatcher.NewWatcher(*PathWatcherManager); + const log = Output.scoped(.PathWatcherManager, false); + main_watcher: *Watcher, + + watchers: bun.BabyList(?*PathWatcher) = .{}, + watcher_count: u32 = 0, + vm: *JSC.VirtualMachine, + file_paths: bun.StringHashMap(PathInfo), + deinit_on_last_watcher: bool = false, + mutex: Mutex, + + const PathInfo = struct { + fd: StoredFileDescriptorType = 0, + is_file: bool = true, + path: [:0]const u8, + dirname: string, + refs: u32 = 0, + hash: Watcher.HashType, + }; + + fn _fdFromAbsolutePathZ( + this: *PathWatcherManager, + path: [:0]const u8, + ) !PathInfo { + if (this.file_paths.getEntry(path)) |entry| { + var info = entry.value_ptr; + info.refs += 1; + return info.*; + } + const cloned_path = try bun.default_allocator.dupeZ(u8, path); + errdefer bun.default_allocator.destroy(cloned_path); + + var stat = try bun.C.lstat_absolute(cloned_path); + var result = PathInfo{ + .path = cloned_path, + .dirname = cloned_path, + .hash = Watcher.getHash(cloned_path), + .refs = 1, + }; + + switch (stat.kind) { + .sym_link => { + var file = try std.fs.openFileAbsoluteZ(cloned_path, .{ .mode = .read_only }); + result.fd = file.handle; + const _stat = try file.stat(); + + result.is_file = _stat.kind != .directory; + if (result.is_file) { + result.dirname = std.fs.path.dirname(cloned_path) orelse cloned_path; + } + }, + .directory => { + const dir = (try std.fs.openIterableDirAbsoluteZ(cloned_path, .{ + .access_sub_paths = true, + })).dir; + result.fd = dir.fd; + result.is_file = false; + }, + else => { + const file = try std.fs.openFileAbsoluteZ(cloned_path, .{ .mode = .read_only }); + result.fd = file.handle; + result.is_file = true; + result.dirname = std.fs.path.dirname(cloned_path) orelse cloned_path; + }, + } + + _ = try this.file_paths.put(cloned_path, result); + return result; + } + + pub fn init(vm: *JSC.VirtualMachine) !*PathWatcherManager { + const this = try bun.default_allocator.create(PathWatcherManager); + errdefer bun.default_allocator.destroy(this); + var watchers = bun.BabyList(?*PathWatcher).initCapacity(bun.default_allocator, 1) catch |err| { + bun.default_allocator.destroy(this); + return err; + }; + errdefer watchers.deinitWithAllocator(bun.default_allocator); + var manager = PathWatcherManager{ + .file_paths = bun.StringHashMap(PathInfo).init(bun.default_allocator), + .watchers = watchers, + .main_watcher = try Watcher.init( + this, + vm.bundler.fs, + bun.default_allocator, + ), + .vm = vm, + .watcher_count = 0, + .mutex = Mutex.init(), + }; + + this.* = manager; + try this.main_watcher.start(); + return this; + } + + pub fn onFileUpdate( + this: *PathWatcherManager, + events: []GenericWatcher.WatchEvent, + changed_files: []?[:0]u8, + watchlist: GenericWatcher.Watchlist, + ) void { + var slice = watchlist.slice(); + const file_paths = slice.items(.file_path); + + var counts = slice.items(.count); + const kinds = slice.items(.kind); + var _on_file_update_path_buf: [bun.MAX_PATH_BYTES]u8 = undefined; + + var ctx = this.main_watcher; + defer ctx.flushEvictions(); + + const timestamp = std.time.milliTimestamp(); + + this.mutex.lock(); + defer this.mutex.unlock(); + + const watchers = this.watchers.slice(); + + for (events) |event| { + const file_path = file_paths[event.index]; + const update_count = counts[event.index] + 1; + counts[event.index] = update_count; + const kind = kinds[event.index]; + + if (comptime Environment.isDebug) { + log("[watch] {s} ({s}, {})", .{ file_path, @tagName(kind), event.op }); + } + + switch (kind) { + .file => { + if (event.op.delete) { + ctx.removeAtIndex( + event.index, + 0, + &.{}, + .file, + ); + } + + if (event.op.write or event.op.delete or event.op.rename) { + const event_type: PathWatcher.EventType = if (event.op.delete or event.op.rename or event.op.move_to) .rename else .change; + const hash = Watcher.getHash(file_path); + + for (watchers) |w| { + if (w) |watcher| { + const entry_point = watcher.path.dirname; + var path = file_path; + + if (path.len < entry_point.len) { + continue; + } + if (watcher.path.is_file) { + if (watcher.path.hash != hash) { + continue; + } + } else { + if (!bun.strings.startsWith(path, entry_point)) { + continue; + } + } + // Remove common prefix, unless the watched folder is "/" + if (!(path.len == 1 and entry_point[0] == '/')) { + path = path[entry_point.len..]; + + // Ignore events with path equal to directory itself + if (path.len <= 1) { + continue; + } + if (path.len == 0) { + while (path.len > 0) { + if (bun.strings.startsWithChar(path, '/')) { + path = path[1..]; + break; + } else { + path = path[1..]; + } + } + } else { + // Skip forward slash + path = path[1..]; + } + } + + // Do not emit events from subdirectories (without option set) + if (path.len == 0 or (bun.strings.containsChar(path, '/') and !watcher.recursive)) { + continue; + } + watcher.emit(path, hash, timestamp, true, event_type); + } + } + } + }, + .directory => { + const affected = event.names(changed_files); + + for (affected) |changed_name_| { + const changed_name: []const u8 = bun.asByteSlice(changed_name_.?); + if (changed_name.len == 0 or changed_name[0] == '~' or changed_name[0] == '.') continue; + + var file_path_without_trailing_slash = std.mem.trimRight(u8, file_path, std.fs.path.sep_str); + + @memcpy(_on_file_update_path_buf[0..file_path_without_trailing_slash.len], file_path_without_trailing_slash); + + _on_file_update_path_buf[file_path_without_trailing_slash.len] = std.fs.path.sep; + + @memcpy(_on_file_update_path_buf[file_path_without_trailing_slash.len + 1 ..][0..changed_name.len], changed_name); + const len = file_path_without_trailing_slash.len + changed_name.len; + const path_slice = _on_file_update_path_buf[0 .. len + 1]; + + const hash = Watcher.getHash(path_slice); + + // skip consecutive duplicates + const event_type: PathWatcher.EventType = .rename; // renaming folders, creating folder or files will be always be rename + for (watchers) |w| { + if (w) |watcher| { + const entry_point = watcher.path.dirname; + var path = path_slice; + + if (watcher.path.is_file or path.len < entry_point.len or !bun.strings.startsWith(path, entry_point)) { + continue; + } + // Remove common prefix, unless the watched folder is "/" + if (!(path.len == 1 and entry_point[0] == '/')) { + path = path[entry_point.len..]; + + if (path.len == 0) { + while (path.len > 0) { + if (bun.strings.startsWithChar(path, '/')) { + path = path[1..]; + break; + } else { + path = path[1..]; + } + } + } else { + // Skip forward slash + path = path[1..]; + } + } + + // Do not emit events from subdirectories (without option set) + if (path.len == 0 or (bun.strings.containsChar(path, '/') and !watcher.recursive)) { + continue; + } + + watcher.emit(path, hash, timestamp, false, event_type); + } + } + } + }, + } + } + + if (comptime Environment.isDebug) { + Output.flush(); + } + for (watchers) |w| { + if (w) |watcher| { + if (watcher.needs_flush) watcher.flush(); + } + } + } + + pub fn onError( + this: *PathWatcherManager, + err: anyerror, + ) void { + this.mutex.lock(); + const watchers = this.watchers.slice(); + const timestamp = std.time.milliTimestamp(); + + // stop all watchers + for (watchers) |w| { + if (w) |watcher| { + watcher.emit(@errorName(err), 0, timestamp, false, .@"error"); + watcher.flush(); + } + } + + // we need a new manager at this point + default_manager_mutex.lock(); + defer default_manager_mutex.unlock(); + default_manager = null; + + // deinit manager when all watchers are closed + this.mutex.unlock(); + this.deinit(); + } + + fn addDirectory(this: *PathWatcherManager, watcher: *PathWatcher, path: PathInfo, buf: *[bun.MAX_PATH_BYTES + 1]u8) !void { + const fd = path.fd; + try this.main_watcher.addDirectory(fd, path.path, path.hash, false); + + var iter = (std.fs.IterableDir{ .dir = std.fs.Dir{ + .fd = fd, + } }).iterate(); + + while (try iter.next()) |entry| { + var parts = [2]string{ path.path, entry.name }; + var entry_path = Path.joinAbsStringBuf( + Fs.FileSystem.instance.topLevelDirWithoutTrailingSlash(), + buf, + &parts, + .auto, + ); + + buf[entry_path.len] = 0; + var entry_path_z = buf[0..entry_path.len :0]; + + var child_path = try this._fdFromAbsolutePathZ(entry_path_z); + errdefer this._decrementPathRef(entry_path_z); + try watcher.file_paths.push(bun.default_allocator, child_path.path); + + if (child_path.is_file) { + try this.main_watcher.addFile(child_path.fd, child_path.path, child_path.hash, options.Loader.file, 0, null, false); + } else { + if (watcher.recursive) { + try this.addDirectory(watcher, child_path, buf); + } + } + } + } + + fn registerWatcher(this: *PathWatcherManager, watcher: *PathWatcher) !void { + this.mutex.lock(); + defer this.mutex.unlock(); + + if (this.watcher_count == this.watchers.len) { + this.watcher_count += 1; + this.watchers.push(bun.default_allocator, watcher) catch unreachable; + } else { + var watchers = this.watchers.slice(); + for (watchers, 0..) |w, i| { + if (w == null) { + watchers[i] = watcher; + this.watcher_count += 1; + break; + } + } + } + const path = watcher.path; + if (path.is_file) { + try this.main_watcher.addFile(path.fd, path.path, path.hash, options.Loader.file, 0, null, false); + } else { + var buf: [bun.MAX_PATH_BYTES + 1]u8 = undefined; + try this.addDirectory(watcher, path, &buf); + } + } + + fn _decrementPathRef(this: *PathWatcherManager, file_path: [:0]const u8) void { + if (this.file_paths.getEntry(file_path)) |entry| { + var path = entry.value_ptr; + if (path.refs > 0) { + path.refs -= 1; + if (path.refs == 0) { + const path_ = path.path; + this.main_watcher.remove(path.hash); + _ = this.file_paths.remove(path_); + bun.default_allocator.free(path_); + } + } + } + } + + fn unregisterWatcher(this: *PathWatcherManager, watcher: *PathWatcher) void { + this.mutex.lock(); + defer this.mutex.unlock(); + + var watchers = this.watchers.slice(); + defer { + if (this.deinit_on_last_watcher and this.watcher_count == 0) { + this.deinit(); + } + } + + for (watchers, 0..) |w, i| { + if (w) |item| { + if (item == watcher) { + watchers[i] = null; + // if is the last one just pop + if (i == watchers.len - 1) { + this.watchers.len -= 1; + } + this.watcher_count -= 1; + + while (watcher.file_paths.popOrNull()) |file_path| { + this._decrementPathRef(file_path); + } + break; + } + } + } + } + + fn deinit(this: *PathWatcherManager) void { + // enable to create a new manager + default_manager_mutex.lock(); + defer default_manager_mutex.unlock(); + if (default_manager == this) { + default_manager = null; + } + + // only deinit if no watchers are registered + if (this.watcher_count > 0) { + // wait last watcher to close + this.deinit_on_last_watcher = true; + return; + } + + this.main_watcher.deinit(false); + + if (this.watcher_count > 0) { + while (this.watchers.popOrNull()) |watcher| { + if (watcher) |w| { + // unlink watcher + w.manager = null; + } + } + } + + // close all file descriptors and free paths + var it = this.file_paths.iterator(); + while (it.next()) |*entry| { + const path = entry.value_ptr.*; + std.os.close(path.fd); + bun.default_allocator.destroy(path.path); + } + + this.file_paths.deinit(); + + this.watchers.deinitWithAllocator(bun.default_allocator); + // this.mutex.deinit(); + + bun.default_allocator.destroy(this); + } +}; + +pub const PathWatcher = struct { + path: PathWatcherManager.PathInfo, + callback: Callback, + flushCallback: UpdateEndCallback, + manager: ?*PathWatcherManager, + recursive: bool, + needs_flush: bool = false, + ctx: ?*anyopaque, + // all watched file paths (including subpaths) except by path it self + file_paths: bun.BabyList([:0]const u8) = .{}, + last_change_event: ChangeEvent = .{}, + + pub const ChangeEvent = struct { + hash: PathWatcherManager.Watcher.HashType = 0, + event_type: EventType = .change, + time_stamp: i64 = 0, + }; + + pub const EventType = enum { + rename, + change, + @"error", + }; + const Callback = *const fn (ctx: ?*anyopaque, path: string, is_file: bool, event_type: EventType) void; + const UpdateEndCallback = *const fn (ctx: ?*anyopaque) void; + + pub fn init(manager: *PathWatcherManager, path: PathWatcherManager.PathInfo, recursive: bool, callback: Callback, updateEndCallback: UpdateEndCallback, ctx: ?*anyopaque) !*PathWatcher { + var this = try bun.default_allocator.create(PathWatcher); + this.* = PathWatcher{ + .path = path, + .callback = callback, + .manager = manager, + .recursive = recursive, + .flushCallback = updateEndCallback, + .ctx = ctx, + .file_paths = bun.BabyList([:0]const u8).initCapacity(bun.default_allocator, 1) catch |err| { + bun.default_allocator.destroy(this); + return err; + }, + }; + + errdefer this.deinit(); + + try manager.registerWatcher(this); + return this; + } + + pub fn emit(this: *PathWatcher, path: string, hash: PathWatcherManager.Watcher.HashType, time_stamp: i64, is_file: bool, event_type: EventType) void { + const time_diff = time_stamp - this.last_change_event.time_stamp; + // skip consecutive duplicates + if ((this.last_change_event.time_stamp == 0 or time_diff > 1) or this.last_change_event.event_type != event_type and this.last_change_event.hash != hash) { + this.last_change_event.time_stamp = time_stamp; + this.last_change_event.event_type = event_type; + this.last_change_event.hash = hash; + this.needs_flush = true; + this.callback(this.ctx, path, is_file, event_type); + } + } + + pub fn flush(this: *PathWatcher) void { + this.needs_flush = false; + this.flushCallback(this.ctx); + } + + pub fn deinit(this: *PathWatcher) void { + if (this.manager) |manager| { + manager.unregisterWatcher(this); + } + this.file_paths.deinitWithAllocator(bun.default_allocator); + + bun.default_allocator.destroy(this); + } +}; + +pub fn watch( + vm: *VirtualMachine, + path: [:0]const u8, + recursive: bool, + callback: PathWatcher.Callback, + updateEnd: PathWatcher.UpdateEndCallback, + ctx: ?*anyopaque, +) !*PathWatcher { + if (default_manager) |manager| { + const path_info = try manager._fdFromAbsolutePathZ(path); + errdefer manager._decrementPathRef(path); + return try PathWatcher.init(manager, path_info, recursive, callback, updateEnd, ctx); + } else { + default_manager_mutex.lock(); + defer default_manager_mutex.unlock(); + if (default_manager == null) { + default_manager = try PathWatcherManager.init(vm); + } + const manager = default_manager.?; + const path_info = try manager._fdFromAbsolutePathZ(path); + errdefer manager._decrementPathRef(path); + return try PathWatcher.init(manager, path_info, recursive, callback, updateEnd, ctx); + } +} diff --git a/src/js/node/os.js b/src/js/node/os.js index 2ff98beea1668..40250ef9af75c 100644 --- a/src/js/node/os.js +++ b/src/js/node/os.js @@ -14,10 +14,72 @@ export var tmpdir = function () { return tmpdir(); }; +// os.cpus() is super expensive +// Specifically: getting the CPU speed on Linux is very expensive +// Some packages like FastGlob only bother to read the length of the array +// so instead of actually populating the entire object +// we turn them into getters +function lazyCpus({ cpus }) { + return () => { + const array = new Array(navigator.hardwareConcurrency); + function populate() { + const results = cpus(); + const length = results.length; + array.length = length; + for (let i = 0; i < length; i++) { + array[i] = results[i]; + } + } + + for (let i = 0; i < array.length; i++) { + // This is technically still observable via + // Object.getOwnPropertyDescriptors(), but it should be okay. + const instance = { + get model() { + if (array[i] === instance) populate(); + return array[i].model; + }, + set model(value) { + if (array[i] === instance) populate(); + array[i].model = value; + }, + + get speed() { + if (array[i] === instance) populate(); + return array[i].speed; + }, + + set speed(value) { + if (array[i] === instance) populate(); + array[i].speed = value; + }, + + get times() { + if (array[i] === instance) populate(); + return array[i].times; + }, + set times(value) { + if (array[i] === instance) populate(); + array[i].times = value; + }, + + toJSON() { + if (array[i] === instance) populate(); + return array[i]; + }, + }; + + array[i] = instance; + } + + return array; + }; +} + function bound(obj) { return { arch: obj.arch.bind(obj), - cpus: obj.cpus.bind(obj), + cpus: lazyCpus(obj), endianness: obj.endianness.bind(obj), freemem: obj.freemem.bind(obj), getPriority: obj.getPriority.bind(obj), diff --git a/src/js/out/modules/node/os.js b/src/js/out/modules/node/os.js index cc457e06d5e08..005e4df667c0d 100644 --- a/src/js/out/modules/node/os.js +++ b/src/js/out/modules/node/os.js @@ -1,7 +1,58 @@ -var bound = function(obj) { +var lazyCpus = function({ cpus }) { + return () => { + const array = new Array(navigator.hardwareConcurrency); + function populate() { + const results = cpus(), length = results.length; + array.length = length; + for (let i = 0;i < length; i++) + array[i] = results[i]; + } + for (let i = 0;i < array.length; i++) { + const instance = { + get model() { + if (array[i] === instance) + populate(); + return array[i].model; + }, + set model(value) { + if (array[i] === instance) + populate(); + array[i].model = value; + }, + get speed() { + if (array[i] === instance) + populate(); + return array[i].speed; + }, + set speed(value) { + if (array[i] === instance) + populate(); + array[i].speed = value; + }, + get times() { + if (array[i] === instance) + populate(); + return array[i].times; + }, + set times(value) { + if (array[i] === instance) + populate(); + array[i].times = value; + }, + toJSON() { + if (array[i] === instance) + populate(); + return array[i]; + } + }; + array[i] = instance; + } + return array; + }; +}, bound = function(obj) { return { arch: obj.arch.bind(obj), - cpus: obj.cpus.bind(obj), + cpus: lazyCpus(obj), endianness: obj.endianness.bind(obj), freemem: obj.freemem.bind(obj), getPriority: obj.getPriority.bind(obj), diff --git a/src/lock.zig b/src/lock.zig index 3e07945c850c2..423e617adb599 100644 --- a/src/lock.zig +++ b/src/lock.zig @@ -116,6 +116,12 @@ pub const Lock = struct { pub inline fn unlock(this: *Lock) void { this.mutex.release(); } + + pub inline fn assertUnlocked(this: *Lock, comptime message: []const u8) void { + if (this.mutex.state.load(.Monotonic) != 0) { + @panic(message); + } + } }; pub fn spinCycle() void {} diff --git a/src/string_immutable.zig b/src/string_immutable.zig index c967544c74d04..fbfe9a3c4f1de 100644 --- a/src/string_immutable.zig +++ b/src/string_immutable.zig @@ -672,15 +672,7 @@ pub fn startsWith(self: string, str: string) bool { return false; } - var i: usize = 0; - while (i < str.len) { - if (str[i] != self[i]) { - return false; - } - i += 1; - } - - return true; + return eqlLong(self[0..str.len], str, false); } pub inline fn endsWith(self: string, str: string) bool { diff --git a/src/watcher.zig b/src/watcher.zig index aded993e8b7dd..9bc61b3af2337 100644 --- a/src/watcher.zig +++ b/src/watcher.zig @@ -382,6 +382,8 @@ pub fn NewWatcher(comptime ContextType: type) type { pub fn init(ctx: ContextType, fs: *Fs.FileSystem, allocator: std.mem.Allocator) !*Watcher { var watcher = try allocator.create(Watcher); + errdefer allocator.destroy(watcher); + if (!PlatformWatcher.isRunning()) { try PlatformWatcher.init(); } @@ -406,14 +408,16 @@ pub fn NewWatcher(comptime ContextType: type) type { } pub fn deinit(this: *Watcher, close_descriptors: bool) void { - this.mutex.lock(); - defer this.mutex.unlock(); - - this.close_descriptors = close_descriptors; if (this.watchloop_handle != null) { + this.mutex.lock(); + defer this.mutex.unlock(); + this.close_descriptors = close_descriptors; this.running = false; } else { - if (this.close_descriptors and this.running) { + // if the mutex is locked, then that's now a UAF. + this.mutex.assertUnlocked("Internal consistency error: watcher mutex is locked when it should not be."); + + if (close_descriptors and this.running) { const fds = this.watchlist.items(.fd); for (fds) |fd| { std.os.close(fd); @@ -454,7 +458,19 @@ pub fn NewWatcher(comptime ContextType: type) type { allocator.destroy(this); } + pub fn remove(this: *Watcher, hash: HashType) void { + this.mutex.lock(); + defer this.mutex.unlock(); + if (this.indexOf(hash)) |index| { + const fds = this.watchlist.items(.fd); + const fd = fds[index]; + std.os.close(fd); + this.watchlist.swapRemove(index); + } + } + var evict_list_i: WatchItemIndex = 0; + pub fn removeAtIndex(_: *Watcher, index: WatchItemIndex, hash: HashType, parents: []HashType, comptime kind: WatchItem.Kind) void { std.debug.assert(index != NoWatchItem); diff --git a/test/js/node/watch/fixtures/relative.js b/test/js/node/watch/fixtures/relative.js index 26e09da1a0365..692d809ec94a9 100644 --- a/test/js/node/watch/fixtures/relative.js +++ b/test/js/node/watch/fixtures/relative.js @@ -1,23 +1,28 @@ import fs from "fs"; -const watcher = fs.watch("relative.txt", { signal: AbortSignal.timeout(2000) }); +try { + const watcher = fs.watch("relative.txt", { signal: AbortSignal.timeout(2000) }); -watcher.on("change", function (event, filename) { - if (filename !== "relative.txt" && event !== "change") { - console.error("fail"); + watcher.on("change", function (event, filename) { + if (filename !== "relative.txt" && event !== "change") { + console.error("fail"); + clearInterval(interval); + watcher.close(); + process.exit(1); + } else { + clearInterval(interval); + watcher.close(); + } + }); + watcher.on("error", err => { clearInterval(interval); - watcher.close(); + console.error(err.message); process.exit(1); - } else { - clearInterval(interval); - watcher.close(); - } -}); -watcher.on("error", err => { - clearInterval(interval); + }); + + const interval = setInterval(() => { + fs.writeFileSync("relative.txt", "world"); + }, 10); +} catch (err) { console.error(err.message); process.exit(1); -}); - -const interval = setInterval(() => { - fs.writeFileSync("relative.txt", "world"); -}, 10); +} diff --git a/test/js/node/watch/fs.watch.test.ts b/test/js/node/watch/fs.watch.test.ts index aa7959bed5916..d228e5a228030 100644 --- a/test/js/node/watch/fs.watch.test.ts +++ b/test/js/node/watch/fs.watch.test.ts @@ -189,7 +189,7 @@ describe("fs.watch", () => { const interval = repeat(() => { fs.writeFileSync(filepath, "world"); }); - }); + }, 10000); test("should error on invalid path", done => { try { @@ -248,7 +248,7 @@ describe("fs.watch", () => { clearInterval(interval); watchers.forEach(watcher => watcher.close()); } - }); + }, 10000); test("should work with url", done => { const filepath = path.join(testDir, "url.txt"); @@ -373,6 +373,11 @@ describe("fs.watch", () => { }); expect(promise).resolves.toBe("change"); }); + + test("immediately closing works correctly", async () => { + for (let i = 0; i < 100; i++) fs.watch(testDir, { persistent: true }).close(); + for (let i = 0; i < 100; i++) fs.watch(testDir, { persistent: false }).close(); + }); }); describe("fs.promises.watch", () => {