Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions test/js/bun/module-graph/module-graph-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -968,8 +968,11 @@ describe.concurrent("ModuleGraph: an error in what a graph opened is the graph's
"fs.watch listener": boom => {
const dir = path.join(import.meta.dirname, "watched");
fs.mkdirSync(dir);
const w = fs.watch(dir, () => { w.close(); boom(); });
fs.writeFileSync(path.join(dir, "f"), "x");
const w = fs.watch(dir, () => { clearInterval(again); w.close(); boom(); });
// macOS starts watching on another thread after fs.watch() returns, and reports nothing written before that.
const write = () => fs.writeFileSync(path.join(dir, "f"), "x");
write();
const again = setInterval(write, 5);
Comment on lines +971 to +975

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) On any run where the watcher never reports, the fixture keeps rewriting watched/f every 5 ms through all ~20 later cases, so the failure report is polluted by unrelated load. The interval at test/js/bun/module-graph/module-graph-io.test.ts:975 is only cleared inside the watch callback at :971; the 10 s fallback at :1005 resolves the case but stops nothing. The sibling interval case at :1051-1052 exports stop() and main.mjs calls it at :1061; this case has no equivalent. Fix: stop the interval and close the watcher on every exit of the case, e.g. return a stop function the driver calls after told.promise settles, or clear it from the fallback path.

Why this was flagged

Trigger: a platform or build where fs.watch delivers no event for the directory within 10 s (the exact failure this PR is fixing on macOS). The case falls through to the fallback at test/js/bun/module-graph/module-graph-io.test.ts:1005 and the loop at :1010 moves on. Nothing clears again (:975) or closes w (:971), so a synchronous fs.writeFileSync runs every 5 ms from the graph's timer for the rest of the run, until process.exit(0) at :1024. On the base branch a never-reporting watcher left one file and one idle handle; after the diff it leaves a hot timer. On debug and ASAN lanes each write is 10-100x slower, so later timing-sensitive cases can also flip to 'nobody was told', hiding the real culprit. The file's own convention for an interval stimulus (:1048-1062) exports stop() and the driver calls it; REVIEW.md requires every timeout path to clear timers and release resources. Remedy: make the case return a cleanup the driver runs after the case settles, or clear the interval and close the watcher from the fallback.

Verification: Triggers only when the fs.watch callback never fires. The interval at test/js/bun/module-graph/module-graph-io.test.ts:975 is cleared only inside the watch callback at :971. The fallback at :1005 resolves the case and the loop at :1010 moves on with nothing stopping the interval or closing w. process.exit(0) at :1024 means the leaked interval cannot keep the process alive.

},
"a read stream's 'data'": boom => { fs.createReadStream(import.meta.filename).on("data", boom); },
"a zlib stream's 'data'": boom => { const z = zlib.createGzip(); z.on("data", boom); z.end("hello"); },
Expand Down
Loading