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

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.

cluster: send suicide message on disconnect

cluster: fix race condition setting suicide prop

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?

@zhongkai

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.

@sam-github

@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.

@zhongkai

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

@sam-github

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

test: rename test-regress-nodejsGH-3238

test: name test files appropriately

cluster: send suicide message on disconnect

Navigation Menu

Repository navigation

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

19 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

19 remaining items

zhongkai commented on Dec 21, 2016

sam-github commented on Dec 21, 2016

dpatti commented on Dec 21, 2016

sam-github commented on Dec 21, 2016

zhongkai commented on Jan 29, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions