fs.watch: stop dispatching watcher events into a terminated Worker VM

[robobun]

What

Terminating a worker_threads Worker while an fs.watch() created inside that worker has pending inotify events dispatched the queued watcher callbacks into the terminated VM. Debug/ASAN builds abort on the JSC entry assert:

ASSERTION FAILED: !exception()
JavaScriptCore/ExceptionScope.h(61) : void JSC::ExceptionScope::assertNoException()
  <- JSC::Interpreter::executeCallImpl
  <- JSC::call
  <- FSWatcher event dispatch

Release builds have the assert compiled out but are in the same state: re-entering JS in a VM whose sticky TerminationException is pending.

Repro

import * as fs from "node:fs";
import { Worker } from "node:worker_threads";
const d = `/tmp/fsw-wterm-${process.pid}`;
fs.mkdirSync(d, { recursive: true });
const workerSrc = `
  const fs = require("node:fs");
  const { parentPort, workerData } = require("node:worker_threads");
  const w = fs.watch(workerData.d, () => {});
  w.on("error", () => {});
  parentPort.postMessage("READY");
`;
for (let i = 0; i < 20; i++) {
  const wk = new Worker(workerSrc, { eval: true, workerData: { d } });
  await new Promise(res => wk.on("message", res));
  for (let j = 0; j < 50; j++) fs.writeFileSync(`${d}/b${j}`, "x");
  await wk.terminate();
}

Node v26 runs clean; bun debug aborts on the 1st/2nd iteration.

Cause

FSWatchTaskPosix::run() loops up to 8 batched entries and calls FSWatcher::emit → listener.call_with_global_this(...) with no check of the owning VM's script-execution status. Two failure modes:

1. A task already queued when terminate() lands is drained afterwards and every emit re-enters a stopped VM.
2. terminate() fires mid-batch: one emit throws the TerminationException, report_active_exception_as_unhandled does not clear it (it is sticky), and the next loop iteration re-enters JS with it still pending.

FSWatchTaskWindows::run() and FSWatcher::close() (which emits 'close' after emit_error/emit_abort) have the same gap.

Fix

Guard each listener emit with vm.script_execution_status() == Running, the same predicate StatWatcherScheduler::timer_callback, CronJob::on_timer_fire and TimerObjectInternals::fire use. The posix task re-checks on every iteration so a mid-batch termination stops the loop; unref_task() still runs on the skipped path so the pending-activity count stays balanced.

Verification

New test in test/js/node/watch/fs.watch.test.ts spawns a subprocess that runs the repro loop; on an unfixed debug build it aborts with ASSERTION FAILED: !exception() (3/3), with the fix it passes (5/5).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d6bdf91-328e-482f-81b2-8685a6f90c98

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5d816 and eebd9dd.

📒 Files selected for processing (2)

• src/runtime/node/node_fs_watcher.rs
• test/js/node/watch/fs.watch.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 9:05 AM PT - Jul 7th, 2026}

❌ @robobun, your commit eebd9dd has some failures in Build #69840 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33642

That installs a local version of the PR into your bun-33642 executable, so you can run:

bun-33642 --bun

[claude]

I didn't find any issues — the guard follows the exact script_execution_status() == Running pattern already used in StatWatcherScheduler, TimerObjectInternals, and CronJob, and the pending-activity ref-count stays balanced on every skipped path — but since this touches worker VM termination and JSC re-entry it's worth a quick human look.

Extended reasoning...

Overview

Adds a can_call_into_js() helper on FSWatcher (wrapping vm.script_execution_status() == Running) and guards three JS-emit sites with it: the per-entry loop in FSWatchTaskPosix::run(), the dispatch in FSWatchTaskWindows::run(), and the 'close' emit inside FSWatcher::close(). Adds a subprocess regression test that spins up workers with an fs.watch, floods the watched dir, and terminate()s mid-dispatch.

Security risks

None. The change is purely additive gating that prevents re-entering JS in a terminated VM; no new inputs are parsed and no trust boundaries move.

Level of scrutiny

Medium-high. The diff itself is small and mechanical, but it sits in native code that intersects worker VM lifecycle, JSC re-entry under a sticky TerminationException, and pending_activity_count bookkeeping (which controls GC rooting via hasPendingActivity). I traced each guarded site:

• Posix run(): break exits the batch loop early; the single trailing unref_task() still runs, balancing the single ref_task() from enqueue().
• Windows run(): only the match is wrapped; ctx.unref_task() remains outside the if and always runs.
• close(): the manual fetch_add(1) and its paired unref_task() are both inside the new guard, so skipping is net-zero; the final unref_task() (dropping the initial ref) still runs, so pending_activity_count reaches 0 and the wrapper stays collectable.

The predicate is identical to what node_fs_stat_watcher.rs:344, timer_object_internals.rs:487, cron.rs:1662/1693/1748, WindowsNamedPipe.rs:599, and UpgradedDuplex.rs:238 already use for the same purpose.

Other factors

The test follows repo conventions well (tempDir, subprocess isolation, concurrent pipe drain, combined-object assertion including signalCode, commented ASAN_OPTIONS override citing the pre-existing #32071 leak). The bug-hunting system found nothing. Deferring only because native VM-termination paths are the sort of thing a maintainer should eyeball, not because I see a problem.

[claude]

I didn't find any issues — the guard matches the established script_execution_status() == Running pattern used in node_fs_stat_watcher.rs, cron.rs, and timer_object_internals.rs, and the pending-activity refcount stays balanced on the skipped paths — but this is native VM-lifecycle / worker-termination code, so worth a quick human look.

Extended reasoning...

Overview

This PR guards FSWatcher listener emits with vm.script_execution_status() == Running so queued watcher events aren't dispatched into a Worker VM that has been terminate()d. Three sites are guarded: the Posix batched-entry loop in FSWatchTaskPosix::run() (re-checked per iteration so a mid-batch termination stops the loop), FSWatchTaskWindows::run(), and the 'close' emit inside FSWatcher::close(). A subprocess test in test/js/node/watch/fs.watch.test.ts reproduces the crash loop.

Security risks

None. This only adds a defensive check before calling into JS; no new inputs are parsed, no auth/crypto/permissions code is touched.

Level of scrutiny

Medium-high. The diff itself is small and follows an established idiom verbatim — the same predicate appears in node_fs_stat_watcher.rs:344, timer_object_internals.rs:487, cron.rs, UpgradedDuplex.rs, WindowsNamedPipe.rs, and JSNodeHTTPServerSocket.cpp. But it sits at the intersection of worker termination, JSC's sticky TerminationException, cross-thread task dispatch, and pending_activity_count refcount balance, which is exactly the class of code where a missed path causes a UAF or a permanently-pinned wrapper.

Other factors

I traced the refcount balance on the new skip paths: in FSWatchTaskPosix::run() the break falls through to unref_task(), and deinit() → clean_entries() still frees all 0..count entries afterward (the loop doesn't mutate count), so no leak from the early exit. In FSWatchTaskWindows::run() the unref_task() is outside the new if and still runs. In close(), the manual fetch_add(1) and its paired unref_task() are both inside the guarded block, and the outer unref_task() for the initial ref still runs unconditionally. The test follows harness conventions (tempDir, concurrent pipe drain, combined {stdout, stderr, exitCode, signalCode} assertion) and documents the detect_leaks=0 ASAN suppression as covering a pre-existing tracked leak (#32071). No prior reviews from me on this PR; no outstanding human reviewer comments.

[robobun]

CI status: the diff is green on every lane that actually ran tests. The only hard failure on both builds (#69828 and #69840) is darwin 26 aarch64 - test-bun dying on

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'

before any tests execute. The same lane is red on ~every concurrent branch build (69810, 69812, 69816, 69820, 69821, 69823, 69827, 69831, 69832, 69834, 69836, 69838), so it is an agent/infra issue unrelated to this change. The remaining annotated failures are pre-existing Windows flakes (bun-install.test.ts, napi.test.ts, hot.test.ts, dev-and-prod.test.ts) that do not touch fs.watch or workers.

Ready for review.