From 8d627246f07bf316a97a35531f437e8845c90941 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 24 Sep 2015 01:30:25 -0400 Subject: [PATCH 1/2] child_process: `null` channel handle on close `HandleWrap::OnClose` destroys the underlying C++ object and null's the internal field pointer to it. Therefore there should be no references to the wrapping JavaScript object. `null` the process' `_channel` field right after closing it, to ensure no crashes will happen. Fix: https://github.com/nodejs/node/issues/2847 --- lib/internal/child_process.js | 1 + .../test-child-process-fork-regr-gh-2847.js | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 test/parallel/test-child-process-fork-regr-gh-2847.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index d1596406cf9d53..8c1abc5f746fc8 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -449,6 +449,7 @@ function setupChannel(target, channel) { target.disconnect(); channel.onread = nop; channel.close(); + target._channel = null; maybeClose(target); } }; diff --git a/test/parallel/test-child-process-fork-regr-gh-2847.js b/test/parallel/test-child-process-fork-regr-gh-2847.js new file mode 100644 index 00000000000000..4bff53d8c1f584 --- /dev/null +++ b/test/parallel/test-child-process-fork-regr-gh-2847.js @@ -0,0 +1,42 @@ +'use strict'; + +const assert = require('assert'); +const common = require('../common'); + +const cluster = require('cluster'); +const net = require('net'); +const util = require('util'); + +if (!cluster.isMaster) { + // Exit on first received handle to leave the queue non-empty in master + process.on('message', function() { + process.exit(1); + }); + return; +} + +var server = net.createServer(function(s) { + setTimeout(function() { + s.destroy(); + }, 100); +}).listen(common.PORT, function() { + var worker = cluster.fork(); + + function send(callback) { + var s = net.connect(common.PORT, function() { + worker.send({}, s, callback); + }); + s.on('error', function() { + }); + } + + worker.process.once('close', common.mustCall(function() { + // Otherwise the crash on `_channel.fd` access may happen + assert(worker.process._channel === null); + server.close(); + })); + + // Queue up several handles, to make `process.disconnect()` wait + for (var i = 0; i < 100; i++) + send(); +}); From 1d883605a06784e6d110fd3d46fc2cc78698d034 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 24 Sep 2015 12:15:11 -0400 Subject: [PATCH 2/2] fixes --- test/parallel/test-child-process-fork-regr-gh-2847.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-child-process-fork-regr-gh-2847.js b/test/parallel/test-child-process-fork-regr-gh-2847.js index 4bff53d8c1f584..07ef6415a3c3e1 100644 --- a/test/parallel/test-child-process-fork-regr-gh-2847.js +++ b/test/parallel/test-child-process-fork-regr-gh-2847.js @@ -1,7 +1,7 @@ 'use strict'; -const assert = require('assert'); const common = require('../common'); +const assert = require('assert'); const cluster = require('cluster'); const net = require('net'); @@ -26,8 +26,6 @@ var server = net.createServer(function(s) { var s = net.connect(common.PORT, function() { worker.send({}, s, callback); }); - s.on('error', function() { - }); } worker.process.once('close', common.mustCall(function() {