Unexpected child_process + stdout behavior change from 0.6.5 to 0.6.6

[Hainish]

When starting a child process with spawn, a self-exit from the parent process and a subsequent write to stdout from the child causes the child to crash unexpectedly. Bug emerged in 0.6.6, Linux x64.

Observed when running process.js:

process.js

var spawn = require("child_process").spawn;

var child = spawn("node", ['child.js']);
child.stdout.on('data', function(data){
  if(data.toString().indexOf("Child started") != -1){
    console.log("Child process started");
    process.exit();
  }
});

child.js

console.log('Child started');

var spawn = require("child_process").spawn;

var grandchild = spawn('node', ['grandchild.js']);
grandchild.stdout.on('data', function(data){
  process.stdout.write('I ' + data);
});

grandchild.js

var x =0;
setInterval(function(){
  console.log(x);
  x++;
}, 1000);

[bnoordhuis]

13324bf seems to be the commit that breaks your test case. Will investigate.

[piscisaureus]

On Windows I get the following message. Is that the same as you guys?

node.js:284
        throw new Error('process.stdout cannot be closed');
              ^
Error: process.stdout cannot be closed
    at Socket.<anonymous> (node.js:284:15)
    at Object.afterWrite [as oncomplete] (net.js:478:10)

The code using to test:

var spawn = require('child_process').spawn;
spawn('node', ['-e', 'setTimeout(function(){console.log("fooo");}, 2000)'], {customFds:[-1,-1,2]});
process.exit(0);

[bnoordhuis]

On Windows I get the following message. Is that the same as you guys?

Yes. The error is thrown by the child process.

[piscisaureus]

Apparently the fd that the child process uses becomes invalid because the master dies (this is expected).

After a stdout write fails, net.js tries to close it with an error status. However closing stdout is now forbidden and makes the program crash.

[isaacs]

Can we just have that error not crash the program? Ie, instead of throwing, just do nothing?

[bnoordhuis]

That's what it was doing before 13324bf so the question is why that behaviour was changed. @igorzi?

[igorzi]

I believe this was done to make it explicit that closing stdout/stderr shouldn't be done (if it's just a noop then people would wrongfully assume that underlying handle actually got closed). Perhaps net.js should try looking at this._isStdio, and not try to close it if it's stdio handle?