fixing interoperability with native child_process. also added tests

src/unix/pty.cc

@@ -109,12 +109,13 @@ void
 sigChldHandler(int sig, siginfo_t *sip, void *ctx) {
     int status = 0;
     pid_t res;
-    res = waitpid(sip->si_pid, &status, 0);
-    if (res != 0) {
-      pidMap[sip->si_pid] = WEXITSTATUS(status);
-    }
-    // Can only have one SIGCHLD handler at a time, so we need to call node/libuv's handler.
-    if (node_sighandler) {
+    if (pidMap[sip->si_pid] == -302) { // this is one of ours
+      res = waitpid(sip->si_pid, &status, 0);
+      if (res != 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);
     }
 }
@@ -234,6 +235,7 @@ 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;
 
       return scope.Close(obj);
   }

test/index.js

@@ -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
   }
 ];
 
@@ -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();
       });
 
@@ -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();
+    });
+  });
+});