Skip to content

Commit

Permalink
child_process: fix handleless NODE_HANDLE handling
Browse files Browse the repository at this point in the history 
It is possible that `recvmsg()` may return an error on ancillary data
reception when receiving a `NODE_HANDLE` message (for example
`MSG_CTRUNC`). This would end up, if the handle type was `net.Socket`,
on a `message` event with a non null but invalid `sendHandle`. To
improve the situation, send a `NODE_HANDLE_NACK` that'll cause the
sending process to retransmit the message again. In case the same
message is retransmitted 3 times without success, close the handle and
print a warning.

Fixes: #9706
PR-URL: #13235
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
@santigimeno @MylesBorins
santigimeno authored and MylesBorins committed Aug 16, 2017
1 parent 0bcbcca commit 2bc7c3a
Showing 1 changed file with 45 additions and 16 deletions.
61 changes: 45 additions & 16 deletions lib/internal/child_process.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ module.exports = {
getSocketList
};

const MAX_HANDLE_RETRANSMISSIONS = 3;


// this object contain function to convert TCP objects to native handle objects
// and back again.
const handleConversion = {
Expand Down Expand Up @@ -94,17 +97,18 @@ const handleConversion = {
return handle;
},

postSend: function(handle, options, target) {
postSend: function(message, handle, options, callback, target) {
// Store the handle after successfully sending it, so it can be closed
// when the NODE_HANDLE_ACK is received. If the handle could not be sent,
// just close it.
if (handle && !options.keepOpen) {
if (target) {
// There can only be one _pendingHandle as passing handles are
// There can only be one _pendingMessage as passing handles are
// processed one at a time: handles are stored in _handleQueue while
// waiting for the NODE_HANDLE_ACK of the current passing handle.
assert(!target._pendingHandle);
target._pendingHandle = handle;
assert(!target._pendingMessage);
target._pendingMessage =
{ callback, message, handle, options, retransmissions: 0 };
} else {
handle.close();
}
Expand Down Expand Up @@ -267,6 +271,11 @@ function getHandleWrapType(stream) {
return false;
}

function closePendingHandle(target) {
target._pendingMessage.handle.close();
target._pendingMessage = null;
}


ChildProcess.prototype.spawn = function(options) {
const self = this;
Expand Down Expand Up @@ -437,7 +446,7 @@ class Control extends EventEmitter {
function setupChannel(target, channel) {
target._channel = channel;
target._handleQueue = null;
target._pendingHandle = null;
target._pendingMessage = null;

const control = new Control(channel);

Expand Down Expand Up @@ -489,16 +498,31 @@ function setupChannel(target, channel) {
// handlers will go through this
target.on('internalMessage', function(message, handle) {
// Once acknowledged - continue sending handles.
if (message.cmd === 'NODE_HANDLE_ACK') {
if (target._pendingHandle) {
target._pendingHandle.close();
target._pendingHandle = null;
if (message.cmd === 'NODE_HANDLE_ACK' ||
message.cmd === 'NODE_HANDLE_NACK') {

if (target._pendingMessage) {
if (message.cmd === 'NODE_HANDLE_ACK') {
closePendingHandle(target);
} else if (target._pendingMessage.retransmissions++ ===
MAX_HANDLE_RETRANSMISSIONS) {
closePendingHandle(target);
process.emitWarning('Handle did not reach the receiving process ' +
'correctly', 'SentHandleNotReceivedWarning');
}
}

assert(Array.isArray(target._handleQueue));
var queue = target._handleQueue;
target._handleQueue = null;

if (target._pendingMessage) {
target._send(target._pendingMessage.message,
target._pendingMessage.handle,
target._pendingMessage.options,
target._pendingMessage.callback);
}

for (var i = 0; i < queue.length; i++) {
var args = queue[i];
target._send(args.message, args.handle, args.options, args.callback);
Expand All @@ -513,6 +537,12 @@ function setupChannel(target, channel) {

if (message.cmd !== 'NODE_HANDLE') return;

// It is possible that the handle is not received because of some error on
// ancillary data reception such as MSG_CTRUNC. In this case, report the
// sender about it by sending a NODE_HANDLE_NACK message.
if (!handle)
return target._send({ cmd: 'NODE_HANDLE_NACK' }, null, true);

// 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
Expand Down Expand Up @@ -623,7 +653,8 @@ function setupChannel(target, channel) {
net._setSimultaneousAccepts(handle);
}
} else if (this._handleQueue &&
!(message && message.cmd === 'NODE_HANDLE_ACK')) {
!(message && (message.cmd === 'NODE_HANDLE_ACK' ||
message.cmd === 'NODE_HANDLE_NACK'))) {
// Queue request anyway to avoid out-of-order messages.
this._handleQueue.push({
callback: callback,
Expand All @@ -645,7 +676,7 @@ function setupChannel(target, channel) {
if (!this._handleQueue)
this._handleQueue = [];
if (obj && obj.postSend)
obj.postSend(handle, options, target);
obj.postSend(message, handle, options, callback, target);
}

req.oncomplete = function() {
Expand All @@ -662,7 +693,7 @@ function setupChannel(target, channel) {
} else {
// Cleanup handle on error
if (obj && obj.postSend)
obj.postSend(handle, options);
obj.postSend(message, handle, options, callback);

if (!options.swallowErrors) {
const ex = errnoException(err, 'write');
Expand Down Expand Up @@ -711,10 +742,8 @@ function setupChannel(target, channel) {
// This marks the fact that the channel is actually disconnected.
this._channel = null;

if (this._pendingHandle) {
this._pendingHandle.close();
this._pendingHandle = null;
}
if (this._pendingMessage)
closePendingHandle(this);

var fired = false;
function finish() {
Expand Down

0 comments on commit 2bc7c3a

Please sign in to comment.