src: restore tty state on exit only if changed

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<bool> 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,14 +676,23 @@ 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;
   }
 
   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;
 

src/node_internals.h

@@ -108,7 +108,14 @@ std::string FormatErrorMessage(v8::Isolate* isolate,
                                const std::string& reason,
                                v8::Local<v8::Message> 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

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<int>(ExitCode::kProcessTimeout));
 }
 // LCOV_EXCL_STOP

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<Value>& args) {
   int mode;
   if (!args[0]->Int32Value(env->context()).To(&mode)) return;
   int err = uv_tty_set_mode(&wrap->handle_, static_cast<uv_tty_mode_t>(mode));
+  if (err == 0 && mode != UV_TTY_MODE_NORMAL) SetTerminalModeChanged();
   args.GetReturnValue().Set(err);
 }
 

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/);
+}));