stdio: skip exit-time tcsetattr unless Bun modified termios

src/bun.js/bindings/c-bindings.cpp

@@ -400,17 +400,45 @@ extern "C" void Bun__onExit();
 extern "C" int32_t bun_stdio_tty[3];
 #if !OS(WINDOWS)
 static termios termios_to_restore_later[3];
+// Whether Bun itself has modified the termios of each stdio fd during this
+// process's lifetime (e.g. via process.stdin.setRawMode). Used to decide
+// whether the exit-time termios restore should fire when Bun is acting as a
+// pipeline producer (stdout is a pipe). See #29592 — writing our startup
+// snapshot back to a shared /dev/pts device clobbers raw mode set on the
+// same device by a downstream consumer (less, fzf, fx, ...).
+//
+// `volatile sig_atomic_t` because bun_restore_stdio() reads this from signal
+// context (onExitSignal, SIGINT/SIGTERM) while Bun__ttySetMode() writes it
+// from normal execution; sig_atomic_t is the only integral type POSIX
+// guarantees can be accessed atomically across that boundary.
+extern "C" volatile sig_atomic_t bun_stdio_modified[3] = { 0, 0, 0 };
 #endif
 
 extern "C" void bun_restore_stdio()
 {
 
 #if !OS(WINDOWS)
 
+    // Only suppress the restore when Bun is a pipeline producer (stdout is a
+    // pipe, not a TTY) and it didn't touch termios itself. That's the #29592
+    // case — a downstream consumer on the same /dev/pts/* device has already
+    // taken over termios and writing our startup snapshot back clobbers it.
+    //
+    // For the interactive-wrapper case (stdout IS a TTY, e.g. `bun run vim`
+    // where the child put the terminal into raw mode and then crashed), we
+    // keep the unconditional restore so the shell prompt comes back cooked.
+    // Same for the crash-handler, --watch reload, and signal-death paths in
+    // run_command / bunx / lifecycle_script_runner — those are all cases
+    // where Bun is the foreground session owner and the startup snapshot is
+    // the state the user expects back.
+    const bool pipeline_producer = bun_stdio_tty[1] == 0;
+
     // restore stdio
     for (int32_t fd = 0; fd < 3; fd++) {
         if (!bun_stdio_tty[fd])
             continue;
+        if (pipeline_producer && !bun_stdio_modified[fd])
+            continue;
 
         sigset_t sa;
         int err;

src/bun.js/bindings/wtf-bindings.cpp

@@ -15,6 +15,7 @@
 #if !OS(WINDOWS)
 #include <stdatomic.h>
 
+#include <signal.h>
 #include <termios.h>
 static int orig_termios_fd = -1;
 static struct termios orig_termios;
@@ -99,6 +100,10 @@ static void uv__tty_make_raw(struct termios* tio)
 
 extern "C" void Bun__atexit(void (*func)(void));
 
+#if !OS(WINDOWS)
+extern "C" volatile sig_atomic_t bun_stdio_modified[3];
+#endif
+
 extern "C" int Bun__ttySetMode(int fd, int mode)
 {
 #if !OS(WINDOWS)
@@ -159,10 +164,27 @@ extern "C" int Bun__ttySetMode(int fd, int mode)
         break;
     }
 
+    // Mark the fd as modified *before* applying the change. If a
+    // SIGINT/SIGTERM lands between the device going raw and our bookkeeping
+    // catching up, bun_restore_stdio would otherwise read 0 and skip the
+    // restore — leaving the terminal in raw mode after exit. A spurious set
+    // when uv__tcsetattr fails is harmless: bun_restore_stdio then writes
+    // the cooked startup snapshot back to a still-cooked device (no-op,
+    // pre-PR behavior). Bounds-checked because the PTY master fd from
+    // Bun.Terminal calls through here too. See #29592.
+    //
+    // Marked on every transition, including setRawMode(true)→(false), so
+    // the signal-exit path (which runs only bun_restore_stdio, not the
+    // atexit uv_tty_reset_mode hook) still restores cooked mode on Ctrl-C.
+    if (fd >= 0 && fd < 3) {
+        bun_stdio_modified[fd] = 1;
+    }
+
     /* Apply changes after draining */
     rc = uv__tcsetattr(fd, TCSADRAIN, &tmp);
-    if (rc == 0)
+    if (rc == 0) {
         current_tty_mode = mode;
+    }
 
     return rc;
 #else

test/js/bun/terminal/terminal-spawn.test.ts

@@ -1,5 +1,7 @@
+import { dlopen, FFIType } from "bun:ffi";
 import { describe, expect, test } from "bun:test";
-import { bunEnv, bunExe, isWindows } from "harness";
+import { bunEnv, bunExe, isMusl, isWindows } from "harness";
+import fs from "node:fs";
 
 // Cross-platform Bun.Terminal + Bun.spawn integration tests that don't rely
 // on POSIX-only behaviour (termios echo, SIGWINCH, cat/echo binaries). The
@@ -205,4 +207,234 @@ describe("Bun.Terminal subprocess integration", () => {
       expect(t.closed).toBe(true);
     }
   });
+
+  // termios c_lflag bit layout is platform-specific. These match sys/termios.h:
+  // Linux uses the "System V" layout; Darwin/BSD share the "4.3BSD" layout.
+  const ICANON = process.platform === "darwin" ? 0x100 : 0x2;
+  const ECHO = 0x8; // same on both
+
+  // Regression: a Bun pipeline producer (stdout is a pipe, stdin/stderr are
+  // TTYs) that never calls setRawMode must not write its startup termios
+  // snapshot back to the terminal device at exit. The bug scenario is
+  // literally `bun foo.js | less`: termios is a property of the /dev/pts/*
+  // device, not the fd, so restoring here clobbers raw mode set on the same
+  // device by the downstream consumer. See #29592.
+  //
+  // openpty via bun:ffi so we can wire stdin/stderr to the slave but keep
+  // stdout as a pipe — exactly the `bun foo.js | less` shape that triggers
+  // the bug.
+  //
+  //   glibc: openpty in libutil.so.1, termios in libc.so.6.
+  //   musl:  everything in libc.musl-{x86_64,aarch64}.so.1.
+  //   macOS: everything in libc.dylib; tcflag_t is `unsigned long` (8 bytes
+  //          on LP64), so c_lflag sits at offset 24 instead of 12. The flag
+  //          bits all fit in the low u32 on both platforms, so reading a
+  //          u32 at the right offset round-trips cleanly.
+  test.skipIf(isWindows)("pipeline producer exit does not clobber raw mode on shared tty device", async () => {
+    const LFLAG_OFFSET = process.platform === "darwin" ? 24 : 12;
+
+    const openptyDecl = {
+      args: [FFIType.ptr, FFIType.ptr, FFIType.ptr, FFIType.ptr, FFIType.ptr],
+      returns: FFIType.i32,
+    } as const;
+    const termiosDecls = {
+      tcgetattr: { args: [FFIType.i32, FFIType.ptr], returns: FFIType.i32 },
+      tcsetattr: { args: [FFIType.i32, FFIType.i32, FFIType.ptr], returns: FFIType.i32 },
+      close: { args: [FFIType.i32], returns: FFIType.i32 },
+    } as const;
+
+    // Musl and macOS both keep openpty + termios in a single libc. Only
+    // glibc splits openpty out into libutil.
+    const lib =
+      process.platform === "darwin"
+        ? dlopen("libc.dylib", { openpty: openptyDecl, ...termiosDecls })
+        : isMusl
+          ? dlopen(process.arch === "arm64" ? "libc.musl-aarch64.so.1" : "libc.musl-x86_64.so.1", {
+              openpty: openptyDecl,
+              ...termiosDecls,
+            })
+          : dlopen("libutil.so.1", { openpty: openptyDecl });
+    const libc = process.platform === "darwin" || isMusl ? lib : dlopen("libc.so.6", termiosDecls);
+
+    const masterBuf = new Int32Array(1);
+    const slaveBuf = new Int32Array(1);
+    expect(lib.symbols.openpty(masterBuf, slaveBuf, null, null, null)).toBe(0);
+    const master = masterBuf[0];
+    const slave = slaveBuf[0];
+
+    // termios struct size:
+    //   Linux  = 60 (4× u32 flags + u8 c_line + 32 cc + 3 pad + 2× u32 speed)
+    //   Darwin = 72 (4× u64 flags + 20 cc + pad + 2× u64 speed)
+    // 128 is generous on both platforms.
+    const termiosBuf = new Uint8Array(128);
+
+    function getLflag(): number {
+      expect(libc.symbols.tcgetattr(master, termiosBuf)).toBe(0);
+      return new DataView(termiosBuf.buffer).getUint32(LFLAG_OFFSET, true);
+    }
+
+    function setLflag(value: number) {
+      expect(libc.symbols.tcgetattr(master, termiosBuf)).toBe(0);
+      new DataView(termiosBuf.buffer).setUint32(LFLAG_OFFSET, value, true);
+      expect(libc.symbols.tcsetattr(master, 0, termiosBuf)).toBe(0);
+    }
+
+    try {
+      // Assert the PTY starts cooked so the test can't pass vacuously.
+      expect(getLflag() & ICANON).not.toBe(0);
+      expect(getLflag() & ECHO).not.toBe(0);
+
+      const proc = Bun.spawn({
+        cmd: [
+          bunExe(),
+          "-e",
+          // Child: stdout is a pipe (pipeline producer), stdin/stderr are
+          // the PTY slave. Writes READY to stdout, then blocks on stdin
+          // until the parent flips termios and tells us to exit.
+          `process.stdout.write("READY\\n"); process.stdin.once("data", () => process.exit(0));`,
+        ],
+        env: bunEnv,
+        stdin: slave,
+        stdout: "pipe",
+        stderr: slave,
+      });
+
+      // Wait for READY so we know the child is up and has finished
+      // `bun_initialize_process` (the termios snapshot).
+      const decoder = new TextDecoder();
+      let buffer = "";
+      const reader = proc.stdout.getReader();
+      while (!buffer.includes("READY")) {
+        const { value, done } = await reader.read();
+        if (done) break;
+        buffer += decoder.decode(value, { stream: true });
+      }
+      reader.releaseLock();
+
+      // Simulate `less` flipping the shared device to raw mode.
+      setLflag(getLflag() & ~(ICANON | ECHO));
+      expect(getLflag() & ICANON).toBe(0);
+      expect(getLflag() & ECHO).toBe(0);
+
+      // Release the child.
+      fs.writeSync(master, "\n");
+      const exitCode = await proc.exited;
+
+      // Termios bits first: these are the regression.
+      expect(getLflag() & ICANON).toBe(0);
+      expect(getLflag() & ECHO).toBe(0);
+      expect(exitCode).toBe(0);
+    } finally {
+      libc.symbols.close(master);
+      libc.symbols.close(slave);
+    }
+  });
+
+  // Companion: an interactive-wrapper case (stdout IS a TTY, like `bun run
+  // vim` where the child may have taken termios raw and crashed) keeps the
+  // unconditional restore. That's the `bun_restore_stdio` branch the
+  // pipeline-producer gate does not apply to: after the child exits, the
+  // parent's startup snapshot is written back so the shell comes back cooked.
+  test.skipIf(isWindows)("interactive wrapper (stdout tty) restores cooked termios on child exit", async () => {
+    const ready = Promise.withResolvers<void>();
+    const decoder = new TextDecoder();
+    let buffer = "";
+    let sawReady = false;
+    await using terminal = new Bun.Terminal({
+      data(_, chunk: Uint8Array) {
+        if (sawReady) return;
+        buffer += decoder.decode(chunk, { stream: true });
+        if (buffer.includes("READY")) {
+          sawReady = true;
+          ready.resolve();
+        }
+      },
+    });
+
+    expect(terminal.localFlags & ICANON).not.toBe(0);
+    expect(terminal.localFlags & ECHO).not.toBe(0);
+
+    const proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        // Interactive wrapper: all three stdio are the PTY. The child
+        // itself does not call setRawMode — but we flip termios from the
+        // parent before the child exits, to simulate a TUI that set raw
+        // mode via FFI/ioctl rather than through Bun__ttySetMode.
+        `process.stdout.write("READY\\n"); process.stdin.once("data", () => process.exit(0));`,
+      ],
+      env: bunEnv,
+      terminal,
+    });
+
+    await ready.promise;
+
+    // Child took termios raw externally (simulated by the parent here).
+    terminal.localFlags = terminal.localFlags & ~(ICANON | ECHO);
+    expect(terminal.localFlags & ICANON).toBe(0);
+    expect(terminal.localFlags & ECHO).toBe(0);
+
+    terminal.write("\n");
+    const exitCode = await proc.exited;
+
+    // Because stdout is a TTY (interactive wrapper, not a pipeline
+    // producer), bun_restore_stdio keeps the unconditional restore and
+    // writes the cooked startup snapshot back. This matches the pre-PR
+    // safety net for `bun run <tui>` after the TUI crashes.
+    expect(terminal.localFlags & ICANON).not.toBe(0);
+    expect(terminal.localFlags & ECHO).not.toBe(0);
+    expect(exitCode).toBe(0);
+  });
+
+  // Companion to the regression test above: setRawMode still has its own
+  // restore path via uv_tty_reset_mode's atexit hook. A child that actually
+  // modifies termios must leave the device in its pre-setRawMode state.
+  //
+  // Handshake with the child across its entire lifetime so the assertions
+  // distinguish the three cases we care about:
+  //   1. child wrote raw → assert cooked before, raw while live, cooked after
+  //   2. setRawMode became a no-op → "raw while live" assertion fails
+  //   3. our bookkeeping skipped the restore → "cooked after" assertion fails
+  test.skipIf(isWindows)("child that called setRawMode restores termios on exit", async () => {
+    const raw = Promise.withResolvers<void>();
+    const decoder = new TextDecoder();
+    let buffer = "";
+    let sawRaw = false;
+    await using terminal = new Bun.Terminal({
+      data(_, chunk: Uint8Array) {
+        if (sawRaw) return;
+        buffer += decoder.decode(chunk, { stream: true });
+        if (buffer.includes("RAW")) {
+          sawRaw = true;
+          raw.resolve();
+        }
+      },
+    });
+
+    expect(terminal.localFlags & ICANON).not.toBe(0);
+    expect(terminal.localFlags & ECHO).not.toBe(0);
+
+    // Child enters raw mode, announces it, then blocks on stdin so the
+    // parent can observe termios state while the child is still alive.
+    const proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        `process.stdin.setRawMode(true); process.stdout.write("RAW\\n"); process.stdin.once("data", () => process.exit(0));`,
+      ],
+      env: bunEnv,
+      terminal,
+    });
+
+    await raw.promise;
+    expect(terminal.localFlags & ICANON).toBe(0);
+    expect(terminal.localFlags & ECHO).toBe(0);
+
+    terminal.write("\n");
+    const exitCode = await proc.exited;
+    expect(terminal.localFlags & ICANON).not.toBe(0);
+    expect(terminal.localFlags & ECHO).not.toBe(0);
+    expect(exitCode).toBe(0);
+  });
 });