Skip to content

src: skip tcsetattr on exit when backgrounded#62946

Open
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:fix-35536-tcsetattr-bg
Open

src: skip tcsetattr on exit when backgrounded#62946
maruthang wants to merge 1 commit intonodejs:mainfrom
maruthang:fix-35536-tcsetattr-bg

Conversation

@maruthang
Copy link
Copy Markdown

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


Note: I was unable to build Node locally on this Windows host (vcbuild fails — missing NASM, MSVC tools not on PATH). cpplint is clean and the new JS test lints clean. The C++ change is a textbook POSIX idiom (tcgetpgrp == getpgrp before tcsetattr) and is gated by the existing s.isatty branch + __POSIX__ ifdef so Windows and non-tty fds are unaffected. Looking forward to CI verification on Linux/macOS — see the PR diff for the manual reproducer.

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: nodejs#35536
Refs: nodejs#42433
Signed-off-by: Maruthan G <[email protected]>
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background node process corrupts terminal state with tcsetattr() on exit

2 participants