From 584c40b74979252e65fdaec4ff73b146d921cc96 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Sun, 19 Jul 2015 19:20:21 +0000 Subject: [PATCH 1/2] test: handling failure cases properly Refer: https://github.com/nodejs/io.js/issues/1543 When this test fails, it leaves dead processes in the system. This patch makes sure that the child processes exit first, in case of errors. --- test/parallel/test-cluster-eaccess.js | 60 +++++++++++++++++---------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/test/parallel/test-cluster-eaccess.js b/test/parallel/test-cluster-eaccess.js index ad4de97e389ec5..2076d7fa9cb538 100644 --- a/test/parallel/test-cluster-eaccess.js +++ b/test/parallel/test-cluster-eaccess.js @@ -1,41 +1,54 @@ 'use strict'; -// test that errors propagated from cluster children are properly -// received in their master creates an EADDRINUSE condition by also -// forking a child process to listen on a socket - -var common = require('../common'); -var assert = require('assert'); -var cluster = require('cluster'); -var fork = require('child_process').fork; -var fs = require('fs'); -var net = require('net'); +// test that errors propagated from cluster workers are properly +// received in their master. Creates an EADDRINUSE condition by forking +// a process in child cluster and propagates the error to the master. +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); +const fork = require('child_process').fork; +const fs = require('fs'); +const net = require('net'); if (cluster.isMaster) { - var worker = cluster.fork(); - var gotError = 0; - worker.on('message', function(err) { - gotError++; + const worker = cluster.fork(); + + // makes sure master is able to fork the worker + cluster.on('fork', common.mustCall(function() {})); + + // makes sure the worker is ready + worker.on('online', common.mustCall(function() {})); + + worker.on('message', common.mustCall(function(err) { + // disconnect first, so that we will not leave zombies + worker.disconnect(); + console.log(err); assert.strictEqual('EADDRINUSE', err.code); - worker.disconnect(); - }); + })); + process.on('exit', function() { console.log('master exited'); try { fs.unlinkSync(common.PIPE); - } catch (e) { + } catch (ex) { + if (ex.code !== 'ENOENT') { + throw ex; + } } - assert.equal(gotError, 1); }); + } else { - var cp = fork(common.fixturesDir + '/listen-on-socket-and-exit.js', + const cp = fork(common.fixturesDir + '/listen-on-socket-and-exit.js', { stdio: 'inherit' }); // message from the child indicates it's ready and listening - cp.on('message', function() { - var server = net.createServer().listen(common.PIPE, function() { - console.log('parent listening, should not be!'); + cp.on('message', common.mustCall(function() { + const server = net.createServer().listen(common.PIPE, function() { + // message child process so that it can exit + cp.send('end'); + // inform master about the unexpected situation + process.send('PIPE should have been in use.'); }); server.on('error', function(err) { @@ -45,5 +58,6 @@ if (cluster.isMaster) { // propagate error to parent process.send(err); }); - }); + + })); } From df946163dd50dcd80dd1d620c199f4ecfa6fac17 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Wed, 5 Aug 2015 18:04:39 +0530 Subject: [PATCH 2/2] Addressing Ben's comments --- test/parallel/test-cluster-eaccess.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-cluster-eaccess.js b/test/parallel/test-cluster-eaccess.js index 2076d7fa9cb538..b8c05a2c8a0924 100644 --- a/test/parallel/test-cluster-eaccess.js +++ b/test/parallel/test-cluster-eaccess.js @@ -1,5 +1,5 @@ 'use strict'; -// test that errors propagated from cluster workers are properly +// Test that errors propagated from cluster workers are properly // received in their master. Creates an EADDRINUSE condition by forking // a process in child cluster and propagates the error to the master. @@ -31,15 +31,12 @@ if (cluster.isMaster) { console.log('master exited'); try { fs.unlinkSync(common.PIPE); - } catch (ex) { - if (ex.code !== 'ENOENT') { - throw ex; - } + } catch (e) { } }); } else { - const cp = fork(common.fixturesDir + '/listen-on-socket-and-exit.js', + var cp = fork(common.fixturesDir + '/listen-on-socket-and-exit.js', { stdio: 'inherit' }); // message from the child indicates it's ready and listening