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

AssertionError: false == true at SharedHandle.add (cluster.js:97:3) #9261

Closed
FLYBYME opened this issue Feb 21, 2015 · 31 comments
Closed

AssertionError: false == true at SharedHandle.add (cluster.js:97:3) #9261

FLYBYME opened this issue Feb 21, 2015 · 31 comments

Comments

@FLYBYME
Copy link

FLYBYME commented Feb 21, 2015

Getting this error

assert.js:86
  throw new assert.AssertionError({
        ^
AssertionError: false == true
    at SharedHandle.add (cluster.js:97:3)
    at queryServer (cluster.js:480:12)
    at Worker.onmessage (cluster.js:438:7)
    at ChildProcess.<anonymous> (cluster.js:692:8)
    at ChildProcess.emit (events.js:129:20)
    at handleMessage (child_process.js:324:10)
    at Pipe.channel.onread (child_process.js:352:11)
$ node -v
v0.12.0
@cjihrig
Copy link

cjihrig commented Feb 22, 2015

Thanks for reporting this. Can you please provide a reduced test case, with no external dependencies, that reproduces the issue?

@elliotstokes
Copy link

I'm also getting this error. Happens when I try to make a udp request from a cluster.

@elliotstokes
Copy link

test case. It will work first request and fail for the second.

var cluster = require('cluster');
var http = require('http');
var dgram = require('dgram');
var numCPUs = require('os').cpus().length;

if (cluster.isMaster) {
  // Fork workers.
  for (var i = 0; i < numCPUs; i++) {
    cluster.fork();
  }

} else {

  var server = http.createServer(function (request, response) {
    var client = dgram.createSocket("udp4");
    var message = new Buffer("Hello");
    client.send(message, 0, message.length, 80, "google.com", function(err, bytes) {
      client.close();
    });
    response.writeHead(200, {"Content-Type": "text/plain"});
    response.end("Hello World\n");
  });

  server.listen(1337);
}

@elliotstokes
Copy link

this doesn't look like it happens on 0.10.x only 0.12

@FLYBYME
Copy link
Author

FLYBYME commented Feb 25, 2015

I tracked it down to been UDP as elliotstokes did.

@elliotstokes
Copy link

@FLYBYME did you find a workaround at all? i tried a few things but didn't get anywhere.

@cjihrig
Copy link

cjihrig commented Mar 1, 2015

The solution to this is in #8643. It landed in master, but not 0.12. A workaround would be to create an exclusive UDP socket in your worker.

@rcrichton
Copy link

I'm getting the exact same problem and using node from master doesn't solve it. I'm only using UDP server sockets and I get the following when closing and restarting the UDP server:

AssertionError: false == true
    at SharedHandle.add (cluster.js:99:3)
    at queryServer (cluster.js:482:12)
    at Worker.onmessage (cluster.js:440:7)
    at ChildProcess.<anonymous> (cluster.js:694:8)
    at ChildProcess.emit (events.js:131:20)
    at handleMessage (child_process.js:326:10)
    at Pipe.channel.onread (child_process.js:354:11)

It works fine without using the cluster API, but breaks with it.

$ node --version v0.13.0-pre

See the above referenced commit if you want to see some code.

@nexflo
Copy link

nexflo commented Mar 29, 2015

This is a major break for clustering (which is a core feature for node).
Is there an ETA in which release this will appear?
Thanks in advance.

@pungoyal
Copy link

pungoyal commented Apr 7, 2015

This has become a major problem for us. Our processes keep getting killed. Workaround is to put a monitoring tool/script, but we'd rather not do that.

@cjihrig
Copy link

cjihrig commented Apr 7, 2015

The following code (the original test case posted in this issue) works in master, but not 0.12, as #8643 did not land in 0.12. Do you have a reduced test case, written in JavaScript, with no external dependencies, that reproduces the problem against the master branch? If you are using 0.12, you may want to look into using the exclusive option with your UDP sockets.

var cluster = require('cluster');
var http = require('http');
var dgram = require('dgram');
var numCPUs = require('os').cpus().length;

if (cluster.isMaster) {
  // Fork workers.
  for (var i = 0; i < numCPUs; i++) {
    cluster.fork();
  }

} else {

  var server = http.createServer(function (request, response) {
    var client = dgram.createSocket("udp4");
    var message = new Buffer("Hello");
    client.send(message, 0, message.length, 80, "google.com", function(err, bytes) {
      client.close();
    });
    response.writeHead(200, {"Content-Type": "text/plain"});
    response.end("Hello World\n");
  });

  server.listen(1337);
}

@elliotstokes
Copy link

When is it going to be released? It's currently stopping me upgrading. I looked into the workaround but it didn't look like it was possible as the socket that causes the problems is created within the datagram code.

@cjihrig
Copy link

cjihrig commented Apr 7, 2015

The master branch being released is probably still quite a while away. Here is a workaround in 0.12:

var cluster = require('cluster');
var http = require('http');
var dgram = require('dgram');
var numCPUs = require('os').cpus().length;

if (cluster.isMaster) {
  // Fork workers.
  for (var i = 0; i < numCPUs; i++) {
    cluster.fork();
  }

} else {

  var server = http.createServer(function (request, response) {
    var client = dgram.createSocket("udp4");

    client.bind({exclusive: true}, function() {
      var message = new Buffer("Hello");
      client.send(message, 0, message.length, 80, "google.com", function(err, bytes) {
        client.close();
      });
      response.writeHead(200, {"Content-Type": "text/plain"});
      response.end("Hello World\n");
    });
  });

  server.listen(1337);
}

@scottnonnenberg
Copy link

+1 for a fix! Since my module thehelp-cluster uses node-statsd to capture statistics (which in turn uses dgram), it's broken on node 0.12. Works in io.js.

@brianreavis
Copy link

Also hitting this (node v0.12.3).

@ctizen
Copy link

ctizen commented Jun 10, 2015

This bug still exists in 0.12.4 and prevents current "stable" version from being used in production.
Is #8643 going to be released in a reasonable time?

@alekzonder
Copy link

bug appears in version 0.11.2, on commit "cluster: use round-robin load balancing" e72cd41 in lib/cluster.js line 71

but in io.js 2.3.0 works fine

adding already existing worker => assertion fires

May be check worker in this.workers?

https://github.com/joyent/node/blob/v0.12.4/lib/cluster.js#L97

SharedHandle.prototype.add = function(worker, send) {
  // assert(this.workers.indexOf(worker) === -1);
  if (this.workers.indexOf(worker) === -1) {
       this.workers.push(worker);
  }
  send(this.errno, null, this.handle);
};

@ctizen
Copy link

ctizen commented Jun 23, 2015

#8643 was partially included in recent release, but this bug still exists in 0.12.5.
The fix in lib/dgram.js was not included, for example.

Cluster mode still crashes -> node 0.12.5 still not ready for high-available production servers :(

@jasnell jasnell added the dgram label Jun 25, 2015
@jasnell
Copy link
Member

jasnell commented Jun 30, 2015

Looking at it more, #8195 appears to the same problem. @joyent/node-tsc @misterdjules @cjihrig ... This definitely is a confirmed issue. Is there any particular reason this should not land in v0.12?

@cjihrig
Copy link

cjihrig commented Jul 3, 2015

@jasnell I think it might have been deemed to be a behavior change. I would personally like to see it in 0.12 as well.

@tleach
Copy link

tleach commented Jul 10, 2015

This is preventing us upgrading all our node apps to 0.12, so a fix would be great.

@hildoer
Copy link

hildoer commented Aug 23, 2015

+1

1 similar comment
@austinkelleher
Copy link

+1

@nodesocket
Copy link

👍 Believe this issue is breaking AppPress/node-connect-datadog#4

@indutny
Copy link
Member

indutny commented Aug 25, 2015

Minimal test case:

var cluster = require('cluster');
var http = require('http');
var dgram = require('dgram');
var numCPUs = require('os').cpus().length;

if (cluster.isMaster)
  return cluster.fork();

dgram.createSocket('udp4').bind(1337);
dgram.createSocket('udp4').bind(1337);

Working on fix.

indutny added a commit to indutny/io.js that referenced this issue Aug 25, 2015
Allow listening on reused dgram ports in cluster workers.

Fix: nodejs/node-v0.x-archive#9261
@indutny
Copy link
Member

indutny commented Aug 25, 2015

Should be fixed by nodejs/node#2548

@nodesocket
Copy link

Will this go out in v0.12.8?

@indutny
Copy link
Member

indutny commented Aug 25, 2015

cc @jasnell

@jasnell
Copy link
Member

jasnell commented Aug 25, 2015

I'll take a look. If the backport isn't too significant then it shouldn't be a problem getting it in. That said, we don't have a clear timeline for v0.12.8 yet so we'll have to figure that out as well.

@jasnell jasnell self-assigned this Aug 25, 2015
indutny added a commit to nodejs/node that referenced this issue Sep 8, 2015
Allow listening on reused dgram ports in cluster workers.

Fix: nodejs/node-v0.x-archive#9261
PR-URL: #2548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
indutny added a commit to nodejs/node that referenced this issue Sep 8, 2015
Allow listening on reused dgram ports in cluster workers.

Fix: nodejs/node-v0.x-archive#9261
PR-URL: #2548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
indutny added a commit to nodejs/node that referenced this issue Sep 12, 2015
Allow listening on reused dgram ports in cluster workers.

Fix: nodejs/node-v0.x-archive#9261
PR-URL: #2548
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@violet-day
Copy link

+1

@austinkelleher
Copy link

Just a heads up, the issues I was experiencing with cluster have been resolved in 4.0.0.

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

No branches or pull requests