Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report status code of child process in term.status property #37

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
language: node_js
node_js:
- "0.10"
- "0.8"
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ term.on('data', function(data) {
console.log(data);
});

term.on('close', function() {
console.log('exit status: %d', term.status);
});

term.write('ls\r');
term.resize(100, 40);
term.write('ls /\r');
Expand Down
4 changes: 3 additions & 1 deletion binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
'src/unix/pty.cc'
],
'libraries': [
'-lutil'
'-lutil',
'-L/usr/lib',
'-L/usr/local/lib'
],
}],
# http://www.gnu.org/software/gnulib/manual/html_node/forkpty.html
Expand Down
4 changes: 4 additions & 0 deletions lib/pty.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ Terminal.prototype.__defineGetter__('process', function() {
return pty.process(this.fd, this.pty) || this.file;
});

Terminal.prototype.__defineGetter__('status', function() {
return pty.status(this.pid);
});

Terminal.prototype._close = function() {
this.socket.writable = false;
this.socket.readable = false;
Expand Down
69 changes: 69 additions & 0 deletions src/unix/pty.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

#include <v8.h>
#include <node.h>
#include <signal.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <sys/wait.h>
#include <fcntl.h>
#include <map>

/* forkpty */
/* http://www.gnu.org/software/gnulib/manual/html_node/forkpty.html */
Expand Down Expand Up @@ -73,6 +76,9 @@ PtyResize(const Arguments&);
static Handle<Value>
PtyGetProc(const Arguments&);

static Handle<Value>
PtyGetStatus(const Arguments&);

static int
pty_execvpe(const char *, char **, char **);

Expand All @@ -92,9 +98,45 @@ pty_forkpty(int *, char *,
const struct termios *,
const struct winsize *);

std::map<int, int> pidMap;

extern "C" void
init(Handle<Object>);

static void (*node_sighandler)(int) = NULL;

void
sigChldHandler(int sig, siginfo_t *sip, void *ctx) {
int status = 0;
pid_t res;
if (pidMap[sip->si_pid] == -302) { // this is one of ours
res = waitpid(sip->si_pid, &status, 0);
pidMap[sip->si_pid] = WEXITSTATUS(status);
} else if (node_sighandler) {
// Can only have one SIGCHLD handler at a time, so we need to call node/libuv's handler.
node_sighandler(sig);
}
}

void
check_sigchld() {
// retrieve node/libuv's SIGCHLD handler.
struct sigaction node_action;
node_action.sa_flags = 0;
sigaction(SIGCHLD, NULL, &node_action);
if (node_action.sa_sigaction == sigChldHandler) {
return; // it's already installed
}
node_sighandler = node_action.sa_handler;
struct sigaction action;
memset (&action, '\0', sizeof(action));
action.sa_sigaction = sigChldHandler;
action.sa_flags = SA_SIGINFO;
action.sa_flags |= SA_NOCLDSTOP;
// set new SIGCHLD handler. this will call node/libuv's handler at the end.
sigaction(SIGCHLD, &action, NULL);
}

/**
* PtyFork
* pty.fork(file, args, env, cwd, cols, rows)
Expand Down Expand Up @@ -210,6 +252,8 @@ PtyFork(const Arguments& args) {
obj->Set(String::New("fd"), Number::New(master));
obj->Set(String::New("pid"), Number::New(pid));
obj->Set(String::New("pty"), String::New(name));
pidMap[pid] = -302;
check_sigchld();

return scope.Close(obj);
}
Expand Down Expand Up @@ -334,6 +378,28 @@ PtyGetProc(const Arguments& args) {
return scope.Close(name_);
}

/**
* PtyGetStatus
* Foreground Process Name
* pty.status(pid)
*/

static Handle<Value>
PtyGetStatus(const Arguments& args) {
HandleScope scope;

if (args.Length() != 1
|| !args[0]->IsNumber()) {
return ThrowException(Exception::Error(
String::New("Usage: pty.status(pid)")));
}

int pid = args[0]->IntegerValue();

Local<Number> statusCode = Number::New(pidMap[pid]);
return scope.Close(statusCode);
}

/**
* execvpe
*/
Expand Down Expand Up @@ -546,17 +612,20 @@ pty_forkpty(int *amaster, char *name,
#endif
}


/**
* Init
*/

extern "C" void
init(Handle<Object> target) {
HandleScope scope;
check_sigchld();
NODE_SET_METHOD(target, "fork", PtyFork);
NODE_SET_METHOD(target, "open", PtyOpen);
NODE_SET_METHOD(target, "resize", PtyResize);
NODE_SET_METHOD(target, "process", PtyGetProc);
NODE_SET_METHOD(target, "status", PtyGetStatus);
}

NODE_MODULE(pty, init)
1 change: 1 addition & 0 deletions test/children/exitCode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.exit(5);
32 changes: 24 additions & 8 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,38 @@ var tests = [
name: 'should be correctly setup',
command: [ 'children/void.js' ],
options: { cwd: __dirname },
exitCode: 0,
test: function () {
assert.equal(this.file, process.execPath);
}
}, {
name: 'should support stdin',
command: [ 'children/stdin.js' ],
options: { cwd: __dirname },
exitCode: 0,
test: function () {
this.write('☃');
}
}, {
name: 'should support resize',
command: [ 'children/resize.js' ],
options: { cwd: __dirname },
exitCode: 0,
test: function () {
this.resize(100, 100);
}
}, {
name: 'should change uid/gid',
command: [ 'children/uidgid.js' ],
options: { cwd: __dirname, uid: 777, gid: 777 },
exitCode: 0,
test: function () {}
}, {
name: 'should report exitCode',
command: [ 'children/exitCode.js' ],
options: { cwd: __dirname },
test: function () {},
exitCode: 5
}
];

Expand All @@ -42,14 +52,8 @@ describe('Pty', function() {
var term = pty.fork(process.execPath, testCase.command, testCase.options);
term.pipe(process.stderr);

// any output is considered failure. this is only a workaround
// until the actual error code is passed through
var count = 0;
term.on('data', function (data) {
count++;
});
term.on('exit', function () {
assert.equal(count, 0);
term.on('close', function () {
assert.equal(term.status, testCase.exitCode);
done();
});

Expand All @@ -58,3 +62,15 @@ describe('Pty', function() {
});
});
});

describe('The SIGCHLD handler', function () {
it('should not interfere with child_process', function (done) {
this.timeout(500);
var spawn = require('child_process').spawn;
var proc = spawn('false')
proc.on('close', function (code) {
assert.equal(code, 1);
done();
});
});
});