Skip to content

Commit 1c77865

Browse files
committed
src: don't kill own process group on failed spawn
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
1 parent 3f2fc8a commit 1c77865

2 files changed

Lines changed: 26 additions & 1 deletion

File tree

‎src/process_wrap.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ class ProcessWrap : public HandleWrap {
112112
object,
113113
reinterpret_cast<uv_handle_t*>(&process_),
114114
AsyncWrap::PROVIDER_PROCESSWRAP) {
115+
process_.pid = 0;
115116
MarkAsUninitialized();
116117
}
117118

@@ -355,7 +356,10 @@ class ProcessWrap : public HandleWrap {
355356
signal = SIGKILL;
356357
}
357358
#endif
358-
int err = uv_process_kill(&wrap->process_, signal);
359+
// uv_spawn() only assigns a pid when it succeeds, and kill(0, signal)
360+
// signals every process in our own process group.
361+
int err = wrap->process_.pid > 0 ? uv_process_kill(&wrap->process_, signal)
362+
: UV_ESRCH;
359363
args.GetReturnValue().Set(err);
360364
}
361365

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { spawn } = require('child_process');
5+
6+
// Killing a child process that never spawned must not signal the process
7+
// group of the caller. The check runs in a detached child so that a
8+
// regression cannot take the test runner down with it.
9+
const code = `
10+
const { spawn } = require('child_process');
11+
const child = spawn('foo123');
12+
child.on('error', () => {});
13+
if (child.kill() !== false || child.killed !== false) process.exit(1);
14+
`;
15+
16+
const child = spawn(process.execPath, ['-e', code], { detached: true });
17+
18+
child.on('exit', common.mustCall((code, signal) => {
19+
assert.strictEqual(signal, null);
20+
assert.strictEqual(code, 0);
21+
}));

0 commit comments

Comments
 (0)