cluster: suicide flag is not set on master when calling disconnect from a worker

[dpatti]

From the documentation:

worker.suicide:

Set by calling .kill() or .disconnect(), until then it is undefined.

worker.disconnect():

In a worker, this function will close all servers, wait for the 'close' event on those servers, and then disconnect the IPC channel.

Causes .suicide to be set.

Repro:

var cluster = require('cluster');

if (cluster.isMaster) {
  // Master forks and listens for events
  var worker = cluster.fork();
  cluster.on('disconnect', function(){
    console.log("disconnect", worker.suicide);
  });
  cluster.on('exit', function(){
    console.log("exit", worker.suicide);
  });
} else {
  // Worker just disconnects
  cluster.worker.disconnect();
}

In node v0.10, both log statements would print true for worker.suicide. In 41b75ca, which landed during the v0.11 branch, this was broken, and all versions up to node v4 will print false. This is because worker.disconnect(), when called from the worker, does not send the suicide message anymore -- only worker.destroy() will send that message to the master.

[zhongkai]

Came across same problem. I set a flag called "isSuicide" of worker Object:

var cluster = require('cluster');

if (cluster.isMaster) {
  // Master forks and listens for events
  var worker = cluster.fork();
  cluster.on('disconnect', function(worker){
    worker.isSuicide = true;
    console.log("disconnect", worker.suicide);
  });
  cluster.on('exit', function(worker){
    worker.isSuicide && console.log("exit by suicide");
  });
} else {
  // Worker just disconnects
  cluster.worker.disconnect();
}

Is this a good practice for all node versions?

[sam-github]

all versions up to node v4 will print false.

Are you saying it will print true for v5 and later?

@zhongkai No, it is not good practice. Your code sets suicide when the IPC channel is disconnected, but the 0.10 behaviour does it only when the worker deliberately disconnected.

Change your line of code cluster.worker.disconnect(); to thow 'farewall, crule world', and your code will also set your custom isSuicide property, which it should not.

[dpatti]

@sam-github Note that this issue was opened over a year ago and was supposedly fixed in f299d87. I don't know what the current behavior is or what is intended.

[sam-github]

Ah, didn't notice that. So, should be fixed. @zhongkai On what node version did you come across this problem?

[zhongkai]

@sam-github , I'm sorry. I just want to make sure the solution is OK for version 0.12.x or not?