src: skip tcsetattr on exit when backgrounded

ResetStdio() unconditionally restored saved termios state on every
tty stdio fd at exit. When Node is not the foreground process
group for that terminal (`node script.js &`, ctrl-Z + bg, or a
later pipeline stage taking the foreground), this clobbers the
foreground pgrp's termios - breaking shell readline, leaving
terminals in raw mode after the foreground pager exits, etc.

Guard the tcsetattr call with `tcgetpgrp(fd) == getpgrp()` so we
only restore when we are still the foreground pgrp for that
terminal. The existing SIGTTOU defensive block is preserved to
cover the unavoidable race where the foreground pgrp changes
between the check and the call.

Fixes: #35536
Refs: #42433
Signed-off-by: Maruthan G <[email protected]>

Author: Maruthan G

src/node.cc

@@ -697,8 +697,22 @@ void ResetStdio() {
       sigset_t sa;
       int err;
 
+      // Only restore the termios state if this process is (still) the
+      // foreground process group for the controlling terminal. If we are a
+      // background job, calling tcsetattr() would either be silently ignored
+      // (with SIGTTOU blocked, see below) or, worse, clobber the terminal
+      // state of whichever process is currently in the foreground -- e.g.
+      // the parent shell's readline.
+      // See https://github.com/nodejs/node/issues/35536.
+      pid_t fg_pgrp = tcgetpgrp(fd);
+      if (fg_pgrp == -1 || fg_pgrp != getpgrp()) continue;
+
       // We might be a background job that doesn't own the TTY so block SIGTTOU
       // before making the tcsetattr() call, otherwise that signal suspends us.
+      // The foreground-pgrp check above handles the common case, but a race
+      // is still possible if the foreground pgrp changes between tcgetpgrp()
+      // and tcsetattr(); blocking SIGTTOU keeps us from being suspended in
+      // that race.
       sigemptyset(&sa);
       sigaddset(&sa, SIGTTOU);
 

test/parallel/test-tty-bg-tcsetattr.js

@@ -0,0 +1,138 @@
+'use strict';
+
+// Regression test for https://github.com/nodejs/node/issues/35536
+//
+// When Node.js is run as a background process, its stdio reset on exit must
+// not call tcsetattr() on the controlling terminal -- otherwise it clobbers
+// the foreground process's termios state (e.g. breaks readline in the parent
+// shell, or the pager when piping `node ... | less`).
+//
+// This test allocates a pty, snapshots termios, then forks/execs Node such
+// that Node's process group is *not* the foreground process group of the pty.
+// After Node exits, it compares termios. If termios changed, the bug is
+// present.
+//
+// Implemented via a Python helper because Node's standard library does not
+// expose pty(7) / tcgetpgrp() / setpgid() directly.
+
+const common = require('../common');
+
+if (common.isWindows) {
+  common.skip('pty test, POSIX only');
+}
+
+const assert = require('assert');
+const child_process = require('child_process');
+
+// Probe for python3 (the helper uses the `pty` and `termios` stdlib modules).
+const pythonCandidates = ['python3', 'python'];
+let python = null;
+for (const candidate of pythonCandidates) {
+  const probe = child_process.spawnSync(candidate, ['-c', 'import pty, termios'], {
+    stdio: 'ignore',
+  });
+  if (probe.status === 0) {
+    python = candidate;
+    break;
+  }
+}
+if (python === null) {
+  common.skip('python3 with pty/termios modules not available');
+}
+
+const helper = `
+import os
+import pty
+import sys
+import termios
+
+node = sys.argv[1]
+
+parent_fd, child_fd = pty.openpty()
+
+# Snapshot termios on the slave side before running node.
+before = termios.tcgetattr(child_fd)
+
+pid = os.fork()
+if pid == 0:
+    # Child: become leader of a new session, attach the pty as our
+    # controlling terminal, then place ourselves in a *new* process group
+    # that is NOT the foreground process group of the tty. From the tty's
+    # perspective, this child is "backgrounded".
+    os.setsid()
+    os.close(parent_fd)
+
+    name = os.ttyname(child_fd)
+    fd = os.open(name, os.O_RDWR)
+    os.dup2(fd, child_fd)
+    os.close(fd)
+
+    os.dup2(child_fd, 0)
+    os.dup2(child_fd, 1)
+    os.dup2(child_fd, 2)
+    if child_fd > 2:
+        os.close(child_fd)
+
+    # The session leader's pgrp is currently the tty's foreground pgrp
+    # (set by the TIOCSCTTY in the open above). Move to a new pgrp so that
+    # the foreground pgrp != getpgrp(). This is the configuration the bug
+    # reporter hits with shell job-control 'bg'.
+    os.setpgid(0, 0)
+    new_pgrp = os.getpgrp()
+    fg_pgrp = os.tcgetpgrp(0)
+    if fg_pgrp == new_pgrp:
+        # Could not background ourselves; bail loudly so the test fails.
+        os._exit(99)
+
+    # Run node. It should exit cleanly without modifying termios.
+    os.execvp(node, [node, '-e', '0'])
+    os._exit(98)
+
+os.close(child_fd)
+# Drain the parent_fd so the child isn't blocked on output (it shouldn't
+# print anything, but be defensive).
+try:
+    import select
+    while True:
+        rfds, _, _ = select.select([parent_fd], [], [], 0.05)
+        if not rfds:
+            # Check whether child has exited.
+            wpid, _ = os.waitpid(pid, os.WNOHANG)
+            if wpid == pid:
+                break
+            continue
+        try:
+            data = os.read(parent_fd, 4096)
+        except OSError:
+            break
+        if not data:
+            break
+finally:
+    pass
+
+# Make sure we have reaped the child.
+try:
+    wpid, status = os.waitpid(pid, 0)
+except ChildProcessError:
+    status = 0
+
+after = termios.tcgetattr(child_fd)
+os.close(parent_fd)
+
+if before != after:
+    sys.stderr.write('termios changed: before=%r after=%r\\n' % (before, after))
+    sys.exit(1)
+
+sys.exit(0)
+`;
+
+const result = child_process.spawnSync(python, ['-c', helper, process.execPath], {
+  encoding: 'utf8',
+  stdio: ['ignore', 'pipe', 'pipe'],
+});
+
+assert.strictEqual(
+  result.status, 0,
+  `helper failed (status=${result.status}, signal=${result.signal})\n` +
+  `stdout: ${result.stdout}\nstderr: ${result.stderr}`
+);