child_process: fix handleless NODE_HANDLE handling

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

Author: santigimeno

lib/internal/child_process.js

@@ -23,6 +23,8 @@ const errnoException = util._errnoException;
 constSocketListSend=SocketList.SocketListSend;
 constSocketListReceive=SocketList.SocketListReceive;
 
+constMAX_HANDLE_RETRANSMISSIONS=3;
+
 // this object contain function to convert TCP objects to native handle objects
 // and back again.
 consthandleConversion={
@@ -88,17 +90,18 @@ const handleConversion ={
 returnhandle;
 },
 
-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){
 returnfalse;
 }
 
+functionclosePendingHandle(target){
+target._pendingMessage.handle.close();
+target._pendingMessage=null;
+}
+
 
 ChildProcess.prototype.spawn=function(options){
 varipc;
@@ -434,7 +442,7 @@ function setupChannel(target, channel){
 });
 
 target._handleQueue=null;
-target._pendingHandle=null;
+target._pendingMessage=null;
 
 constcontrol=newControl(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);
+}elseif(target._pendingMessage.retransmissions++===
+MAX_HANDLE_RETRANSMISSIONS){
+closePendingHandle(target);
+process.emitWarning('Handle did not reach the receiving process '+
+'correctly','SentHandleNotReceivedWarning');
+}
 }
 
 assert(Array.isArray(target._handleQueue));
 varqueue=target._handleQueue;
 target._handleQueue=null;
 
+if(target._pendingMessage){
+target._send(target._pendingMessage.message,
+target._pendingMessage.handle,
+target._pendingMessage.options,
+target._pendingMessage.callback);
+}
+
 for(vari=0;i<queue.length;i++){
 varargs=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)
+returntarget._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);
 }
 }elseif(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){
 constex=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);
 
 varfired=false;
 functionfinish(){