Bun.ModuleGraph: dispose() closes the Bun.Terminal terminals the graph opened

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`

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<bun_core::tty::State>,
+
+    /// 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<JSValue>,
@@ -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::<Terminal>(),
         }));
         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<CreateResult, InitError> {
-        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<JSValue> {
-        // 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

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 {

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 <div> 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 () => {

test/js/bun/module-graph/module-graph-io.test.ts

@@ -252,6 +252,37 @@ 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({});
+    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.
+      late = inGraph(() => app.open()) as Bun.Terminal;
+      await until(() => late!.closed);
+      await until(() => !isRunning(app.child.pid));
+      expect(app.heard).toEqual([]);
+    } finally {
+      app.child.kill("SIGKILL");
+      late?.close();
+    }
+  });
+
   test("dispose() terminates the workers the graph started and closes its ports", async () => {
     const dir = fixture({
       "spawner.mjs": `