Skip to content

Commit

Permalink
child_process: fix handle passing w large payloads
Browse files Browse the repository at this point in the history
Fix situations in which the handle passed along with a message
that has a large payload and can’t be read entirely by a single
`recvmsg()` call isn’t associated with the message to which it
belongs.

PR-URL: #14588
Fixes: #13778
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
  • Loading branch information
addaleax authored and jasnell committed Aug 8, 2017
1 parent 9564c20 commit 611851d
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
12 changes: 9 additions & 3 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,14 @@ function setupChannel(target, channel) {

var decoder = new StringDecoder('utf8');
var jsonBuffer = '';
var pendingHandle = null;
channel.buffering = false;
channel.onread = function(nread, pool, recvHandle) {
// TODO(bnoordhuis) Check that nread > 0.
if (pool) {
if (recvHandle)
pendingHandle = recvHandle;

// Linebreak is used as a message end sign
var chunks = decoder.write(pool).split('\n');
var numCompleteChunks = chunks.length - 1;
Expand All @@ -478,10 +482,12 @@ function setupChannel(target, channel) {
// read because SCM_RIGHTS messages don't get coalesced. Make sure
// that we deliver the handle with the right message however.
if (isInternal(message)) {
if (message.cmd === 'NODE_HANDLE')
handleMessage(message, recvHandle, true);
else
if (message.cmd === 'NODE_HANDLE') {
handleMessage(message, pendingHandle, true);
pendingHandle = null;
} else {
handleMessage(message, undefined, true);
}
} else {
handleMessage(message, undefined, false);
}
Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-cluster-send-handle-large-payload.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const cluster = require('cluster');
const net = require('net');

const payload = 'a'.repeat(800004);

if (cluster.isMaster) {
const server = net.createServer();

server.on('connection', common.mustCall((socket) => socket.unref()));

const worker = cluster.fork();
worker.on('message', common.mustCall(({ payload: received }, handle) => {
assert.strictEqual(payload, received);
assert(handle instanceof net.Socket);
server.close();
handle.destroy();
}));

server.listen(0, common.mustCall(() => {
const port = server.address().port;
const socket = new net.Socket();
socket.connect(port, (err) => {
assert.ifError(err);
worker.send({ payload }, socket);
});
}));
} else {
process.on('message', common.mustCall(({ payload: received }, handle) => {
assert.strictEqual(payload, received);
assert(handle instanceof net.Socket);
process.send({ payload }, handle);

// Prepare for a clean exit.
process.channel.unref();
handle.unref();
}));
}

0 comments on commit 611851d

Please sign in to comment.