Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

cluster.disconnect callback getting called too early in 0.11#6346

Closed
fiws wants to merge 2 commits intonodejs:masterfrom
fiws:cluster-disconnect-fix
Closed

cluster.disconnect callback getting called too early in 0.11#6346
fiws wants to merge 2 commits intonodejs:masterfrom
fiws:cluster-disconnect-fix

Conversation

@fiws
Copy link
Copy Markdown

@fiws fiws commented Oct 13, 2013 

var cluster = require('cluster');

if (cluster.isMaster) {
  // Fork workers.
  cluster.fork();
  cluster.fork();

  // "called when all workers are disconnected [...]"
  cluster.disconnect(function(){
    for(var prop in cluster.workers){
      var worker = cluster.workers[prop];
      console.log(worker.state);
    }
  });

} else {
  require('net').createServer().listen(8000);
}

It seems like the disconnect callback is getting called to early. If I loop through the workers I still get one with the 'online' state. This does not happen on 0.10

Expected output: nothing or disconnect/exit
got: online

node version: v0.11.7
OS: Ubuntu Linux 64bit

Edit:

Sorry for the pull request hassle. I'm not so familiar with the way github handles them.

@Nodejs-Jenkins
Copy link
Copy Markdown

Nodejs-Jenkins commented Oct 13, 2013

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit fiws/node@75e796c has the following error(s):

  • First line of commit message must be no longer than 50 characters

The following commiters were not found in the CLA:

  • Filip Weiss

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@fiws fiws closed this Oct 13, 2013
@fiws fiws deleted the cluster-disconnect-fix branch October 13, 2013 20:59
@fiws
cluster: fix dc callback called too early
03d62e9 
cluster.disconnect should only callback if all workers have disconnected.
@fiws fiws reopened this Oct 13, 2013
@bnoordhuis
Copy link
Copy Markdown
Member

bnoordhuis commented Oct 14, 2013

Thanks for your PR. I didn't take it but I landed a slightly different fix and regression test in 45885a1. Cheers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

master pr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fiws @Nodejs-Jenkins @bnoordhuis