From 16f2600ecf13e01424504e8d0eb33c62fb6db720 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Wed, 21 Jun 2017 14:40:50 -0400
Subject: [PATCH] child_process: emit IPC messages on next tick

This commit fixes a regression related to IPC 'message'
events. When messages are not emitted in the next tick,
a 'message' handler that throws can break the IPC read
loop.

Refs: https://github.com/nodejs/node/pull/6909
Refs: https://github.com/nodejs/node/pull/13459
Refs: https://github.com/nodejs/node/pull/13648
PR-URL: https://github.com/nodejs/node/pull/13856
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
---
 lib/internal/child_process.js                 | 18 ++++-----
 .../test-child-process-ipc-next-tick.js       | 39 +++++++++++++++++++
 2 files changed, 46 insertions(+), 11 deletions(-)
 create mode 100644 test/parallel/test-child-process-ipc-next-tick.js

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
index 25190277e2e076..725bcf2da4b382 100644
--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@@ -456,7 +456,6 @@ function setupChannel(target, channel) {
       }
       chunks[0] = jsonBuffer + chunks[0];
 
-      var nextTick = false;
       for (var i = 0; i < numCompleteChunks; i++) {
         var message = JSON.parse(chunks[i]);
 
@@ -465,12 +464,11 @@ function setupChannel(target, channel) {
         // that we deliver the handle with the right message however.
         if (isInternal(message)) {
           if (message.cmd === 'NODE_HANDLE')
-            handleMessage(message, recvHandle, true, false);
+            handleMessage(message, recvHandle, true);
           else
-            handleMessage(message, undefined, true, false);
+            handleMessage(message, undefined, true);
         } else {
-          handleMessage(message, undefined, false, nextTick);
-          nextTick = true;
+          handleMessage(message, undefined, false);
         }
       }
       jsonBuffer = incompleteChunk;
@@ -532,7 +530,7 @@ function setupChannel(target, channel) {
 
     // Convert handle object
     obj.got.call(this, message, handle, function(handle) {
-      handleMessage(message.msg, handle, isInternal(message.msg), false);
+      handleMessage(message.msg, handle, isInternal(message.msg));
     });
   });
 
@@ -742,15 +740,13 @@ function setupChannel(target, channel) {
     target.emit(event, message, handle);
   }
 
-  function handleMessage(message, handle, internal, nextTick) {
+  function handleMessage(message, handle, internal) {
     if (!target.channel)
       return;
 
     var eventName = (internal ? 'internalMessage' : 'message');
-    if (nextTick)
-      process.nextTick(emit, eventName, message, handle);
-    else
-      target.emit(eventName, message, handle);
+
+    process.nextTick(emit, eventName, message, handle);
   }
 
   channel.readStart();
diff --git a/test/parallel/test-child-process-ipc-next-tick.js b/test/parallel/test-child-process-ipc-next-tick.js
new file mode 100644
index 00000000000000..b23aefc85d11d8
--- /dev/null
+++ b/test/parallel/test-child-process-ipc-next-tick.js
@@ -0,0 +1,39 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const cp = require('child_process');
+const NUM_MESSAGES = 10;
+const values = [];
+
+for (let i = 0; i < NUM_MESSAGES; ++i) {
+  values[i] = i;
+}
+
+if (process.argv[2] === 'child') {
+  const received = values.map(() => { return false; });
+
+  process.on('uncaughtException', common.mustCall((err) => {
+    received[err] = true;
+    const done = received.every((element) => { return element === true; });
+
+    if (done)
+      process.disconnect();
+  }, NUM_MESSAGES));
+
+  process.on('message', (msg) => {
+    // If messages are handled synchronously, throwing should break the IPC
+    // message processing.
+    throw msg;
+  });
+
+  process.send('ready');
+} else {
+  const child = cp.fork(__filename, ['child']);
+
+  child.on('message', common.mustCall((msg) => {
+    assert.strictEqual(msg, 'ready');
+    values.forEach((value) => {
+      child.send(value);
+    });
+  }));
+}