Permalink
Browse files

child_process: emit IPC messages on next tick

This commit fixes a regression related to IPC 'message'
events. When messages are not emitted in the next tick,
a 'message' handler that throws can break the IPC read
loop.

Refs: #6909
Refs: #13459
Refs: #13648
PR-URL: #13856
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
  • Loading branch information...
cjihrig committed Jun 21, 2017
1 parent 748b2a8 commit 87d682b69a2ed1d6b74215ee8fbf9af9d6674ee9
Showing with 46 additions and 11 deletions.
  1. +7 −11 lib/internal/child_process.js
  2. +39 −0 test/parallel/test-child-process-ipc-next-tick.js
@@ -457,7 +457,6 @@ function setupChannel(target, channel) {
}
chunks[0] = jsonBuffer + chunks[0];
var nextTick = false;
for (var i = 0; i < numCompleteChunks; i++) {
var message = JSON.parse(chunks[i]);
@@ -466,12 +465,11 @@ function setupChannel(target, channel) {
// that we deliver the handle with the right message however.
if (isInternal(message)) {
if (message.cmd === 'NODE_HANDLE')
handleMessage(message, recvHandle, true, false);
handleMessage(message, recvHandle, true);
else
handleMessage(message, undefined, true, false);
handleMessage(message, undefined, true);
} else {
handleMessage(message, undefined, false, nextTick);
nextTick = true;
handleMessage(message, undefined, false);
}
}
jsonBuffer = incompleteChunk;
@@ -533,7 +531,7 @@ function setupChannel(target, channel) {
// Convert handle object
obj.got.call(this, message, handle, function(handle) {
handleMessage(message.msg, handle, isInternal(message.msg), false);
handleMessage(message.msg, handle, isInternal(message.msg));
});
});
@@ -743,15 +741,13 @@ function setupChannel(target, channel) {
target.emit(event, message, handle);
}
function handleMessage(message, handle, internal, nextTick) {
function handleMessage(message, handle, internal) {
if (!target.channel)
return;
var eventName = (internal ? 'internalMessage' : 'message');
if (nextTick)
process.nextTick(emit, eventName, message, handle);
else
target.emit(eventName, message, handle);
process.nextTick(emit, eventName, message, handle);
}
channel.readStart();
@@ -0,0 +1,39 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const cp = require('child_process');
const NUM_MESSAGES = 10;
const values = [];
for (let i = 0; i < NUM_MESSAGES; ++i) {
values[i] = i;
}
if (process.argv[2] === 'child') {
const received = values.map(() => { return false; });
process.on('uncaughtException', common.mustCall((err) => {
received[err] = true;
const done = received.every((element) => { return element === true; });
if (done)
process.disconnect();
}, NUM_MESSAGES));
process.on('message', (msg) => {
// If messages are handled synchronously, throwing should break the IPC
// message processing.
throw msg;
});
process.send('ready');
} else {
const child = cp.fork(__filename, ['child']);
child.on('message', common.mustCall((msg) => {
assert.strictEqual(msg, 'ready');
values.forEach((value) => {
child.send(value);
});
}));
}

0 comments on commit 87d682b

Please sign in to comment.