diff --git a/src/Watcher.zig b/src/Watcher.zig index 3f4dc4e87f1..ffab0b689c7 100644 --- a/src/Watcher.zig +++ b/src/Watcher.zig @@ -758,6 +758,19 @@ pub fn indexOf(this: *Watcher, hash: HashType) ?u32 { pub fn remove(this: *Watcher, hash: HashType) void { this.mutex.lock(); defer this.mutex.unlock(); + // `removeAtIndex` only queues the index into `evict_list`; the real + // removal happens in `flushEvictions()`, which is normally driven by + // `onFileUpdate()` on the watcher thread. `remove()` can be called many + // times without any fs events firing (e.g. `fs.watch()` followed by + // `.close()` in a tight loop), so the fixed-size `evict_list` can fill + // up and overflow. Drain it here when full — we already hold + // `this.mutex`, matching how `flushEvictions()` is invoked from the + // platform watch loops. Must happen BEFORE `indexOf()`: flushing + // `swapRemove`s entries and relocates/shrinks the list, so an index + // captured beforehand could point at the wrong entry or past the end. + if (this.evict_list_i >= max_eviction_count) { + this.flushEvictions(); + } if (this.indexOf(hash)) |index| { this.removeAtIndex(@truncate(index), hash, &[_]HashType{}, .file); } diff --git a/test/js/node/watch/fs.watch.evict-list-overflow.test.ts b/test/js/node/watch/fs.watch.evict-list-overflow.test.ts new file mode 100644 index 00000000000..58c70edf2d5 --- /dev/null +++ b/test/js/node/watch/fs.watch.evict-list-overflow.test.ts @@ -0,0 +1,76 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; + +// Regression test for `Watcher.evict_list` overflow. +// +// `Watcher.remove()` appends the watchlist index to a fixed-size +// `evict_list[max_eviction_count = 8096]` buffer; the actual removal +// happens in `flushEvictions()`, which was only driven by `onFileUpdate()` +// on the watcher thread — i.e. only when a filesystem event arrives. +// +// `fs.watch(path).close()` reaches `_decrementPathRefNoLock()` → +// `main_watcher.remove(hash)` for the watched path. Repeating that +// without ever modifying the filesystem means `flushEvictions()` never +// runs, and once the cumulative remove count passes 8096, +// `removeAtIndex()` writes past the end of `evict_list`: +// +// panic(main thread): index out of bounds: index 8096, len 8096 +// Watcher.removeAtIndex src/Watcher.zig +// Watcher.remove +// PathWatcherManager._decrementPathRefNoLock +// +// The fix drains `evict_list` inside `remove()` when it's full (the +// mutex is already held there, matching how `flushEvictions()` is +// invoked from the platform watch loops). +// +// Watching a single file (not a directory) keeps the test deterministic: +// file watches don't schedule a `DirectoryRegisterTask`, so `deinit()` +// runs to completion on every `close()` and exactly one `remove()` is +// issued per iteration; and the inotify file mask doesn't include +// IN_OPEN / IN_CLOSE so re-opening the fd each cycle doesn't generate +// events that would opportunistically flush. +// +// Windows uses win_watcher.zig (no evict_list). macOS/FreeBSD file +// watches go through KEventWatcher and hit the same `Watcher.remove()` +// → `evict_list` path, so they're covered too — only directory watches +// on macOS bypass it via FSEvents. +test.skipIf(isWindows)( + "Watcher.remove() does not overflow evict_list when no fs events fire", + async () => { + using dir = tempDir("fswatch-evict-overflow", { "f.txt": "x" }); + + const fixture = /* js */ ` + const fs = require("fs"); + const path = require("path"); + const target = path.join(process.argv[1], "f.txt"); + // > max_eviction_count (8096): one remove() per cycle, no fs events, + // so without the fix evict_list_i hits 8096 and removeAtIndex panics. + const ITERS = 8200; + for (let i = 0; i < ITERS; i++) { + const w = fs.watch(target, { persistent: false }, () => {}); + w.close(); + } + console.log("ok " + ITERS); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture, String(dir)], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const filteredStderr = stderr + .split("\n") + .filter(l => l && !l.startsWith("WARNING: ASAN interferes")) + .join("\n"); + + expect({ stdout: stdout.trim(), stderr: filteredStderr, exitCode }).toEqual({ + stdout: "ok 8200", + stderr: "", + exitCode: 0, + }); + }, + 30000, +);