1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

child_process: don't crash process on internal ops

Browse Source 
1. Swallow errors when sending internal NODE_HANDLE_ACK messages, so
   they don't crash the process.
2. Queue process.disconnect() if there are any pending queued messages.

Fixes test-child-process-fork-net2.js on win.
This commit is contained in:
Alexis Campailla 2013-11-21 18:49:41 -08:00 committed by Trevor Norris
parent 06b1945298
commit 0da4e0e843
2 changed files with 57 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
lib/child_process.js
View File

@ -369,16 +369,25 @@ function setupChannel(target, channel) {
assert(util.isArray(target._handleQueue));
var queue = target._handleQueue;
target._handleQueue = null;
queue.forEach(function(args) {
target.send(args.message, args.handle);
target._send(args.message, args.handle, false);
});
// Process a pending disconnect (if any).
if (!target.connected && target._channel && !target._handleQueue)
target._disconnect();
return;
}
if (message.cmd !== 'NODE_HANDLE') return;
// Acknowledge handle receival.
target.send({ cmd: 'NODE_HANDLE_ACK' });
// Acknowledge handle receival. Don't emit error events (for example if
// the other side has disconnected) because this call to send() is not
// initiated by the user and it shouldn't be fatal to be unable to ACK
// a message.
target._send({ cmd: 'NODE_HANDLE_ACK' }, null, true);
var obj = handleConversion[message.type];
@ -395,14 +404,17 @@ function setupChannel(target, channel) {
});
target.send = function(message, handle) {
if (util.isUndefined(message)) {
throw new TypeError('message cannot be undefined');
}
if (!this.connected) {
if (!this.connected)
this.emit('error', new Error('channel closed'));
return;
}
else
this._send(message, handle, false);
};
target._send = function(message, handle, swallowErrors) {
assert(this.connected || this._channel);
if (util.isUndefined(message))
throw new TypeError('message cannot be undefined');
// package messages with a handle object
if (handle) {
@ -454,7 +466,8 @@ function setupChannel(target, channel) {
var err = channel.writeUtf8String(req, string, handle);
if (err) {
this.emit('error', errnoException(err, 'write'));
if (!swallowErrors)
this.emit('error', errnoException(err, 'write'));
} else if (handle && !this._handleQueue) {
this._handleQueue = [];
}
@ -467,15 +480,37 @@ function setupChannel(target, channel) {
return channel.writeQueueSize < (65536 * 2);
};
// connected will be set to false immediately when a disconnect() is
// requested, even though the channel might still be alive internally to
// process queued messages. The three states are distinguished as follows:
// - disconnect() never requested: _channel is not null and connected
// is true
// - disconnect() requested, messages in the queue: _channel is not null
// and connected is false
// - disconnect() requested, channel actually disconnected: _channel is
// null and connected is false
target.connected = true;
target.disconnect = function() {
if (!this.connected) {
this.emit('error', new Error('IPC channel is already disconnected'));
return;
}
// do not allow messages to be written
// Do not allow any new messages to be written.
this.connected = false;
// If there are no queued messages, disconnect immediately. Otherwise,
// postpone the disconnect so that it happens internally after the
// queue is flushed.
if (!this._handleQueue)
this._disconnect();
}
target._disconnect = function() {
assert(this._channel);
// This marks the fact that the channel is actually disconnected.
this._channel = null;
var fired = false;

14
test/simple/test-child-process-fork-net2.js
View File

@ -52,8 +52,8 @@ if (process.argv[2] === 'child') {
needEnd.push(socket);
}
socket.on('close', function() {
console.error('[%d] socket.close', id, m);
socket.on('close', function(had_error) {
console.error('[%d] socket.close', id, had_error, m);
});
socket.on('finish', function() {
@ -65,7 +65,7 @@ if (process.argv[2] === 'child') {
if (m !== 'close') return;
console.error('[%d] got close message', id);
needEnd.forEach(function(endMe, i) {
console.error('[%d] ending %d', id, i);
console.error('[%d] ending %d/%d', id, i, needEnd.length);
endMe.end('end');
});
});
@ -73,7 +73,7 @@ if (process.argv[2] === 'child') {
process.on('disconnect', function() {
console.error('[%d] process disconnect, ending', id);
needEnd.forEach(function(endMe, i) {
console.error('[%d] ending %d', id, i);
console.error('[%d] ending %d/%d', id, i, needEnd.length);
endMe.end('end');
});
});
@ -120,6 +120,11 @@ if (process.argv[2] === 'child') {
var j = count, client;
while (j--) {
client = net.connect(common.PORT, '127.0.0.1');
client.on('error', function() {
// This can happen if we kill the child too early.
// The client should still get a close event afterwards.
console.error('[m] CLIENT: error event');
});
client.on('close', function() {
console.error('[m] CLIENT: close event');
disconnected += 1;
@ -136,6 +141,7 @@ if (process.argv[2] === 'child') {
console.error('[m] server close');
closeEmitted = true;
console.error('[m] killing child processes');
child1.kill();
child2.kill();
child3.kill();