cluster: fix race condition setting suicide prop

There is no guarantee that the `suicide` property of a worker in the master
process is going to be set when the `disconnect` and `exit` events are emitted.
To fix it, wait for the ACK of the suicide message from the master before
disconnecting the worker.
Modify `test-regress-nodejsGH-3238` so it checks both the `kill` and `disconnect`
cases. Also take into account that the `disconnect` event may be received after
the `exit` event.

lib/cluster.js

@@ -434,7 +434,7 @@ function masterInit() {
     else if (message.act === 'listening')
       listening(worker, message);
     else if (message.act === 'suicide')
-      worker.suicide = true;
+      suicide(worker, message);
     else if (message.act === 'close')
       close(worker, message);
   }
@@ -445,6 +445,11 @@ function masterInit() {
     cluster.emit('online', worker);
   }
 
+  function suicide(worker, message) {
+    worker.suicide = true;
+    send(worker, { ack: message.seq });
+  }
+
   function queryServer(worker, message) {
     var args = [message.address,
                 message.port,
@@ -665,8 +670,7 @@ function workerInit() {
     function checkWaitingCount() {
       waitingCount--;
       if (waitingCount === 0) {
-        send({ act: 'suicide' });
-        process.disconnect();
+        send({ act: 'suicide' }, () => process.disconnect());
       }
     }
 
@@ -684,9 +688,8 @@ function workerInit() {
     this.suicide = true;
     if (!this.isConnected()) process.exit(0);
     var exit = process.exit.bind(null, 0);
-    send({ act: 'suicide' }, exit);
+    send({ act: 'suicide' }, () => process.disconnect());
     process.once('disconnect', exit);
-    process.disconnect();
   };
 
   function send(message, cb) {

test/parallel/test-regress-GH-3238.js

@@ -4,18 +4,19 @@ const assert = require('assert');
 const cluster = require('cluster');
 
 if (cluster.isMaster) {
-  const worker = cluster.fork();
-  let disconnected = false;
+  function forkWorker(action) {
+    const worker = cluster.fork({ action });
+    worker.on('disconnect', common.mustCall(() => {
+      assert.strictEqual(worker.suicide, true);
+    }));
 
-  worker.on('disconnect', common.mustCall(function() {
-    assert.strictEqual(worker.suicide, true);
-    disconnected = true;
-  }));
+    worker.on('exit', common.mustCall(() => {
+      assert.strictEqual(worker.suicide, true);
+    }));
+  }
 
-  worker.on('exit', common.mustCall(function() {
-    assert.strictEqual(worker.suicide, true);
-    assert.strictEqual(disconnected, true);
-  }));
+  forkWorker('disconnect');
+  forkWorker('kill');
 } else {
-  cluster.worker.disconnect();
+  cluster.worker[process.env.action]();
 }