Home Original page

build: block SIGTTOU before calling tcsetattr() by bnoordhuis · Pull Request #28535 · nodejs/node

@nodejs-github-bot added the c++

Issues and PRs that require attention from people who are familiar with C++.

label

Jul 4, 2019

gireeshpunathil

richardlau

@bnoordhuis

sam-github

@Trott Trott added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Jul 5, 2019

pull bot pushed a commit to Mattlk13/node that referenced this pull request

Jul 6, 2019 
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.

This is a better fix than PR nodejs#28490 for issue nodejs#28479.

Fixes: nodejs#28530
Fixes: nodejs#28479
Refs: nodejs#28490

PR-URL: nodejs#28535
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

targos pushed a commit that referenced this pull request

Jul 20, 2019 
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.

This is a better fix than PR #28490 for issue #28479.

Fixes: #28530
Fixes: #28479
Refs: #28490

PR-URL: #28535
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

This was referenced

Jul 23, 2019

sunshowers added a commit to nextest-rs/nextest that referenced this pull request

Jan 3, 2026 
When a test spawns an interactive shell (e.g., `zsh -ic`), the shell
opens `/dev/tty` directly and calls `tcsetpgrp` to become the foreground
process group. When the shell exits, nextest is now in the background.
If nextest then tries to restore terminal state via `tcsetattr`, it
receives SIGTTOU and gets suspended.

The fix is to block SIGTTOU around `tcsetattr` calls using
`sigprocmask`. This is a well-established pattern used by:

- less: https://github.com/gwsw/less/blob/e77e117/signal.c#L213-L221 
- Node.js: nodejs/node#28535

Also adds a test helper binary (`grab-foreground`) that simulates what
interactive shells do, and an ignored integration test to verify the
fix. The test is currently ignored because it requires a version of nextest
with the fix -- we'll un-ignore it once a new version of nextest is out.

Fixes: #2878