child_process: workaround fd passing issue on OS X

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: #7512 PR-URL: #7572 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

Author: santigimeno

lib/internal/child_process.js

@@ -94,10 +94,21 @@ const handleConversion ={
 returnhandle;
 },
 
-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){
@@ -400,6 +411,7 @@ ChildProcess.prototype.unref = function(){
 functionsetupChannel(target,channel){
 target._channel=channel;
 target._handleQueue=null;
+target._pendingHandle=null;
 
 constcontrol=newclassextendsEventEmitter{
 constructor(){
@@ -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));
 varqueue=target._handleQueue;
 target._handleQueue=null;
@@ -610,13 +627,16 @@ function setupChannel(target, channel){
 varerr=channel.writeUtf8String(req,string,handle);
 
 if(err===0){
-if(handle&&!this._handleQueue)
-this._handleQueue=[];
+if(handle){
+if(!this._handleQueue)
+this._handleQueue=[];
+if(obj&&obj.postSend)
+obj.postSend(handle,options,target);
+}
+
 req.oncomplete=function(){
 if(this.async===true)
 control.unref();
-if(obj&&obj.postSend)
-obj.postSend(handle,options);
 if(typeofcallback==='function')
 callback(null);
 };
@@ -677,6 +697,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;
+}
+
 varfired=false;
 functionfinish(){
 if(fired)return;

test/sequential/test-child-process-pass-fd.js

@@ -0,0 +1,53 @@
+'use strict';
+constcommon=require('../common');
+constassert=require('assert');
+constfork=require('child_process').fork;
+constnet=require('net');
+
+if((process.config.variables.arm_version==='6')||
+(process.config.variables.arm_version==='7')){
+common.skip('Too slow for armv6 and armv7 bots');
+return;
+}
+
+constN=80;
+
+if(process.argv[2]!=='child'){
+for(leti=0;i<N;++i){
+constworker=fork(__filename,['child',common.PORT+i]);
+worker.once('message',common.mustCall((msg,handle)=>{
+assert.strictEqual(msg,'handle');
+assert.ok(handle);
+worker.send('got');
+
+letrecvData='';
+handle.on('data',common.mustCall((data)=>{
+recvData+=data;
+}));
+
+handle.on('end',()=>{
+assert.strictEqual(recvData,'hello');
+worker.kill();
+});
+}));
+}
+}else{
+letsocket;
+constport=process.argv[3];
+letcbcalls=0;
+functionsocketConnected(){
+if(++cbcalls===2)
+process.send('handle',socket);
+}
+
+constserver=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);
+});
+}