[node-core-library] SubprocessTerminator ignores SIGHUP, so detached children outlive a closed terminal
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 75/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- node.js, typescript
- Domain
- backend, operating-systems
Research direction
Start by reading SubprocessTerminator in the @rushstack/node-core-library package, especially _ensureInitialized and _cleanupChildProcesses, which the issue names. Check the existing SIGINT/SIGTERM behavior and its tests, then verify that SIGHUP is handled and cleaned up on non-Windows. Done when the repro no longer leaves the detached child running after terminal closure or SIGHUP.
Written by the indexing model from the issue text.
Description
🤖 Filed by GitHub Copilot acting for @namankanakiya (light review only).
Package: @rushstack/node-core-library 5.25.1. Linux/macOS (non-Windows).
Repro: script.js:
const { SubprocessTerminator } = require('@rushstack/node-core-library');
const c = require('child_process').spawn('sleep', ['1000'], SubprocessTerminator.RECOMMENDED_OPTIONS);
SubprocessTerminator.killProcessTreeOnExit(c, SubprocessTerminator.RECOMMENDED_OPTIONS);
setInterval(() => {}, 1000);
Run it in a terminal, then close the terminal (or kill -HUP <pid>).
Observed: sleep keeps running with ppid 1. RECOMMENDED_OPTIONS (detached: true) puts the child in its own session, so the terminal's hangup never reaches it, and killProcessTreeOnExit only handles SIGINT, SIGTERM and exit. With SIGINT or SIGTERM the child is killed as expected.
Expected: on non-Windows, SIGHUP is handled like SIGINT/SIGTERM, so closing the terminal of a tool that uses SubprocessTerminator does not leak its detached children.
Suggested fix: in _ensureInitialized, also process.prependListener('SIGHUP', _onTerminateSignal), and remove it in _cleanupChildProcesses.
Happy to test a candidate fix.
- Dominant language
- TypeScript
- Stars
- 6.5k
- Forks
- 710
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 45
Getting set up
Starts the project's dev container in your browser, under your own GitHub account.
- No Dockerfile or Docker Compose file
- Has a pull request template
- No contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from microsoft/rushstack
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
[rush] Upgrade the pnpm-sync-lib dependency to 0.3.5.Possibly taken @martinnaj claimed this 11 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
microsoft/rushstack#5971 · 2 comments ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
microsoft/rushstack#5902 · 1 reaction ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
microsoft/rushstack#5839 · 1 reaction ·
Maintainers usually reply within 1 day
All issues in microsoft/rushstack
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
Agenta-AI/agenta#7542 · 1 comment · 1 reaction ·
Maintainers usually reply within 1 day
-
code quality security
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
enhancement good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
-
Difficulty 1/5 Under an hour Newbie friendliness 82/100
homarr-labs/dashboard-icons#2939 ·
Maintainers usually reply within 1 day