From bd04f484caa62cab1c6ef6e809fdf356b710cba0 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 29 Apr 2026 14:17:59 +0000 Subject: [PATCH 1/3] Watcher: flush evict_list in remove() when full to prevent overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Watcher.remove() appends the watchlist index to a fixed-size evict_list[8096]; the real removal is deferred to 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). Repeating that in a tight loop 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 PathWatcherManager.unregisterWatcher PathWatcher.deinit Drain evict_list inside remove() when it's full — this.mutex is already held there, matching how flushEvictions() is invoked from the platform watch loops. --- src/Watcher.zig | 11 +++ .../fs.watch.evict-list-overflow.test.ts | 78 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 test/js/node/watch/fs.watch.evict-list-overflow.test.ts diff --git a/src/Watcher.zig b/src/Watcher.zig index 3f4dc4e87f1..8f6520efced 100644 --- a/src/Watcher.zig +++ b/src/Watcher.zig @@ -759,6 +759,17 @@ pub fn remove(this: *Watcher, hash: HashType) void { this.mutex.lock(); defer this.mutex.unlock(); if (this.indexOf(hash)) |index| { + // `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. + if (this.evict_list_i >= max_eviction_count) { + this.flushEvictions(); + } 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..bc4b2b3df14 --- /dev/null +++ b/test/js/node/watch/fs.watch.evict-list-overflow.test.ts @@ -0,0 +1,78 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isMacOS, 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 directory watches +// use FSEvents. The overflow is reachable on Linux/FreeBSD only. +test.skipIf(isWindows || isMacOS)( + "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, +); From 05c8a869a1eabda77cd48683c2f390dabfd2d4e1 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 14:20:19 +0000 Subject: [PATCH 2/3] [autofix.ci] apply automated fixes --- test/js/node/watch/fs.watch.evict-list-overflow.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 index bc4b2b3df14..a932c44585a 100644 --- a/test/js/node/watch/fs.watch.evict-list-overflow.test.ts +++ b/test/js/node/watch/fs.watch.evict-list-overflow.test.ts @@ -57,11 +57,7 @@ test.skipIf(isWindows || isMacOS)( stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); const filteredStderr = stderr .split("\n") From ca9c48a0ddceea6967bbf41991fc1a2eda72d5a5 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 29 Apr 2026 16:00:18 +0000 Subject: [PATCH 3/3] address review: flush evict_list before indexOf() so the captured index isn't stale; enable test on macOS (file watches use KEventWatcher, not FSEvents) --- src/Watcher.zig | 24 ++++++++++--------- .../fs.watch.evict-list-overflow.test.ts | 10 ++++---- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/Watcher.zig b/src/Watcher.zig index 8f6520efced..ffab0b689c7 100644 --- a/src/Watcher.zig +++ b/src/Watcher.zig @@ -758,18 +758,20 @@ 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| { - // `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. - if (this.evict_list_i >= max_eviction_count) { - this.flushEvictions(); - } 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 index a932c44585a..58c70edf2d5 100644 --- a/test/js/node/watch/fs.watch.evict-list-overflow.test.ts +++ b/test/js/node/watch/fs.watch.evict-list-overflow.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isMacOS, isWindows, tempDir } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; // Regression test for `Watcher.evict_list` overflow. // @@ -30,9 +30,11 @@ import { bunEnv, bunExe, isMacOS, isWindows, tempDir } from "harness"; // 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 directory watches -// use FSEvents. The overflow is reachable on Linux/FreeBSD only. -test.skipIf(isWindows || isMacOS)( +// 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" });