From c01bedbadda7cd6fa7a761a1b526307862f8ef01 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 24 Dec 2015 13:00:29 -0800 Subject: [PATCH 1/3] child_process: guard against race condition It is possible that the internal hnadleMessage() might try to send to a channel that has been closed. The result can be an AssertionError. Guard against this. Fixes: https://github.com/nodejs/node/issues/4205 --- lib/internal/child_process.js | 3 +++ test/parallel/test-cluster-disconnect-race.js | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/parallel/test-cluster-disconnect-race.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index cd5313d65ce457..8757a78f05e885 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -675,6 +675,9 @@ function setupChannel(target, channel) { const INTERNAL_PREFIX = 'NODE_'; function handleMessage(target, message, handle) { + if (!target._channel) + return; + var eventName = 'message'; if (message !== null && typeof message === 'object' && diff --git a/test/parallel/test-cluster-disconnect-race.js b/test/parallel/test-cluster-disconnect-race.js new file mode 100644 index 00000000000000..878ca393bb89d6 --- /dev/null +++ b/test/parallel/test-cluster-disconnect-race.js @@ -0,0 +1,27 @@ +'use strict'; + +// Ref: https://github.com/nodejs/node/issues/4205 + +const common = require('../common'); +const net = require('net'); +const cluster = require('cluster'); +cluster.schedulingPolicy = cluster.SCHED_NONE; + +if (cluster.isMaster) { + var worker1, worker2; + + worker1 = cluster.fork(); + worker1.on('message', common.mustCall(function() { + worker2 = cluster.fork(); + worker1.disconnect(); + worker2.on('online', common.mustCall(worker2.disconnect)); + })); + + return; +} + +var server = net.createServer(); + +server.listen(common.PORT, function() { + process.send('listening'); +}); From 7d98eae191da3c58bc92e03bccbd390e34a2db15 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 24 Dec 2015 13:06:39 -0800 Subject: [PATCH 2/3] fixup: add comment --- test/parallel/test-cluster-disconnect-race.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-cluster-disconnect-race.js b/test/parallel/test-cluster-disconnect-race.js index 878ca393bb89d6..c56087d74ac175 100644 --- a/test/parallel/test-cluster-disconnect-race.js +++ b/test/parallel/test-cluster-disconnect-race.js @@ -1,5 +1,6 @@ 'use strict'; +// This code triggers an AssertionError on Linux in Node.js 5.3.0 and earlier. // Ref: https://github.com/nodejs/node/issues/4205 const common = require('../common'); From 39df518c2c1e964841758e7fbe8f42ccfb0c4053 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 24 Dec 2015 13:32:49 -0800 Subject: [PATCH 3/3] fixup: throw assertion error if there is a worker error --- test/parallel/test-cluster-disconnect-race.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/parallel/test-cluster-disconnect-race.js b/test/parallel/test-cluster-disconnect-race.js index c56087d74ac175..40cfd919e3e964 100644 --- a/test/parallel/test-cluster-disconnect-race.js +++ b/test/parallel/test-cluster-disconnect-race.js @@ -4,6 +4,7 @@ // Ref: https://github.com/nodejs/node/issues/4205 const common = require('../common'); +const assert = require('assert'); const net = require('net'); const cluster = require('cluster'); cluster.schedulingPolicy = cluster.SCHED_NONE; @@ -18,6 +19,10 @@ if (cluster.isMaster) { worker2.on('online', common.mustCall(worker2.disconnect)); })); + cluster.on('exit', function(worker, code) { + assert.strictEqual(code, 0, 'worker exited with error'); + }); + return; }