NodeJS listener bug

[adrian-nitu-92]

• Version: all after https://github.com/nodejs/node/commit/b7a8a691b4c6ff0f89b5d8dba3184eaef0efc898
• Platform: Linux automation-tck 3.19.0-25-generic #26~14.04.1-Ubuntu SMP Fri Jul 24 21:16:20 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: core

Commit at: https://github.com/nodejs/node/commit/b7a8a691b4c6ff0f89b5d8dba3184eaef0efc898 contains a bug that prevents node from listening on a port.

I have detected this bug while trying to run acmeair-nodejs ( https://github.com/acmeair/acmeair-nodejs ) on the latest version of node.

Netstat report:

../builds/latest/node/node app.js &
[1] 130509
netstat -l | grep :9080
fg
../builds/latest/node/node app.js
^C

Expected report:

~/baseline/node/install/usr/local/bin/node app.js &
[1] 130519
netstat -l | grep :9080
tcp6       0      0 [::]:9080               [::]:*                  LISTEN     
fg
~/baseline/node/install/usr/local/bin/node app.js
^C

I am sure this is the commit with the issue because the commit https://github.com/nodejs/node/commit/1b99093df78b795052c944fc6388a934e84e89ef is a_ok (manually tested).

[adrian-nitu-92]

Oups :), missclick

[addaleax]

Can you try to reduce this to a small reproducible test case?

[adrian-nitu-92]

Looking into it :D

[jasnell]

FWIW, A simple test case with an http server running os x with master isn't showing any issues, so a reproducible test case would be very helpful :-)

[addaleax]

Added the 7.0.0 milestone here because the commit that might be causing it is a semver-major one that would be included in v7, just so we don’t forget about this issue.

[gareth-ellis]

That seems genuine:

https://benchmarking.nodejs.org/charts/acmeair_throughput.png

The last three days we've not been able to collect throughput on acmeair in the benchmarking workgroup - see the three blue dots at the bottom!

Edit: that said, I can't recreate it on my machine here....I'm trying to get a copy of the logs from our benchmarking run to see if there's anything that sticks out

[adrian-nitu-92]

Helllo, i'm still looking into the issue and what caused it. I managed to solve the issue by re-cloning acmeair and doing a npm install. The clone seems identical to my version of acmeair (_I checked only files tracked by git_) but maybe a secret (deep) dependency in node_modules breaks when it has to "cooperate" with this commit. I am still trying to see exactly what happened but the walk over the dependency tree is really expensive :)

[gareth-ellis]

Can you do npm ls and find the versions of modules ? - this could perhaps be it breaks on an old copy of express....

[adrian-nitu-92]

Oups, seems I made a mistake on my previous post. It did not actually work(oups, I also have a stable version of node installed :D) and I was going mad trying to find the issue.

The issue happens in mongodb.connect. Sample code that doesn't work:

var mongodb = require('mongodb');

var mongourl = "mongodb://localhost:27017/test";

var c_opt = {server:{auto_reconnect:true,poolSize: 10}};
mongodb.connect(mongourl, c_opt, function(err, conn){
         console.error("fixed!");
    });

Acmeair doesn't work because it first wants the connection to the DB established and then it wants to actually start listening.

[gareth-ellis]

Mongo db seems to use timer in quite a lot of places - however upgrading mongo to 2.0.x seems to work fine for both the simple mongo test and acme air.

[adrian-nitu-92]

I can fully confirm that using mongo 2.0.x is a working fix.

I can continue our tests with the new change, so as far as I'm concerned, the issue is resolved.

1. Should I try to make a pull request to acmeair-nodejs with this version of mongo(1.4.x->2.0.x)?
2. I see the benchmarking group is still reporting no results for acmeair, @gareth-ellis, can you give them a nudge :D?

[gareth-ellis]

I'm in the workgroup, I'll get the copy of acmeair updated shortly.

However, I still think we should identify which bit of mongo fails on the the latest, and convince ourselves that the change in Node is correct - there could well be a number of other modules that are also broken, but no one has tested on the master branch.....

@jasnell ,what are your thoughts on this?

[jasnell]

Additional testing is always good. I'm still pretty certain the change in core was the correct thing to do but it would be good to see if anything else was affected this way.

On Monday, August 22, 2016, Gareth Ellis notifications@github.com wrote:

I'm in the workgroup, I'll get the copy of acmeair updated shortly.

However, I still think we should identify which bit of mongo fails on the the latest, and convince ourselves that the change in Node is correct - there could well be a number of other modules that are also broken, but no one has tested on the master branch.....

@jasnell https://github.com/jasnell ,what are your thoughts on this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/8051#issuecomment-241378006, or mute the thread https://github.com/notifications/unsubscribe-auth/AAa2ebHm4bFm_EPijGxmUkmq5KprVyWdks5qiX8AgaJpZM4JhL8K .

[Fishrock123]

Added the 7.0.0 milestone here because the commit that might be causing it is a semver-major one that would be included in v7, just so we don’t forget about this issue.

@addaleax you tagged this for v7, mind to check it again? :D

[addaleax]

Mhm, I wasn’t too successful getting the mongodb@1.4.x test suite up and running. So, unless @adrian-nitu-92 or @gareth-ellis can give any further clues here, there might be nothing we can do.

[gareth-ellis]

I also haven't been successful in narrowing down what in MongoDB 1.4.x stops working but works in 2.0.x.

[Trott]

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.