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: #1944
Reviewed-By: Johan Bergstrom <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>

Author: sam-github

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');
   });
 }

test/parallel/test-listen-fd-server.js

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