Skip to content

Commit

Permalink
Revert commits 12c8b27 and 88f416a, fixed properly in 2fe4558.
Browse files Browse the repository at this point in the history
  • Loading branch information
bnoordhuis committed Aug 5, 2011
1 parent 2fe4558 commit 30d20cf
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 34 deletions.
21 changes: 2 additions & 19 deletions doc/api/child_processes.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -234,24 +234,7 @@ be sent `'SIGTERM'`. See `signal(7)` for a list of available signals.
// send SIGHUP to process
grep.kill('SIGHUP');

Note that while the function is called `kill`, the signal delivered to the
child process may not actually kill it. `kill` really just sends a signal
to a process.

Please note that the example contains a potential race condition on
(at least) UNIX systems. Here is why:

The canonical approach to starting a child process is to call `fork()` to
create a copy of the current process, followed by a call to `execve()` to
replace the copy with the actual child process. This runs in tandem with
the parent process.

The time between `fork()` and `execve()` is short but it's not zero.
The child process may not have actually started when `spawn()` returns.
Thus, if you send a signal immediately after the call to `spawn()`, it may
end up being delivered to the copy of the current process and *not* the
actual child process.

The resulting behavior is undefined. It likely won't do what you want it to.
Note that while the function is called `kill`, the signal delivered to the child
process may not actually kill it. `kill` really just sends a signal to a process.

See `kill(2)`
18 changes: 3 additions & 15 deletions test/simple/test-child-process-kill.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,11 @@ var termSignal;
var gotStdoutEOF = false;
var gotStderrEOF = false;

var ping = "42\n";

var cat = spawn('cat');

//
// This test sends a signal to a child process.
//
// There is a potential race here where the signal is delivered
// after the fork() but before execve(). IOW, the signal is sent
// before the child process has truly been started.
//
// So we wait for a sign of life from the child (the ping response)
// before sending the signal.
//
cat.stdin.write(ping);

cat.stdout.addListener('data', function(chunk) {
assert.equal(chunk.toString(), ping);
cat.kill();
assert.ok(false);
});

cat.stdout.addListener('end', function() {
Expand All @@ -67,6 +53,8 @@ cat.addListener('exit', function(code, signal) {
termSignal = signal;
});

cat.kill();

process.addListener('exit', function() {
assert.strictEqual(exitCode, null);
assert.strictEqual(termSignal, 'SIGTERM');
Expand Down

0 comments on commit 30d20cf

Please sign in to comment.