From 432dbc788d384fed9c74556a24a482349e280ae4 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Thu, 15 Oct 2026 01:38:49 +0000 Subject: [PATCH 1/2] Bun.ModuleGraph: dispose() closes the Bun.Terminal terminals the graph opened --- docs/runtime/module-graph.mdx | 2 +- src/runtime/api/bun/Terminal.rs | 40 +++++++++++++------ src/runtime/api/bun/js_bun_spawn_bindings.rs | 2 +- .../bun/module-graph/module-graph-gc.test.ts | 17 ++++++++ .../bun/module-graph/module-graph-io.test.ts | 30 ++++++++++++++ 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/docs/runtime/module-graph.mdx b/docs/runtime/module-graph.mdx index e3d626d2d6b2..63dd69071677 100644 --- a/docs/runtime/module-graph.mdx +++ b/docs/runtime/module-graph.mdx @@ -31,7 +31,7 @@ A graph has a context of its own for timers and I/O. Everything its code opens b - timers - `Bun.serve` and `Bun.listen` servers, sockets, `fetch()` requests, WebSockets - watchers -- child processes, including `Bun.$` +- child processes, including `Bun.$`, and `Bun.Terminal` terminals - workers - database connections - files opened through `Bun.file().writer()`, `bun:sqlite` and `node:sqlite` diff --git a/src/runtime/api/bun/Terminal.rs b/src/runtime/api/bun/Terminal.rs index b25f4cdc13ee..28128575cb63 100644 --- a/src/runtime/api/bun/Terminal.rs +++ b/src/runtime/api/bun/Terminal.rs @@ -159,8 +159,16 @@ pub(crate) struct Terminal { /// going raw never makes another terminal's setRawMode a no-op. #[cfg(unix)] tty_state: Cell, + + /// Armed while open, for a terminal that script of a `Bun.ModuleGraph` opened: it closes with the graph. + abort_handle: jsc::AbortHandle, } +jsc::impl_abort_handle_owner!(Terminal, abort_handle, |this, _cause| { + // SAFETY: trait contract — `this` is live; the wrapper's ref outlasts the close. + unsafe { (*this).close_without_callbacks() } +}); + bitflags::bitflags! { #[derive(Clone, Copy, Default)] pub struct Flags: u8 { @@ -404,6 +412,7 @@ impl Terminal { /// Internal initialization - shared by constructor and createFromSpawn fn init_terminal( global_object: &JSGlobalObject, + context: &jsc::ScriptExecutionContext, options: &Options, // If provided, use this JSValue; otherwise create one via toJS existing_js_value: Option, @@ -441,6 +450,7 @@ impl Terminal { writer_has_buffered: Cell::new(false), #[cfg(unix)] tty_state: Cell::new(bun_core::tty::State::new()), + abort_handle: jsc::AbortHandle::for_owner::(), })); let parent_ptr = terminal; // SAFETY: just allocated, non-null, exclusively owned here. R-2: `&` @@ -540,6 +550,11 @@ impl Terminal { js::gc::set(js::GcValue::Drain, this_value, global_object, cb); } + if let Some(context) = global_object.bun_vm().as_graph_context(context) { + // SAFETY: heap-allocated above; leaves its context in `close_internal`. + unsafe { jsc::AbortHandle::arm_owner(parent_ptr, context) }; + } + Ok(CreateResult { // SAFETY: `parent_ptr` is the live `heap::into_raw` pointer above. terminal: unsafe { bun_ptr::BackRef::from_raw_mut(parent_ptr) }, @@ -579,7 +594,8 @@ impl Terminal { let options = Options::parse_from_js(global_object, js_options)?; - match Self::init_terminal(global_object, &options, Some(this_value)) { + let context = global_object.bun_vm().context_of_caller(callframe); + match Self::init_terminal(global_object, context, &options, Some(this_value)) { Ok(result) => { // Hand the intrusive ref to the JS wrapper as m_ctx; finalize() // releases it via deref_(). @@ -607,10 +623,10 @@ impl Terminal { /// Returns the Terminal and its JS wrapper value /// The slave_fd should be used for the subprocess's stdin/stdout/stderr pub(crate) fn create_from_spawn( - global_object: &JSGlobalObject, + cx: &jsc::JsThread<'_>, options: &Options, ) -> Result { - Self::init_terminal(global_object, options, None) + Self::init_terminal(cx.global(), cx.context(), options, None) } /// Get the slave fd for subprocess to use @@ -633,10 +649,13 @@ impl Terminal { } /// Spawn-side error-path teardown for a terminal created via - /// `create_from_spawn` whose subprocess never started. Downgrades the - /// JSRef so the wrapper is GC-eligible, marks `finalized` so - /// `on_reader_done` skips the JS exit callback, and runs `close_internal`. + /// `create_from_spawn` whose subprocess never started. pub(crate) fn abandon_from_spawn(&self) { + self.close_without_callbacks(); + } + + /// No callback follows (on Windows the reader drains on after `close_internal`), and the wrapper may be collected. + fn close_without_callbacks(&self) { self.this_value.with_mut(|v| v.downgrade()); self.update_flags(|f| f.insert(Flags::FINALIZED)); self.close_internal(); @@ -1695,13 +1714,7 @@ impl Terminal { global_object: &JSGlobalObject, _f: &CallFrame, ) -> JsResult { - // After dispose the caller must not see further data/exit callbacks. - // closeInternal on Windows leaves the reader draining off-thread, so - // suppress callbacks and downgrade the JSRef so the wrapper is - // GC-eligible once the caller's reference is dropped. - self.this_value.with_mut(|v| v.downgrade()); - self.update_flags(|f| f.insert(Flags::FINALIZED)); - self.close_internal(); + self.close_without_callbacks(); Ok(jsc::JSPromise::resolved_promise_value( global_object, JSValue::UNDEFINED, @@ -1713,6 +1726,7 @@ impl Terminal { return; } self.update_flags(|f| f.insert(Flags::CLOSED)); + self.abort_handle.leave(); // Close writer (closes write_fd). R-2: `with_mut` borrow is held across // the synchronous `on_writer_close` parent callback, but that callback diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 6c504debb352..93b311f0c4cf 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -834,7 +834,7 @@ fn spawn_maybe_sync( // Create a new terminal from options let term_options = TerminalOptions::parse_from_js(cx.global(), terminal_val)?; - match Terminal::create_from_spawn(cx.global(), &term_options) { + match Terminal::create_from_spawn(cx, &term_options) { Ok(created) => terminal_info.0 = Some(created), Err(err) => { return Err(match err { diff --git a/test/js/bun/module-graph/module-graph-gc.test.ts b/test/js/bun/module-graph/module-graph-gc.test.ts index 411ff07c8cb2..6a7e4bf70121 100644 --- a/test/js/bun/module-graph/module-graph-gc.test.ts +++ b/test/js/bun/module-graph/module-graph-gc.test.ts @@ -68,6 +68,10 @@ const dir = String( const w = new Worker("data:text/javascript,setInterval(() => {}, 1000); postMessage('up')"); return new Promise(resolve => { w.onmessage = () => resolve(); w.addEventListener("close", () => control.heard.push("worker close")); }); } + // An open terminal is a GC root, and its callbacks are the graph's. + export function openTerminal() { + new Bun.Terminal({ data() { control.heard.push("terminal data"); }, exit() { control.heard.push("terminal exit"); } }); + } // A rewrite whose
is never closed, with an onEndTag() callback that reaches the output Response. export function rewriteThatWaitsForAnEndTag() { let controller; @@ -544,6 +548,19 @@ describe("ModuleGraph GC: what the graph's context owns", () => { expect(await lifetimes.stillAlive("graph")).toEqual([]); }); + test("a Bun.Terminal that is open when its graph is disposed goes with the graph", async () => { + const lifetimes = new Lifetimes(); + const state = control(); + await (async () => { + const graph = lifetimes.track("graph", new ModuleGraph({ globals: { control: state } })); + const io = await graph.import(file("io.mjs")); + graph.run(() => io.openTerminal()); + graph.dispose(); + })(); + expect(await lifetimes.stillAlive("graph")).toEqual([]); + expect(state.heard).toEqual([]); + }); + // The tick's promise is the host's to settle: the job is still there when it does, and gone afterwards. for (const how of ["resolves", "rejects"] as const) { test(`a Bun.cron() job whose tick waits on a promise the host ${how} after the graph was disposed: the job outlives the wait, and the graph goes`, async () => { diff --git a/test/js/bun/module-graph/module-graph-io.test.ts b/test/js/bun/module-graph/module-graph-io.test.ts index 4904ab5dcd3a..1cc8d5f1ec9a 100644 --- a/test/js/bun/module-graph/module-graph-io.test.ts +++ b/test/js/bun/module-graph/module-graph-io.test.ts @@ -252,6 +252,36 @@ describe.concurrent("ModuleGraph: what a graph opens is the graph's", () => { } }); + test("dispose() closes the terminals the graph opened, not the host's, and calls none of their callbacks", async () => { + const dir = fixture({ + "terminal.mjs": ` + export const heard = []; + const callbacks = name => ({ data() { heard.push(name + " data"); }, exit() { heard.push(name + " exit"); } }); + export const open = () => new Bun.Terminal(callbacks("constructed")); + export const constructed = open(); + export const child = Bun.spawn({ cmd: [process.execPath, "-e", "setInterval(() => {}, 1000)"], terminal: callbacks("inline") }); + `, + }); + await using hosts = new Bun.Terminal({}); + const graph = new Bun.ModuleGraph(); + const app = await graph.import(join(dir, "terminal.mjs")); + const inGraph = graph.run(() => AsyncLocalStorage.snapshot()); + try { + expect([app.constructed.closed, app.child.terminal.closed]).toEqual([false, false]); + app.heard.length = 0; + graph.dispose(); + expect([app.constructed.closed, app.child.terminal.closed, hosts.closed]).toEqual([true, true, false]); + // What the graph's leftover code opens is closed on the next turn of the loop. + const late = inGraph(() => app.open()); + await until(() => late.closed); + await until(() => !isRunning(app.child.pid)); + await hostTimerTurns(); + expect(app.heard).toEqual([]); + } finally { + app.child.kill("SIGKILL"); + } + }); + test("dispose() terminates the workers the graph started and closes its ports", async () => { const dir = fixture({ "spawner.mjs": ` From 0b11a7805d16395f67e84aea7eec09231759d4fc Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Thu, 15 Oct 2026 02:01:17 +0000 Subject: [PATCH 2/2] test: the terminal case disposes its graph on every exit and waits only for conditions --- test/js/bun/module-graph/module-graph-io.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/js/bun/module-graph/module-graph-io.test.ts b/test/js/bun/module-graph/module-graph-io.test.ts index 1cc8d5f1ec9a..e566275dbee2 100644 --- a/test/js/bun/module-graph/module-graph-io.test.ts +++ b/test/js/bun/module-graph/module-graph-io.test.ts @@ -263,22 +263,23 @@ describe.concurrent("ModuleGraph: what a graph opens is the graph's", () => { `, }); await using hosts = new Bun.Terminal({}); - const graph = new Bun.ModuleGraph(); + using graph = new Bun.ModuleGraph(); const app = await graph.import(join(dir, "terminal.mjs")); const inGraph = graph.run(() => AsyncLocalStorage.snapshot()); + let late: Bun.Terminal | undefined; try { expect([app.constructed.closed, app.child.terminal.closed]).toEqual([false, false]); app.heard.length = 0; graph.dispose(); expect([app.constructed.closed, app.child.terminal.closed, hosts.closed]).toEqual([true, true, false]); // What the graph's leftover code opens is closed on the next turn of the loop. - const late = inGraph(() => app.open()); - await until(() => late.closed); + late = inGraph(() => app.open()) as Bun.Terminal; + await until(() => late!.closed); await until(() => !isRunning(app.child.pid)); - await hostTimerTurns(); expect(app.heard).toEqual([]); } finally { app.child.kill("SIGKILL"); + late?.close(); } });