Skip to content

Commit

Permalink
child_process: workaround fd passing issue on OS X
Browse files Browse the repository at this point in the history
There's an issue on some `OS X` versions when passing fd's between processes.
When the handle associated to a specific file descriptor is closed by the sender
process before it's received in the destination, the handle is indeed closed
while it should remain opened. In order to fix this behaviour, don't close the
handle until the `NODE_HANDLE_ACK` is received by the sender.
Added `test-child-process-pass-fd` that is basically `test-cluster-net-send` but
creating lots of workers, so the issue reproduces on `OS X` consistently.

Fixes: nodejs#7512
  • Loading branch information
santigimeno committed Jul 26, 2016
1 parent 392c70a commit 05d27e4
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 5 deletions.
32 changes: 27 additions & 5 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,21 @@ const handleConversion = {
return handle;
},

postSend: function(handle, options) {
// Close the Socket handle after sending it
if (handle && !options.keepOpen)
handle.close();
postSend: function(handle, options, 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
// 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;
} else {
handle.close();
}
}
},

got: function(message, handle, emit) {
Expand Down Expand Up @@ -400,6 +411,7 @@ ChildProcess.prototype.unref = function() {
function setupChannel(target, channel) {
target._channel = channel;
target._handleQueue = null;
target._pendingHandle = null;

const control = new class extends EventEmitter {
constructor() {
Expand Down Expand Up @@ -465,6 +477,11 @@ function setupChannel(target, channel) {
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;
}

assert(Array.isArray(target._handleQueue));
var queue = target._handleQueue;
target._handleQueue = null;
Expand Down Expand Up @@ -616,7 +633,7 @@ function setupChannel(target, channel) {
if (this.async === true)
control.unref();
if (obj && obj.postSend)
obj.postSend(handle, options);
obj.postSend(handle, options, target);
if (typeof callback === 'function')
callback(null);
};
Expand Down Expand Up @@ -677,6 +694,11 @@ 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;
}

var fired = false;
function finish() {
if (fired) return;
Expand Down
47 changes: 47 additions & 0 deletions test/sequential/test-child-process-pass-fd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const fork = require('child_process').fork;
const net = require('net');

const N = 80;

if (process.argv[2] !== 'child') {
for (let i = 0; i < N; ++i) {
const worker = fork(__filename, ['child', common.PORT + i]);
worker.once('message', common.mustCall((msg, handle) => {
assert.strictEqual(msg, 'handle');
assert.ok(handle);
worker.send('got');

let recvData = '';
handle.on('data', common.mustCall((data) => {
recvData += data;
}));

handle.on('end', () => {
assert.strictEqual(recvData, 'hello');
worker.kill();
});
}));
}
} else {
let socket;
const port = process.argv[3];
let cbcalls = 0;
function socketConnected() {
if (++cbcalls === 2)
process.send('handle', socket);
}

const server = net.createServer((c) => {
process.once('message', function(msg) {
assert.strictEqual(msg, 'got');
c.end('hello');
});
socketConnected();
});
server.listen(port, common.localhostIPv4, () => {
socket = net.connect(port, common.localhostIPv4, socketConnected);
});
}

0 comments on commit 05d27e4

Please sign in to comment.