From 0ee4df9c7a02a1bce5659c7a5ad8d4372b6afcc1 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 10 Jun 2015 08:41:03 -0700 Subject: [PATCH] test: make listen-fd-cluster/server more robust - eliminate unnecessary intermediate process ("parent") - children exit if runner dies unexpectedly (killed on a test timeout, for example) - use explicit messaging from children to parents to indicate when worker is ready to accept http requests, rather than racing to see whether runner will make request before worker is listening PR-URL: https://github.com/nodejs/io.js/pull/1944 Reviewed-By: Johan Bergstrom Reviewed-By: Ben Noordhuis --- test/parallel/test-listen-fd-cluster.js | 85 +++++++++++++++---------- test/parallel/test-listen-fd-server.js | 82 +++++++++++------------- 2 files changed, 88 insertions(+), 79 deletions(-) diff --git a/test/parallel/test-listen-fd-cluster.js b/test/parallel/test-listen-fd-cluster.js index e895a2944789bd..f6d00c72a0f78c 100644 --- a/test/parallel/test-listen-fd-cluster.js +++ b/test/parallel/test-listen-fd-cluster.js @@ -7,63 +7,62 @@ var PORT = common.PORT; var spawn = require('child_process').spawn; var cluster = require('cluster'); -console.error('Cluster listen fd test', process.argv.slice(2)); +console.error('Cluster listen fd test', process.argv[2] || 'runner'); if (common.isWindows) { console.log('1..0 # Skipped: This test is disabled on windows.'); return; } +// Process relationship is: +// +// parent: the test main script +// -> master: the cluster master +// -> worker: the cluster worker switch (process.argv[2]) { case 'master': return master(); case 'worker': return worker(); case 'parent': return parent(); - default: return test(); } +var ok; + +process.on('exit', function() { + assert.ok(ok); +}); + // spawn the parent, and listen for it to tell us the pid of the cluster. // WARNING: This is an example of listening on some arbitrary FD number // that has already been bound elsewhere in advance. However, binding // server handles to stdio fd's is NOT a good or reliable way to do // concurrency in HTTP servers! Use the cluster module, or if you want // a more low-level approach, use child process IPC manually. -function test() { - var parent = spawn(process.execPath, [__filename, 'parent'], { - stdio: [ 0, 'pipe', 2 ] - }); - var json = ''; - parent.stdout.on('data', function(c) { - json += c.toString(); - if (json.indexOf('\n') !== -1) next(); - }); - function next() { - console.error('output from parent = %s', json); - var cluster = JSON.parse(json); - // now make sure that we can request to the worker, then kill it. - http.get({ - server: 'localhost', - port: PORT, - path: '/', - }).on('response', function(res) { - var s = ''; - res.on('data', function(c) { - s += c.toString(); - }); - res.on('end', function() { - // kill the worker before we start doing asserts. - // it's really annoying when tests leave orphans! - parent.kill(); - process.kill(cluster.master, 'SIGKILL'); - +test(function(parent) { + // now make sure that we can request to the worker, then kill it. + http.get({ + server: 'localhost', + port: PORT, + path: '/', + }).on('response', function(res) { + var s = ''; + res.on('data', function(c) { + s += c.toString(); + }); + res.on('end', function() { + // kill the worker before we start doing asserts. + // it's really annoying when tests leave orphans! + parent.kill(); + parent.on('exit', function() { assert.equal(s, 'hello from worker\n'); assert.equal(res.statusCode, 200); console.log('ok'); + ok = true; }); }); - } -} + }); +}); -function parent() { +function test(cb) { console.error('about to listen in parent'); var server = net.createServer(function(conn) { console.error('connection on parent'); @@ -73,7 +72,7 @@ function parent() { var spawn = require('child_process').spawn; var master = spawn(process.execPath, [__filename, 'master'], { - stdio: [ 0, 1, 2, server._handle ], + stdio: [ 0, 'pipe', 2, server._handle, 'ipc' ], detached: true }); @@ -90,6 +89,11 @@ function parent() { console.error('master closed'); }); console.error('master spawned'); + master.on('message', function(msg) { + if (msg === 'started worker') { + cb(master); + } + }); }); } @@ -99,7 +103,17 @@ function master() { args: [ 'worker' ] }); var worker = cluster.fork(); - console.log('%j\n', { master: process.pid, worker: worker.pid }); + worker.on('message', function(msg) { + if (msg === 'worker ready') { + process.send('started worker'); + } + }); + // Prevent outliving our parent process in case it is abnormally killed - + // under normal conditions our parent kills this process before exiting. + process.on('disconnect', function() { + console.error('master exit on disconnect'); + process.exit(0); + }); } @@ -112,5 +126,6 @@ function worker() { res.end('hello from worker\n'); }).listen({ fd: 3 }, function() { console.error('worker listening on fd=3'); + process.send('worker ready'); }); } diff --git a/test/parallel/test-listen-fd-server.js b/test/parallel/test-listen-fd-server.js index d4d11e5a8d9693..d51d51ee8eb4de 100644 --- a/test/parallel/test-listen-fd-server.js +++ b/test/parallel/test-listen-fd-server.js @@ -13,54 +13,50 @@ if (common.isWindows) { switch (process.argv[2]) { case 'child': return child(); - case 'parent': return parent(); - default: return test(); } -// spawn the parent, and listen for it to tell us the pid of the child. +var ok; + +process.on('exit', function() { + assert.ok(ok); +}); + // WARNING: This is an example of listening on some arbitrary FD number // that has already been bound elsewhere in advance. However, binding // server handles to stdio fd's is NOT a good or reliable way to do // concurrency in HTTP servers! Use the cluster module, or if you want // a more low-level approach, use child process IPC manually. -function test() { - var parent = spawn(process.execPath, [__filename, 'parent'], { - stdio: [ 0, 'pipe', 2 ] - }); - var json = ''; - parent.stdout.on('data', function(c) { - json += c.toString(); - if (json.indexOf('\n') !== -1) next(); - }); - function next() { - console.error('output from parent = %s', json); - var child = JSON.parse(json); - // now make sure that we can request to the child, then kill it. - http.get({ - server: 'localhost', - port: PORT, - path: '/', - }).on('response', function(res) { - var s = ''; - res.on('data', function(c) { - s += c.toString(); - }); - res.on('end', function() { - // kill the child before we start doing asserts. - // it's really annoying when tests leave orphans! - process.kill(child.pid, 'SIGKILL'); - try { - parent.kill(); - } catch (e) {} - +test(function(child) { + // now make sure that we can request to the child, then kill it. + http.get({ + server: 'localhost', + port: PORT, + path: '/', + }).on('response', function(res) { + var s = ''; + res.on('data', function(c) { + s += c.toString(); + }); + res.on('end', function() { + child.kill(); + child.on('exit', function() { assert.equal(s, 'hello from child\n'); assert.equal(res.statusCode, 200); + console.log('ok'); + ok = true; }); }); - } -} + }); +}); function child() { + // Prevent outliving the parent process in case it is terminated before + // killing this child process. + process.on('disconnect', function() { + console.error('exit on disconnect'); + process.exit(0); + }); + // start a server on fd=3 http.createServer(function(req, res) { console.error('request on child'); @@ -68,10 +64,11 @@ function child() { res.end('hello from child\n'); }).listen({ fd: 3 }, function() { console.error('child listening on fd=3'); + process.send('listening'); }); } -function parent() { +function test(cb) { var server = net.createServer(function(conn) { console.error('connection on parent'); conn.end('hello from parent\n'); @@ -80,7 +77,7 @@ function parent() { var spawn = require('child_process').spawn; var child = spawn(process.execPath, [__filename, 'child'], { - stdio: [ 0, 1, 2, server._handle ] + stdio: [ 0, 1, 2, server._handle, 'ipc' ] }); console.log('%j\n', { pid: child.pid }); @@ -90,13 +87,10 @@ function parent() { // be accepted, because the child has the fd open. server.close(); - child.on('exit', function(code) { - console.error('child exited', code); - }); - - child.on('close', function() { - console.error('child closed'); + child.on('message', function(msg) { + if (msg === 'listening') { + cb(child); + } }); - console.error('child spawned'); }); }