src: don't kill own process group on failed spawn by lazerg · Pull Request #65054 · nodejs/node

lazerg · 2026-08-05T16:34:50Z

libuv only assigns a pid to the process handle once uv_spawn() succeeds, so a child that never started keeps pid 0. Calling kill() on that child still reached uv_process_kill(), which ended up in kill(0, signal) and signalled every process in the caller's own process group, Node included. The handle is now reported as ESRCH when there is no pid, so child.kill() just returns false. The pid is also zeroed in the constructor so the check never reads an unassigned value.

Reproducing it on its own needs no prototype tampering:

const { spawn } = require('child_process');
const child = spawn('foo123');
child.on('error', () => {});
child.kill();  // terminates the whole process group

In the report the spawn failed for a different reason: overriding Array.prototype[Symbol.iterator] makes normalizeSpawnArguments() build an empty env, so the command was no longer found through PATH. The dead handle is what took the parent down.

Behavior change

kill() on a child that failed to spawn used to return true on POSIX. It now returns false.

Windows was never affected by the process-group problem, since uv_process_kill() bails out with EINVAL when the handle has no process handle. It reached the throw new ErrnoException(err, 'kill') branch instead, so there kill() goes from throwing EINVAL to returning false. Both platforms now agree.

subprocess.killed is also left alone in this case, which matches what the docs already say about it being set only when a signal is sent successfully.

Fixes: #65052

nodejs-github-bot · 2026-08-21T01:20:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76096/

codecov · 2026-08-21T03:10:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.99%. Comparing base (728ae35) to head (2eeec11).
⚠️ Report is 223 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65054      +/-   ##
==========================================
- Coverage   90.01%   89.99%   -0.02%     
==========================================
  Files         757      757              
  Lines      257739   257740       +1     
  Branches    48882    48890       +8     
==========================================
- Hits       232001   231956      -45     
- Misses      16844    16865      +21     
- Partials     8894     8919      +25

Files with missing lines	Coverage Δ
src/process_wrap.cc	`75.11% <100.00%> (+0.11%)`	⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-21T15:24:36Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76155/

libuv only assigns a pid to the process handle once uv_spawn() has succeeded, so a child that never started keeps pid 0. Calling kill() on such a child still reached uv_process_kill(), which ended up in kill(0, signal) and signalled every process in the caller's own process group, Node included. Return ESRCH when the handle has no pid, and zero the pid in the constructor so the check never reads an unassigned value. Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>

lazerg · 2026-09-02T18:27:13Z

Thanks, rebased. The branch now sits on top of 728ae35678, which includes 02f4630, so doc/node.1 is back in sync. make tools/.manpagelintstamp passes locally.

New head: 2eeec115f5. There were no conflicts, and the fix itself is unchanged: src/process_wrap.cc plus the test and its fixture.

nodejs-github-bot · 2026-09-08T06:36:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/77181/

nodejs-github-bot · 2026-09-08T07:00:58Z

Commit Queue failed

   ✘  GitHub CI is still running
   ✘  Last Jenkins CI still running

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output

- Loading data for nodejs/node/pull/65054
✔  Done loading data for nodejs/node/pull/65054
----------------------------------- PR info ------------------------------------
Title      src: don't kill own process group on failed spawn (#65054)
Author     Lazizbek Ergashev <lazerg2@gmail.com> (@lazerg)
Branch     lazerg:fix/issue-65052-kill-unspawned-process -> nodejs:main
Labels     child_process, c++, needs-ci, commit-queue
Commits    3
 - src: don't kill own process group on failed spawn
 - test: drop redundant Refs comment from kill-spawn-error test
 - test: move inline kill-spawn-error script to a fixture
Committers 1
 - Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 05 Aug 2026 16:34:50 GMT
   ✔  Approvals: 2
   ✔  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/65054#pullrequestreview-4867699604
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/65054#pullrequestreview-5138143552
   ✘  GitHub CI is still running
   ℹ  Last Full PR CI on 2026-09-08T06:36:20Z: https://ci.nodejs.org/job/node-test-pull-request/77181/
- Querying data for job/node-test-pull-request/77181/
✔  Build data downloaded
   ✘  Last Jenkins CI still running
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

View workflow run

lazerg · 2026-09-11T00:01:30Z

I checked other node PRs from the same time window on 2026-09-08. The node-test-commit-arm-fanned job failed for all of them too, while every individual platform build here passed. This is a Jenkins infra issue, not caused by this change.

nodejs-github-bot · 2026-09-11T00:05:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/77331/

nodejs-github-bot · 2026-09-12T09:03:19Z

Commit Queue failed

This pull request has multiple commits, but no landing policy was selected.

Add commit-queue-squash PRs the Commit Queue should land as one squashed commit. to land it as one commit, or commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. to land the commits separately.

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output

- Loading data for nodejs/node/pull/65054
✔  Done loading data for nodejs/node/pull/65054
----------------------------------- PR info ------------------------------------
Title      src: don't kill own process group on failed spawn (#65054)
Author     Lazizbek Ergashev <lazerg2@gmail.com> (@lazerg)
Branch     lazerg:fix/issue-65052-kill-unspawned-process -> nodejs:main
Labels     child_process, c++, needs-ci, commit-queue
Commits    3
 - src: don't kill own process group on failed spawn
 - test: drop redundant Refs comment from kill-spawn-error test
 - test: move inline kill-spawn-error script to a fixture
Committers 1
 - Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 05 Aug 2026 16:34:50 GMT
   ✔  Approvals: 2
   ✔  - Aviv Keller (@avivkeller): https://github.com/nodejs/node/pull/65054#pullrequestreview-4867699604
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/65054#pullrequestreview-5138143552
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-09-11T00:05:15Z: https://ci.nodejs.org/job/node-test-pull-request/77331/
- Querying data for job/node-test-pull-request/77331/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   dff7a160fe..312c562608  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - de9400ebe0 quic: add promise to QuicStream for pending strms
 - 312c562608 quic: add promise to QuicStream for pending strms
--------------------------------------------------------------------------------
HEAD is now at 312c562608 quic: add promise to QuicStream for pending strms
   ✔  Reset to origin/main
- Downloading patch for 65054
From https://github.com/nodejs/node
 * branch                  refs/pull/65054/merge -> FETCH_HEAD
✔  Fetched commits as c8b346e98e83..2eeec115f5bb
--------------------------------------------------------------------------------
[main 3644546160] src: don't kill own process group on failed spawn
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 21:33:11 2026 +0500
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-child-process-kill-spawn-error.js
[main bfc9d42b2f] test: drop redundant Refs comment from kill-spawn-error test
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 21:57:48 2026 +0500
 1 file changed, 1 deletion(-)
[main 443b423c8d] test: move inline kill-spawn-error script to a fixture
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 23:22:53 2026 +0500
 2 files changed, 7 insertions(+), 8 deletions(-)
 create mode 100644 test/fixtures/child-process-kill-spawn-error.js
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
(node:793) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src: don't kill own process group on failed spawn

libuv only assigns a pid to the process handle once uv_spawn() has
succeeded, so a child that never started keeps pid 0. Calling kill()
on such a child still reached uv_process_kill(), which ended up in
kill(0, signal) and signalled every process in the caller's own
process group, Node included.

Return ESRCH when the handle has no pid, and zero the pid in the
constructor so the check never reads an unassigned value.

Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
--------------------------------------------------------------------------------
[detached HEAD ffe8ca52ac] src: don't kill own process group on failed spawn
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 21:33:11 2026 +0500
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-child-process-kill-spawn-error.js
Rebasing (3/6)
Rebasing (4/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: drop redundant Refs comment from kill-spawn-error test

PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
--------------------------------------------------------------------------------
[detached HEAD 3534173dc0] test: drop redundant Refs comment from kill-spawn-error test
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 21:57:48 2026 +0500
 1 file changed, 1 deletion(-)
Rebasing (5/6)
Rebasing (6/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: move inline kill-spawn-error script to a fixture

PR-URL: https://github.com/nodejs/node/pull/65054
Fixes: https://github.com/nodejs/node/issues/65052
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
--------------------------------------------------------------------------------
[detached HEAD 8965775810] test: move inline kill-spawn-error script to a fixture
 Author: Lazizbek Ergashev <lazerg2@gmail.com>
 Date: Wed Aug 5 23:22:53 2026 +0500
 2 files changed, 7 insertions(+), 8 deletions(-)
 create mode 100644 test/fixtures/child-process-kill-spawn-error.js
Successfully rebased and updated refs/heads/main.
--------------------------------------------------------------------------------
   ℹ  Add `commit-queue-squash` label to land the PR as one commit, or `commit-queue-rebase` to land as separate commits.

View workflow run

nodejs-github-bot · 2026-09-12T10:43:40Z

Landed in ce20f7a

libuv only assigns a pid to the process handle once uv_spawn() has succeeded, so a child that never started keeps pid 0. Calling kill() on such a child still reached uv_process_kill(), which ended up in kill(0, signal) and signalled every process in the caller's own process group, Node included. Return ESRCH when the handle has no pid, and zero the pid in the constructor so the check never reads an unassigned value. Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com> PR-URL: #65054 Fixes: #65052 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Robert Nagy <ronagy@icloud.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. child_process Issues and PRs related to the child_process subsystem. needs-ci PRs that need a full CI run. labels Aug 5, 2026

lazerg force-pushed the fix/issue-65052-kill-unspawned-process branch from 1c77865 to 38627f1 Compare August 5, 2026 16:51

avivkeller reviewed Aug 5, 2026

View reviewed changes

Comment thread src/process_wrap.cc

avivkeller reviewed Aug 5, 2026

View reviewed changes

Comment thread test/parallel/test-child-process-kill-spawn-error.js Outdated

avivkeller approved these changes Aug 5, 2026

View reviewed changes

trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 21, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 21, 2026

This comment was marked as resolved.

Sign in to view

lazerg added 3 commits September 2, 2026 23:25

test: drop redundant Refs comment from kill-spawn-error test

b4675db

test: move inline kill-spawn-error script to a fixture

2eeec11

lazerg force-pushed the fix/issue-65052-kill-unspawned-process branch from 3174c46 to 2eeec11 Compare September 2, 2026 18:26

MikeMcC399 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 3, 2026

panva removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 3, 2026

ronag approved these changes Sep 8, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 8, 2026

ronag added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 8, 2026

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 8, 2026

panva added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Sep 12, 2026

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 12, 2026

ronag added commit-queue-squash PRs the Commit Queue should land as one squashed commit. commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Sep 12, 2026

nodejs-github-bot merged commit ce20f7a into nodejs:main Sep 12, 2026
100 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 12, 2026

github-actions Bot mentioned this pull request Sep 20, 2026

2026-09-22, Version 26.10.0 (Current) #66163

Open

Navigation Menu

Uh oh!

src: don't kill own process group on failed spawn - #65054

src: don't kill own process group on failed spawn#65054
nodejs-github-bot merged 3 commits into
nodejs:mainfrom
lazerg:fix/issue-65052-kill-unspawned-process

lazerg commented Aug 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

codecov Bot commented Aug 21, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

This comment was marked as resolved.

lazerg commented Sep 2, 2026

Uh oh!

nodejs-github-bot commented Sep 8, 2026

Uh oh!

nodejs-github-bot commented Sep 8, 2026

Uh oh!

lazerg commented Sep 11, 2026

Uh oh!

nodejs-github-bot commented Sep 11, 2026

Uh oh!

nodejs-github-bot commented Sep 12, 2026

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

lazerg commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behavior change

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

codecov Bot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Aug 21, 2026

Uh oh!

This comment was marked as resolved.

lazerg commented Sep 2, 2026

Uh oh!

nodejs-github-bot commented Sep 8, 2026

Uh oh!

nodejs-github-bot commented Sep 8, 2026

Commit Queue failed

Uh oh!

lazerg commented Sep 11, 2026

Uh oh!

nodejs-github-bot commented Sep 11, 2026

Uh oh!

nodejs-github-bot commented Sep 12, 2026

Commit Queue failed

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

lazerg commented Aug 5, 2026 •

edited

Loading

codecov Bot commented Aug 21, 2026 •

edited

Loading