From f8a3cf980f09b48ce1c56f11dc47e204093ac8b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisend=C3=B6rfer?= Date: Wed, 28 Apr 2010 15:04:08 +0200 Subject: [PATCH] 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) --- doc/api.markdown | 20 ++- lib/child_process.js | 13 +- src/node.cc | 146 ++++++++++++++++++++ src/node.h | 1 + src/node_child_process.cc | 19 ++- test/fixtures/exit.js | 1 + test/simple/test-child-process-exit-code.js | 11 ++ test/simple/test-child-process-kill.js | 11 +- test/simple/test-exec.js | 3 + 9 files changed, 205 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/exit.js create mode 100644 test/simple/test-child-process-exit-code.js diff --git a/doc/api.markdown b/doc/api.markdown index a21d2d59f2..041ec39cd9 100644 --- a/doc/api.markdown +++ b/doc/api.markdown @@ -849,12 +849,17 @@ Child processes always have three streams associated with them. `child.stdin`, ### Event: 'exit' -`function (code) {} ` +`function (code, signal) {} ` -This event is emitted after the child process ends. `code` is the final exit -code of the process. After this event is emitted, the `'output'` and -`'error'` callbacks will no longer be made. +This event is emitted after the child process ends. If the process terminated +normally, `code` is the final exit code of the process, otherwise `null`. If +the process terminated due to receipt of a signal, `signal` is the string name +of the signal, otherwise `null`. +After this event is emitted, the `'output'` and `'error'` callbacks will no +longer be made. + +See `waitpid(2)` ### child_process.spawn(command, args, env) @@ -906,8 +911,8 @@ be sent `'SIGTERM'`. See `signal(7)` for a list of available signals. spawn = require('child_process').spawn, grep = spawn('grep', ['ssh']); - grep.addListener('exit', function (code) { - sys.puts('child process exited with code ' + code); + grep.addListener('exit', function (code, signal) { + sys.puts('child process terminated due to receipt of signal '+signal); }); // send SIGHUP to process @@ -1013,7 +1018,8 @@ output, and return it all in a callback. The callback gets the arguments `(error, stdout, stderr)`. On success, `error` will be `null`. On error, `error` will be an instance of `Error` and `err.code` -will be the exit code of the child process. +will be the exit code of the child process, and `err.signal` will be set to the +signal that terminated the process. There is a second optional argument to specify several options. The default options are diff --git a/lib/child_process.js b/lib/child_process.js index f34a8f47b9..1a445054e1 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -70,14 +70,15 @@ exports.execFile = function (file, args /*, options, callback */) { } }); - child.addListener("exit", function (code) { + child.addListener("exit", function (code, signal) { if (timeoutId) clearTimeout(timeoutId); - if (code == 0) { + if (code === 0 && signal === null) { if (callback) callback(null, stdout, stderr); } else { var e = new Error("Command failed: " + stderr); e.killed = killed; e.code = code; + e.signal = signal; if (callback) callback(e, stdout, stderr); } }); @@ -91,6 +92,7 @@ function ChildProcess () { var gotCHLD = false; var exitCode; + var termSignal; var internal = this._internal = new InternalChildProcess(); var stdin = this.stdin = new Stream(); @@ -99,16 +101,17 @@ function ChildProcess () { stderr.onend = stdout.onend = function () { if (gotCHLD && !stdout.readable && !stderr.readable) { - self.emit('exit', exitCode); + self.emit('exit', exitCode, termSignal); } }; - internal.onexit = function (code) { + internal.onexit = function (code, signal) { gotCHLD = true; exitCode = code; + termSignal = signal; stdin.end(); if (!stdout.readable && !stderr.readable) { - self.emit('exit', exitCode); + self.emit('exit', exitCode, termSignal); } }; diff --git a/src/node.cc b/src/node.cc index c7b17d4dfc..8e2111129e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -602,6 +602,152 @@ static inline const char *errno_string(int errorno) { } } +const char *signo_string(int signo) { +#define SIGNO_CASE(e) case e: return #e; + switch (signo) { + +#ifdef SIGHUP + SIGNO_CASE(SIGHUP); +#endif + +#ifdef SIGINT + SIGNO_CASE(SIGINT); +#endif + +#ifdef SIGQUIT + SIGNO_CASE(SIGQUIT); +#endif + +#ifdef SIGILL + SIGNO_CASE(SIGILL); +#endif + +#ifdef SIGTRAP + SIGNO_CASE(SIGTRAP); +#endif + +#ifdef SIGABRT + SIGNO_CASE(SIGABRT); +#endif + +#ifdef SIGIOT +# if SIGABRT != SIGIOT + SIGNO_CASE(SIGIOT); +# endif +#endif + +#ifdef SIGBUS + SIGNO_CASE(SIGBUS); +#endif + +#ifdef SIGFPE + SIGNO_CASE(SIGFPE); +#endif + +#ifdef SIGKILL + SIGNO_CASE(SIGKILL); +#endif + +#ifdef SIGUSR1 + SIGNO_CASE(SIGUSR1); +#endif + +#ifdef SIGSEGV + SIGNO_CASE(SIGSEGV); +#endif + +#ifdef SIGUSR2 + SIGNO_CASE(SIGUSR2); +#endif + +#ifdef SIGPIPE + SIGNO_CASE(SIGPIPE); +#endif + +#ifdef SIGALRM + SIGNO_CASE(SIGALRM); +#endif + + SIGNO_CASE(SIGTERM); + SIGNO_CASE(SIGCHLD); + +#ifdef SIGSTKFLT + SIGNO_CASE(SIGSTKFLT); +#endif + + +#ifdef SIGCONT + SIGNO_CASE(SIGCONT); +#endif + +#ifdef SIGSTOP + SIGNO_CASE(SIGSTOP); +#endif + +#ifdef SIGTSTP + SIGNO_CASE(SIGTSTP); +#endif + +#ifdef SIGTTIN + SIGNO_CASE(SIGTTIN); +#endif + +#ifdef SIGTTOU + SIGNO_CASE(SIGTTOU); +#endif + +#ifdef SIGURG + SIGNO_CASE(SIGURG); +#endif + +#ifdef SIGXCPU + SIGNO_CASE(SIGXCPU); +#endif + +#ifdef SIGXFSZ + SIGNO_CASE(SIGXFSZ); +#endif + +#ifdef SIGVTALRM + SIGNO_CASE(SIGVTALRM); +#endif + +#ifdef SIGPROF + SIGNO_CASE(SIGPROF); +#endif + +#ifdef SIGWINCH + SIGNO_CASE(SIGWINCH); +#endif + +#ifdef SIGIO + SIGNO_CASE(SIGIO); +#endif + +#ifdef SIGPOLL + SIGNO_CASE(SIGPOLL); +#endif + +#ifdef SIGLOST + SIGNO_CASE(SIGLOST); +#endif + +#ifdef SIGPWR + SIGNO_CASE(SIGPWR); +#endif + +#ifdef SIGSYS + SIGNO_CASE(SIGSYS); +#endif + +#ifdef SIGUNUSED + SIGNO_CASE(SIGUNUSED); +#endif + + default: return ""; + } +} + Local ErrnoException(int errorno, const char *syscall, diff --git a/src/node.h b/src/node.h index 5d6bec6e20..bb9e90e9b0 100644 --- a/src/node.h +++ b/src/node.h @@ -80,5 +80,6 @@ v8::Local ErrnoException(int errorno, const char *syscall = NULL, const char *msg = ""); +const char *signo_string(int errorno); } // namespace node #endif // SRC_NODE_H_ diff --git a/src/node_child_process.cc b/src/node_child_process.cc index 5cfd9ff9e2..f490417c0a 100644 --- a/src/node_child_process.cc +++ b/src/node_child_process.cc @@ -1,5 +1,6 @@ // Copyright 2009 Ryan Dahl #include +#include #include #include @@ -244,7 +245,7 @@ int ChildProcess::Spawn(const char *file, } -void ChildProcess::OnExit(int code) { +void ChildProcess::OnExit(int status) { HandleScope scope; pid_ = -1; @@ -258,10 +259,20 @@ void ChildProcess::OnExit(int code) { TryCatch try_catch; - Local argv[1]; - argv[0] = Integer::New(code); + Local argv[2]; + if (WIFEXITED(status)) { + argv[0] = Integer::New(WEXITSTATUS(status)); + } else { + argv[0] = Local::New(Null()); + } + + if (WIFSIGNALED(status)) { + argv[1] = String::NewSymbol(signo_string(WTERMSIG(status))); + } else { + argv[1] = Local::New(Null()); + } - onexit->Call(handle_, 1, argv); + onexit->Call(handle_, 2, argv); if (try_catch.HasCaught()) { FatalException(try_catch); diff --git a/test/fixtures/exit.js b/test/fixtures/exit.js new file mode 100644 index 0000000000..6f85c5d332 --- /dev/null +++ b/test/fixtures/exit.js @@ -0,0 +1 @@ +process.exit(process.argv[2] || 1); \ No newline at end of file diff --git a/test/simple/test-child-process-exit-code.js b/test/simple/test-child-process-exit-code.js new file mode 100644 index 0000000000..3a33dd461b --- /dev/null +++ b/test/simple/test-child-process-exit-code.js @@ -0,0 +1,11 @@ +require("../common"); +var spawn = require('child_process').spawn + , path = require('path') + , sub = path.join(fixturesDir, 'exit.js') + , child = spawn(process.argv[0], [sub, 23]) + ; + +child.addListener('exit', function(code, signal) { + assert.strictEqual(code, 23); + assert.strictEqual(signal, null); +}); \ No newline at end of file diff --git a/test/simple/test-child-process-kill.js b/test/simple/test-child-process-kill.js index d506bef8cb..ccdb238647 100644 --- a/test/simple/test-child-process-kill.js +++ b/test/simple/test-child-process-kill.js @@ -2,7 +2,8 @@ require("../common"); var spawn = require('child_process').spawn; -var exitStatus = -1; +var exitCode; +var termSignal; var gotStdoutEOF = false; var gotStderrEOF = false; @@ -25,14 +26,16 @@ cat.stderr.addListener("end", function () { gotStderrEOF = true; }); -cat.addListener("exit", function (status) { - exitStatus = status; +cat.addListener("exit", function (code, signal) { + exitCode = code; + termSignal = signal; }); cat.kill(); process.addListener("exit", function () { - assert.ok(exitStatus > 0); + assert.strictEqual(exitCode, null); + assert.strictEqual(termSignal, 'SIGTERM'); assert.ok(gotStdoutEOF); assert.ok(gotStderrEOF); }); diff --git a/test/simple/test-exec.js b/test/simple/test-exec.js index c0cfa5c2d1..8e92c98f39 100644 --- a/test/simple/test-exec.js +++ b/test/simple/test-exec.js @@ -23,6 +23,7 @@ exec("ls /DOES_NOT_EXIST", function (err, stdout, stderr) { assert.equal("", stdout); assert.equal(true, err.code != 0); assert.equal(false, err.killed); + assert.strictEqual(null, err.signal); puts("error code: " + err.code); puts("stdout: " + JSON.stringify(stdout)); puts("stderr: " + JSON.stringify(stderr)); @@ -36,11 +37,13 @@ exec("ls /DOES_NOT_EXIST", function (err, stdout, stderr) { exec("sleep 10", { timeout: 50 }, function (err, stdout, stderr) { assert.ok(err); assert.ok(err.killed); + assert.equal(err.signal, 'SIGKILL'); }); exec('python -c "print 200000*\'C\'"', { maxBuffer: 1000 }, function (err, stdout, stderr) { assert.ok(err); assert.ok(err.killed); + assert.equal(err.signal, 'SIGKILL'); }); process.addListener("exit", function () {