From c61847da5e1c01ecfd46136c1e5f38b1217fbd3a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 15 Oct 2026 11:43:22 +0200 Subject: [PATCH] src: restore tty state on exit only if changed ResetStdio() unconditionally writes the terminal attributes recorded at startup back to stdin and stderr when the process exits, even when the process never touched the terminal. This clobbers changes made in the meantime by another process sharing the terminal: in `node -h | less`, less switches the terminal to raw mode once it receives the first byte of output, and node switches it back to cooked mode when it exits. Only restore the recorded attributes when node itself changed the terminal mode through setRawMode(), or when exiting because of a signal. Restoring the O_NONBLOCK flag and libuv's own terminal mode reset are unchanged. Fixes: https://github.com/nodejs/node/issues/66440 Refs: https://github.com/nodejs/node/issues/35536 Signed-off-by: Matteo Collina --- src/node.cc | 27 +++++++++++++---- src/node_internals.h | 9 +++++- src/node_watchdog.cc | 2 +- src/tty_wrap.cc | 2 ++ .../test-stdio-exit-keeps-foreign-tty-mode.js | 30 +++++++++++++++++++ ...test-stdio-exit-keeps-foreign-tty-mode.out | 0 6 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.js create mode 100644 test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.out diff --git a/src/node.cc b/src/node.cc index d0b5310626c3..f6e4f8ba7749 100644 --- a/src/node.cc +++ b/src/node.cc @@ -189,7 +189,7 @@ const char* conf_section_name = STRINGIFY(NODE_OPENSSL_CONF_NAME); #ifdef __POSIX__ void SignalExit(int signo, siginfo_t* info, void* ucontext) { - ResetStdio(); + ResetStdio(true); raise(signo); } #endif // __POSIX__ @@ -450,7 +450,7 @@ void TrapWebAssemblyOrContinue(int signo, siginfo_t* info, void* ucontext) { sa.sa_handler = SIG_DFL; CHECK_EQ(sigaction(signo, &sa, nullptr), 0); - ResetStdio(); + ResetStdio(true); raise(signo); } } @@ -500,6 +500,14 @@ static struct { } stdio[1 + STDERR_FILENO]; #endif // __POSIX__ +// Set once Node.js changes the terminal mode itself. Accessed from signal +// handlers, so it must be lock-free. +static std::atomic terminal_mode_changed{false}; + +void SetTerminalModeChanged() { + terminal_mode_changed.store(true, std::memory_order_relaxed); +} + void ResetSignalHandlers() { #ifdef __POSIX__ // Restore signal dispositions, the parent process may have changed them. @@ -544,7 +552,7 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { init_process_flags.store(flags); if (!(flags & ProcessInitializationFlags::kNoStdioInitialization)) { - atexit(ResetStdio); + atexit([]() { ResetStdio(); }); } #ifdef __POSIX__ @@ -668,7 +676,7 @@ static void PlatformInit(ProcessInitializationFlags::Flags flags) { } // Safe to call more than once and from signal handlers. -void ResetStdio() { +void ResetStdio(bool force_terminal_restore) { if (init_process_flags.load() & ProcessInitializationFlags::kNoStdioInitialization) { return; @@ -676,6 +684,15 @@ void ResetStdio() { uv_tty_reset_mode(); #ifdef __POSIX__ + // Only write back the terminal attributes recorded at startup when Node.js + // changed the terminal mode itself, or when exiting because of a signal. + // Another process sharing the terminal (e.g. a pager in `node -h | less`) + // may have changed them in the meantime, and a process that never touched + // the terminal has nothing to restore. + const bool restore_terminal = + force_terminal_restore || + terminal_mode_changed.load(std::memory_order_relaxed); + for (auto& s : stdio) { const int fd = &s - stdio; @@ -707,7 +724,7 @@ void ResetStdio() { CHECK_NE(err, -1); } - if (s.isatty) { + if (s.isatty && restore_terminal) { sigset_t sa; int err; diff --git a/src/node_internals.h b/src/node_internals.h index bb45d73e9090..87c2efabf99c 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -108,7 +108,14 @@ std::string FormatErrorMessage(v8::Isolate* isolate, const std::string& reason, v8::Local message, bool add_source_line = true); -void ResetStdio(); // Safe to call more than once and from signal handlers. +// Restores the stdio file descriptors to their startup state. Safe to call +// more than once and from signal handlers. The terminal attributes are only +// restored when Node.js changed the terminal mode itself (see +// SetTerminalModeChanged()) or when |force_terminal_restore| is true. +void ResetStdio(bool force_terminal_restore = false); +// Records that Node.js changed the terminal mode, e.g. through +// tty.ReadStream.prototype.setRawMode(). Safe to call from any thread. +void SetTerminalModeChanged(); #ifdef __POSIX__ void SignalExit(int signal, siginfo_t* info, void* ucontext); #endif diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc index c403f8cb0715..ddf986839d0f 100644 --- a/src/node_watchdog.cc +++ b/src/node_watchdog.cc @@ -137,7 +137,7 @@ std::string FormatProcessTimeoutHeader(const std::string& duration) { [[noreturn]] void ForceProcessTimeoutExit(const std::string& message) { FPrintF(stderr, "%s", message); fflush(stderr); - ResetStdio(); + ResetStdio(true); std::_Exit(static_cast(ExitCode::kProcessTimeout)); } // LCOV_EXCL_STOP diff --git a/src/tty_wrap.cc b/src/tty_wrap.cc index 0c4c02f6aa40..1f7161e0eec8 100644 --- a/src/tty_wrap.cc +++ b/src/tty_wrap.cc @@ -25,6 +25,7 @@ #include "handle_wrap.h" #include "node_buffer.h" #include "node_external_reference.h" +#include "node_internals.h" #include "stream_base-inl.h" #include "stream_wrap.h" #include "util-inl.h" @@ -134,6 +135,7 @@ void TTYWrap::SetRawMode(const FunctionCallbackInfo& args) { int mode; if (!args[0]->Int32Value(env->context()).To(&mode)) return; int err = uv_tty_set_mode(&wrap->handle_, static_cast(mode)); + if (err == 0 && mode != UV_TTY_MODE_NORMAL) SetTerminalModeChanged(); args.GetReturnValue().Set(err); } diff --git a/test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.js b/test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.js new file mode 100644 index 000000000000..04dda3ad7092 --- /dev/null +++ b/test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.js @@ -0,0 +1,30 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); + +// Tests that a process which never changed the terminal mode does not write +// the terminal attributes recorded at startup back on exit. Doing so would +// clobber changes made in the meantime by another process sharing the +// terminal, e.g. the pager in `node -h | less`. + +const child = child_process.spawn(process.execPath, [ + '-e', 'console.log("Y"); require("fs").readSync(3, Buffer.alloc(1));', +], { stdio: ['inherit', 'pipe', 'inherit', 'pipe'] }); + +child.stdout.on('data', common.mustCall(() => { + // The child has recorded the terminal state by now. Change it behind its + // back, like a pager does, then let the child exit. + process.stdin.setRawMode(true); + child.stdio[3].end('x'); +})); + +child.on('exit', common.mustCall((code) => { + assert.strictEqual(code, 0); + const { stdout } = child_process.spawnSync('stty', ['-a'], { + stdio: ['inherit', 'pipe', 'inherit'], + encoding: 'utf8', + }); + process.stdin.setRawMode(false); + assert.match(stdout, /-icanon\b/); +})); diff --git a/test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.out b/test/pseudo-tty/test-stdio-exit-keeps-foreign-tty-mode.out new file mode 100644 index 000000000000..e69de29bb2d1