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