diff --git a/src/node.cc b/src/node.cc index 92d0402b7dfbaf..4954c4ab6b1e1b 100644 --- a/src/node.cc +++ b/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); diff --git a/test/parallel/test-tty-bg-tcsetattr.js b/test/parallel/test-tty-bg-tcsetattr.js new file mode 100644 index 00000000000000..40dc1a8f7a472f --- /dev/null +++ b/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}` +);