Skip to content

Commit

Permalink
test: make listen-fd-cluster/server more robust
Browse files Browse the repository at this point in the history 
- 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: #1944
Reviewed-By: Johan Bergstrom <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
@sam-github @rvagg
sam-github authored and rvagg committed Aug 17, 2015
1 parent 1738c96 commit 0ee4df9
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 79 deletions.
85 changes: 50 additions & 35 deletions test/parallel/test-listen-fd-cluster.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
});

Expand All @@ -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);
}
});
});
}

Expand All @@ -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);
});
}


Expand All @@ -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');
});
}
82 changes: 38 additions & 44 deletions test/parallel/test-listen-fd-server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,65 +13,62 @@ 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');
console.error('%s %s', req.method, req.url, req.headers);
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');
Expand All @@ -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 });
Expand All @@ -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');
});
}

0 comments on commit 0ee4df9

Please sign in to comment.