Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

child_process: fix handleless NODE_HANDLE handling #13235

Merged
merged 1 commit into from
Jul 1, 2017
Merged

child_process: fix handleless NODE_HANDLE handling #13235

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions lib/internal/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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 = {
Expand Down Expand Up @@ -89,17 +91,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 @@ -250,6 +253,11 @@ function getHandleWrapType(stream) {
return false;
}

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


ChildProcess.prototype.spawn = function(options) {
var ipc;
Expand Down Expand Up @@ -435,7 +443,7 @@ function setupChannel(target, channel) {
});

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

const control = new Control(channel);

Expand Down Expand Up @@ -491,16 +499,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 @@ -515,6 +538,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 @@ -625,7 +654,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 @@ -647,7 +677,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) {
Expand All @@ -663,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 @@ -712,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