Skip to content

Commit f8a3cf9

Browse files
felixgery
felixge
authored and
ry
committed
Properly handle child process exit codes
The child process 'exit' was returning the status of the process, rather than the exit code. This patch properly deconstructs the status into the exit code and the term signal a process may have received. See: http://pod.tst.eu/http://cvs.schmorp.de/libev/ev.pod#Watcher_Specific_Functions_and_Data_-5 and waitpid(2)
1 parent 9b2aac6 commit f8a3cf9

9 files changed

Lines changed: 205 additions & 20 deletions

File tree

‎doc/api.markdown‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -849,12 +849,17 @@ Child processes always have three streams associated with them. `child.stdin`,
849849

850850
### Event: 'exit'
851851

852-
`function (code) {} `
852+
`function (code, signal) {} `
853853

854-
This event is emitted after the child process ends. `code` is the final exit
855-
code of the process. After this event is emitted, the `'output'` and
856-
`'error'` callbacks will no longer be made.
854+
This event is emitted after the child process ends. If the process terminated
855+
normally, `code` is the final exit code of the process, otherwise `null`. If
856+
the process terminated due to receipt of a signal, `signal` is the string name
857+
of the signal, otherwise `null`.
857858

859+
After this event is emitted, the `'output'` and `'error'` callbacks will no
860+
longer be made.
861+
862+
See `waitpid(2)`
858863

859864
### child_process.spawn(command, args, env)
860865

@@ -906,8 +911,8 @@ be sent `'SIGTERM'`. See `signal(7)` for a list of available signals.
906911
spawn = require('child_process').spawn,
907912
grep = spawn('grep', ['ssh']);
908913

909-
grep.addListener('exit', function (code) {
910-
sys.puts('child process exited with code ' + code);
914+
grep.addListener('exit', function (code, signal) {
915+
sys.puts('child process terminated due to receipt of signal '+signal);
911916
});
912917

913918
// send SIGHUP to process
@@ -1013,7 +1018,8 @@ output, and return it all in a callback.
10131018

10141019
The callback gets the arguments `(error, stdout, stderr)`. On success, `error`
10151020
will be `null`. On error, `error` will be an instance of `Error` and `err.code`
1016-
will be the exit code of the child process.
1021+
will be the exit code of the child process, and `err.signal` will be set to the
1022+
signal that terminated the process.
10171023

10181024
There is a second optional argument to specify several options. The default options are
10191025

‎lib/child_process.js‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,15 @@ exports.execFile = function (file, args /*, options, callback */) {
7070
}
7171
});
7272

73-
child.addListener("exit", function (code) {
73+
child.addListener("exit", function (code, signal) {
7474
if (timeoutId) clearTimeout(timeoutId);
75-
if (code == 0) {
75+
if (code === 0 && signal === null) {
7676
if (callback) callback(null, stdout, stderr);
7777
} else {
7878
var e = new Error("Command failed: " + stderr);
7979
e.killed = killed;
8080
e.code = code;
81+
e.signal = signal;
8182
if (callback) callback(e, stdout, stderr);
8283
}
8384
});
@@ -91,6 +92,7 @@ function ChildProcess () {
9192

9293
var gotCHLD = false;
9394
var exitCode;
95+
var termSignal;
9496
var internal = this._internal = new InternalChildProcess();
9597

9698
var stdin = this.stdin = new Stream();
@@ -99,16 +101,17 @@ function ChildProcess () {
99101

100102
stderr.onend = stdout.onend = function () {
101103
if (gotCHLD && !stdout.readable && !stderr.readable) {
102-
self.emit('exit', exitCode);
104+
self.emit('exit', exitCode, termSignal);
103105
}
104106
};
105107

106-
internal.onexit = function (code) {
108+
internal.onexit = function (code, signal) {
107109
gotCHLD = true;
108110
exitCode = code;
111+
termSignal = signal;
109112
stdin.end();
110113
if (!stdout.readable && !stderr.readable) {
111-
self.emit('exit', exitCode);
114+
self.emit('exit', exitCode, termSignal);
112115
}
113116
};
114117

‎src/node.cc‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,152 @@ static inline const char *errno_string(int errorno) {
602602
}
603603
}
604604

605+
const char *signo_string(int signo) {
606+
#define SIGNO_CASE(e) case e: return #e;
607+
switch (signo) {
608+
609+
#ifdef SIGHUP
610+
SIGNO_CASE(SIGHUP);
611+
#endif
612+
613+
#ifdef SIGINT
614+
SIGNO_CASE(SIGINT);
615+
#endif
616+
617+
#ifdef SIGQUIT
618+
SIGNO_CASE(SIGQUIT);
619+
#endif
620+
621+
#ifdef SIGILL
622+
SIGNO_CASE(SIGILL);
623+
#endif
624+
625+
#ifdef SIGTRAP
626+
SIGNO_CASE(SIGTRAP);
627+
#endif
628+
629+
#ifdef SIGABRT
630+
SIGNO_CASE(SIGABRT);
631+
#endif
632+
633+
#ifdef SIGIOT
634+
# if SIGABRT != SIGIOT
635+
SIGNO_CASE(SIGIOT);
636+
# endif
637+
#endif
638+
639+
#ifdef SIGBUS
640+
SIGNO_CASE(SIGBUS);
641+
#endif
642+
643+
#ifdef SIGFPE
644+
SIGNO_CASE(SIGFPE);
645+
#endif
646+
647+
#ifdef SIGKILL
648+
SIGNO_CASE(SIGKILL);
649+
#endif
650+
651+
#ifdef SIGUSR1
652+
SIGNO_CASE(SIGUSR1);
653+
#endif
654+
655+
#ifdef SIGSEGV
656+
SIGNO_CASE(SIGSEGV);
657+
#endif
658+
659+
#ifdef SIGUSR2
660+
SIGNO_CASE(SIGUSR2);
661+
#endif
662+
663+
#ifdef SIGPIPE
664+
SIGNO_CASE(SIGPIPE);
665+
#endif
666+
667+
#ifdef SIGALRM
668+
SIGNO_CASE(SIGALRM);
669+
#endif
670+
671+
SIGNO_CASE(SIGTERM);
672+
SIGNO_CASE(SIGCHLD);
673+
674+
#ifdef SIGSTKFLT
675+
SIGNO_CASE(SIGSTKFLT);
676+
#endif
677+
678+
679+
#ifdef SIGCONT
680+
SIGNO_CASE(SIGCONT);
681+
#endif
682+
683+
#ifdef SIGSTOP
684+
SIGNO_CASE(SIGSTOP);
685+
#endif
686+
687+
#ifdef SIGTSTP
688+
SIGNO_CASE(SIGTSTP);
689+
#endif
690+
691+
#ifdef SIGTTIN
692+
SIGNO_CASE(SIGTTIN);
693+
#endif
694+
695+
#ifdef SIGTTOU
696+
SIGNO_CASE(SIGTTOU);
697+
#endif
698+
699+
#ifdef SIGURG
700+
SIGNO_CASE(SIGURG);
701+
#endif
702+
703+
#ifdef SIGXCPU
704+
SIGNO_CASE(SIGXCPU);
705+
#endif
706+
707+
#ifdef SIGXFSZ
708+
SIGNO_CASE(SIGXFSZ);
709+
#endif
710+
711+
#ifdef SIGVTALRM
712+
SIGNO_CASE(SIGVTALRM);
713+
#endif
714+
715+
#ifdef SIGPROF
716+
SIGNO_CASE(SIGPROF);
717+
#endif
718+
719+
#ifdef SIGWINCH
720+
SIGNO_CASE(SIGWINCH);
721+
#endif
722+
723+
#ifdef SIGIO
724+
SIGNO_CASE(SIGIO);
725+
#endif
726+
727+
#ifdef SIGPOLL
728+
SIGNO_CASE(SIGPOLL);
729+
#endif
730+
731+
#ifdef SIGLOST
732+
SIGNO_CASE(SIGLOST);
733+
#endif
734+
735+
#ifdef SIGPWR
736+
SIGNO_CASE(SIGPWR);
737+
#endif
738+
739+
#ifdef SIGSYS
740+
SIGNO_CASE(SIGSYS);
741+
#endif
742+
743+
#ifdef SIGUNUSED
744+
SIGNO_CASE(SIGUNUSED);
745+
#endif
746+
747+
default: return "";
748+
}
749+
}
750+
605751

606752
Local<Value> ErrnoException(int errorno,
607753
const char *syscall,

‎src/node.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,6 @@ v8::Local<v8::Value> ErrnoException(int errorno,
8080
const char *syscall = NULL,
8181
const char *msg = "");
8282

83+
const char *signo_string(int errorno);
8384
} // namespace node
8485
#endif // SRC_NODE_H_

‎src/node_child_process.cc‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright 2009 Ryan Dahl <ry@tinyclouds.org>
22
#include <node_child_process.h>
3+
#include <node.h>
34

45
#include <assert.h>
56
#include <string.h>
@@ -244,7 +245,7 @@ int ChildProcess::Spawn(const char *file,
244245
}
245246

246247

247-
void ChildProcess::OnExit(int code) {
248+
void ChildProcess::OnExit(int status) {
248249
HandleScope scope;
249250

250251
pid_ = -1;
@@ -258,10 +259,20 @@ void ChildProcess::OnExit(int code) {
258259

259260
TryCatch try_catch;
260261

261-
Local<Value> argv[1];
262-
argv[0] = Integer::New(code);
262+
Local<Value> argv[2];
263+
if (WIFEXITED(status)) {
264+
argv[0] = Integer::New(WEXITSTATUS(status));
265+
} else {
266+
argv[0] = Local<Value>::New(Null());
267+
}
268+
269+
if (WIFSIGNALED(status)) {
270+
argv[1] = String::NewSymbol(signo_string(WTERMSIG(status)));
271+
} else {
272+
argv[1] = Local<Value>::New(Null());
273+
}
263274

264-
onexit->Call(handle_, 1, argv);
275+
onexit->Call(handle_, 2, argv);
265276

266277
if (try_catch.HasCaught()) {
267278
FatalException(try_catch);

‎test/fixtures/exit.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
process.exit(process.argv[2] || 1);

‎test/simple/test-child-process-exit-code.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
require("../common");
2+
var spawn = require('child_process').spawn
3+
, path = require('path')
4+
, sub = path.join(fixturesDir, 'exit.js')
5+
, child = spawn(process.argv[0], [sub, 23])
6+
;
7+
8+
child.addListener('exit', function(code, signal) {
9+
assert.strictEqual(code, 23);
10+
assert.strictEqual(signal, null);
11+
});

‎test/simple/test-child-process-kill.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ require("../common");
22

33
var spawn = require('child_process').spawn;
44

5-
var exitStatus = -1;
5+
var exitCode;
6+
var termSignal;
67
var gotStdoutEOF = false;
78
var gotStderrEOF = false;
89

@@ -25,14 +26,16 @@ cat.stderr.addListener("end", function () {
2526
gotStderrEOF = true;
2627
});
2728

28-
cat.addListener("exit", function (status) {
29-
exitStatus = status;
29+
cat.addListener("exit", function (code, signal) {
30+
exitCode = code;
31+
termSignal = signal;
3032
});
3133

3234
cat.kill();
3335

3436
process.addListener("exit", function () {
35-
assert.ok(exitStatus > 0);
37+
assert.strictEqual(exitCode, null);
38+
assert.strictEqual(termSignal, 'SIGTERM');
3639
assert.ok(gotStdoutEOF);
3740
assert.ok(gotStderrEOF);
3841
});

‎test/simple/test-exec.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ exec("ls /DOES_NOT_EXIST", function (err, stdout, stderr) {
2323
assert.equal("", stdout);
2424
assert.equal(true, err.code != 0);
2525
assert.equal(false, err.killed);
26+
assert.strictEqual(null, err.signal);
2627
puts("error code: " + err.code);
2728
puts("stdout: " + JSON.stringify(stdout));
2829
puts("stderr: " + JSON.stringify(stderr));
@@ -36,11 +37,13 @@ exec("ls /DOES_NOT_EXIST", function (err, stdout, stderr) {
3637
exec("sleep 10", { timeout: 50 }, function (err, stdout, stderr) {
3738
assert.ok(err);
3839
assert.ok(err.killed);
40+
assert.equal(err.signal, 'SIGKILL');
3941
});
4042

4143
exec('python -c "print 200000*\'C\'"', { maxBuffer: 1000 }, function (err, stdout, stderr) {
4244
assert.ok(err);
4345
assert.ok(err.killed);
46+
assert.equal(err.signal, 'SIGKILL');
4447
});
4548

4649
process.addListener("exit", function () {

0 commit comments

Comments
 (0)