Fix terminal mode interference when piping to less

Fixes #22785

The issue was that Bun was saving and restoring terminal settings for
all TTY file descriptors at process initialization and exit, even when
it never modified them. This could interfere with programs like 'less'
that need to control the terminal when receiving piped input.

The fix follows libuv's approach:
- Only save terminal settings when we actually modify them via Bun__ttySetMode
- Don't preemptively save terminal settings at initialization
- Only restore settings for file descriptors we actually modified

This prevents Bun from interfering with terminal control when stdout
is piped to pagers like 'less'.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2025-09-19 04:28:28 +00:00
parent 2eebcee522
commit ca76f01bb5
3 changed files with 144 additions and 21 deletions

View File

@@ -382,7 +382,9 @@ extern "C" ssize_t pwritev2(int fd, const struct iovec* iov, int iovcnt,
extern "C" void Bun__onExit();
extern "C" int32_t bun_stdio_tty[3];
#if !OS(WINDOWS)
static termios termios_to_restore_later[3];
termios termios_to_restore_later[3];
// Track which terminal settings were actually saved
bool termios_was_saved[3] = { false, false, false };
#endif
extern "C" void bun_restore_stdio()
@@ -392,7 +394,7 @@ extern "C" void bun_restore_stdio()
// restore stdio
for (int32_t fd = 0; fd < 3; fd++) {
if (!bun_stdio_tty[fd])
if (!bun_stdio_tty[fd] || !termios_was_saved[fd])
continue;
sigset_t sa;
@@ -459,7 +461,7 @@ extern "C" void bun_initialize_process()
#if OS(LINUX) || OS(DARWIN)
int devNullFd_ = -1;
bool anyTTYs = false;
// bool anyTTYs = false; // No longer needed since we don't set up signal handlers here
const auto setDevNullFd = [&](int target_fd) -> void {
bun_is_stdio_null[target_fd] = 1;
@@ -494,15 +496,10 @@ extern "C" void bun_initialize_process()
}
} else {
bun_stdio_tty[fd] = 1;
int err = 0;
do {
err = tcgetattr(fd, &termios_to_restore_later[fd]);
} while (err == -1 && errno == EINTR);
if (err == 0) [[likely]] {
anyTTYs = true;
}
// Following libuv's approach: DO NOT save any terminal settings at initialization
// Only save them when/if we actually modify them via Bun__ttySetMode
// This prevents interfering with programs like 'less' that need terminal control
}
}
@@ -511,18 +508,21 @@ extern "C" void bun_initialize_process()
close(devNullFd_);
}
// Restore TTY state on exit
if (anyTTYs) {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
// Only restore TTY state on exit if we're actually modifying terminal settings
// Don't set up signal handlers just because stdout/stderr are TTYs, as this
// can interfere with programs like 'less' that control stdin when our output is piped
// Signal handlers will be set up by Bun__ttySetMode if/when terminal modes are actually changed
// if (anyTTYs) {
// struct sigaction sa;
// memset(&sa, 0, sizeof(sa));
// sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESETHAND;
sa.sa_handler = onExitSignal;
// sa.sa_flags = SA_RESETHAND;
// sa.sa_handler = onExitSignal;
sigaction(SIGTERM, &sa, nullptr);
sigaction(SIGINT, &sa, nullptr);
}
// sigaction(SIGTERM, &sa, nullptr);
// sigaction(SIGINT, &sa, nullptr);
// }
#elif OS(WINDOWS)
for (int fd = 0; fd <= 2; ++fd) {
auto handle = reinterpret_cast<HANDLE>(uv_get_osfhandle(fd));

View File

@@ -23,6 +23,15 @@ static std::once_flag reset_once_flag;
static int current_tty_mode = 0;
static struct termios orig_tty_termios;
// External declarations for terminal restoration
extern "C" int32_t bun_stdio_tty[3];
extern struct termios termios_to_restore_later[3];
extern bool termios_was_saved[3];
#include <signal.h>
extern "C" void onExitSignal(int sig);
static std::once_flag signal_handler_once_flag;
int uv__tcsetattr(int fd, int how, const struct termios* term)
{
int rc;
@@ -124,6 +133,13 @@ extern "C" int Bun__ttySetMode(int fd, int mode)
if (orig_termios_fd == -1) {
orig_termios = orig_tty_termios;
orig_termios_fd = fd;
// Save the terminal settings for restoration later
// This follows libuv's approach - only save when we actually modify
if (fd >= 0 && fd <= 2 && bun_stdio_tty[fd]) {
termios_to_restore_later[fd] = orig_tty_termios;
termios_was_saved[fd] = true;
}
}
atomic_store(&orig_termios_spinlock, 0);
@@ -146,6 +162,17 @@ extern "C" int Bun__ttySetMode(int fd, int mode)
uv_tty_reset_mode();
});
});
// Set up signal handlers when we actually modify terminal settings
std::call_once(signal_handler_once_flag, [] {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESETHAND;
sa.sa_handler = onExitSignal;
sigaction(SIGTERM, &sa, nullptr);
sigaction(SIGINT, &sa, nullptr);
});
break;
case 2: // io
uv__tty_make_raw(&tmp);
@@ -155,6 +182,17 @@ extern "C" int Bun__ttySetMode(int fd, int mode)
uv_tty_reset_mode();
});
});
// Set up signal handlers when we actually modify terminal settings
std::call_once(signal_handler_once_flag, [] {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESETHAND;
sa.sa_handler = onExitSignal;
sigaction(SIGTERM, &sa, nullptr);
sigaction(SIGINT, &sa, nullptr);
});
break;
}

View File

@@ -0,0 +1,85 @@
import { test, expect } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { mkdirSync, writeFileSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";
// Test for GitHub issue #22785
// `bun script.js | less` sometimes causes terminal input to `less` to be line-buffered
test.skip("piping to less should not interfere with terminal mode", async () => {
// This test is skipped by default because:
// 1. It requires the 'less' command to be installed
// 2. It requires an interactive terminal
// 3. The issue is intermittent (~10% occurrence rate)
//
// To run this test manually:
// bun test test/regression/issue/22785-less-terminal-mode.test.ts
const dir = tmpdir();
const scriptPath = join(dir, "test-script.js");
// Create a script that outputs many lines
const scriptContent = `
for (let i = 0; i < 1000; i++) {
process.stdout.write(\`\${i}\\n\`);
}
`;
writeFileSync(scriptPath, scriptContent);
// Test multiple times since the issue is intermittent
const attempts = 50;
let failureCount = 0;
for (let i = 0; i < attempts; i++) {
// Run bun piped to less and send 'q' to quit
// If the bug occurs, less will require Enter after 'q'
const proc = Bun.spawn({
cmd: ["sh", "-c", `echo q | ${bunExe()} ${scriptPath} | less`],
env: { ...bunEnv, TERM: "xterm" },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr] = await Promise.all([
proc.stdout.text(),
proc.stderr.text(),
]);
// If 'q' appears in stdout, it means less didn't quit immediately
// and displayed the 'q' character (bug occurred)
if (stdout.includes("q")) {
failureCount++;
}
await proc.exited;
}
// The bug should not occur with our fix
expect(failureCount).toBe(0);
});
// Test that stderr being redirected prevents the issue (as reported)
test.skip("piping to less with stderr redirected should work", async () => {
const dir = tmpdir();
const scriptPath = join(dir, "test-script.js");
const scriptContent = `
for (let i = 0; i < 100; i++) {
process.stdout.write(\`\${i}\\n\`);
}
`;
writeFileSync(scriptPath, scriptContent);
// With stderr redirected, the issue should never occur
const proc = Bun.spawn({
cmd: ["sh", "-c", `echo q | ${bunExe()} ${scriptPath} 2>/dev/null | less`],
env: { ...bunEnv, TERM: "xterm" },
stdout: "pipe",
});
const stdout = await proc.stdout.text();
await proc.exited;
// 'q' should not appear in output
expect(stdout).not.toContain("q");
});