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.

PR-URL: nodejs#13235
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
santigimeno authored and addaleax committed Jul 3, 2017
1 parent 8cb061a commit c2b8556
Showing 1 changed file with 44 additions and 16 deletions.
60 changes: 44 additions & 16 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
@@ -23,6 +23,8 @@ const errnoException = util._errnoException;
const SocketListSend = SocketList.SocketListSend;
const SocketListReceive = SocketList.SocketListReceive;

const MAX_HANDLE_RETRANSMISSIONS = 3;

// this object contain function to convert TCP objects to native handle objects
// and back again.
const handleConversion = {
@@ -88,17 +90,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();
}
@@ -249,6 +252,11 @@ function getHandleWrapType(stream) {
return false;
}

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


ChildProcess.prototype.spawn = function(options) {
var ipc;
@@ -434,7 +442,7 @@ function setupChannel(target, channel) {
});

target._handleQueue = null;
target._pendingHandle = null;
target._pendingMessage = null;

const control = new Control(channel);

@@ -490,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);
@@ -514,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
@@ -624,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,
@@ -646,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);
}

if (req.async) {
@@ -662,7 +692,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');
@@ -711,10 +741,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() {

0 comments on commit c2b8556

Please sign in to comment.