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.

Fixes: #9706
PR-URL: #13235
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: santigimeno

lib/internal/child_process.js

@@ -29,6 +29,9 @@ module.exports = {
   getSocketList
 };
 
+const MAX_HANDLE_RETRANSMISSIONS = 3;
+
+
 // this object contain function to convert TCP objects to native handle objects
 // and back again.
 const handleConversion = {
@@ -94,17 +97,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();
         }
@@ -267,6 +271,11 @@ function getHandleWrapType(stream) {
   return false;
 }
 
+function closePendingHandle(target) {
+  target._pendingMessage.handle.close();
+  target._pendingMessage = null;
+}
+
 
 ChildProcess.prototype.spawn = function(options) {
   const self = this;
@@ -437,7 +446,7 @@ class Control extends EventEmitter {
 function setupChannel(target, channel) {
   target._channel = channel;
   target._handleQueue = null;
-  target._pendingHandle = null;
+  target._pendingMessage = null;
 
   const control = new Control(channel);
 
@@ -489,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);
@@ -513,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
@@ -623,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,
@@ -645,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);
       }
 
       req.oncomplete = function() {
@@ -662,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');
@@ -711,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() {