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

callback of failed listen call wrongly invoked after successful call #2325

Closed
vincentcr opened this issue Dec 14, 2011 · 6 comments
Closed

Comments

@vincentcr
Copy link

with node 0.6.5 on a mac:

it seems that if one retries a listen on a server after the first one failed (because, eg, we're trying to find a free port), the callbacks will be invoked both for the first, failed, call, and the second, successful call.

here's a sample that explains it better, i hope:


var http = require('http');

var port = 12345;

var s1 = http.createServer();

s1.listen(port, function(){
    console.log('s1 listen successful:', s1.address().port);
});

var s2 = http.createServer();

s2.on('error', function(err) {
    if (err.code !== 'EADDRINUSE') throw err;

    //retry next port
    s2.listen(port + 1, function(){
        console.log('s2 listen # 2 successful:', s2.address().port);
    });
});

s2.listen(port, function(){
    console.log('s2 listen # 1 successful:', s2.address().port);
});

this outputs:


s1 listen successful: 12345
s2 listen # 1 successful: 12346
s2 listen # 2 successful: 12346

's2 listen # 1' should not happen because it wasn't, in fact, sucessful as the port was busy with s1.

@bnoordhuis
Copy link
Member

What happens is that both calls to s2.listen() register their own 'listening' event handler. When the second listen attempt succeeds, both handlers fire. If you explicitly register the handler it works as expected:

var http = require('http');

var port = 12345;

var s1 = http.createServer();

s1.listen(port, function(){
    console.log('s1 listen successful:', s1.address().port);
});

var s2 = http.createServer();

// REGISTER HANDLER
s2.on('listening', function() {
    console.log('s2 listen # 1 successful:', s2.address().port);
});

s2.on('error', function(err) {
    if (err.code !== 'EADDRINUSE') throw err;

    //retry next port
    s2.listen(port + 1);
});

s2.listen(port);

I agree it's somewhat confusing behaviour, we should probably fix it.

@vincentcr
Copy link
Author

thanks for the workaround.

@vincentcr
Copy link
Author

btw i would like to point out that this is mostly a documentation issue.

the doc for net.listen makes clear what is happening and, if i had read it, i would not have thought of this behavior as a bug.

however, the doc for http.listen (which is what i read) is more ambiguous. also it's not mentioned that http extends net (however obvious this seems once you know), and so that http.listen is in fact net.listen.

@koichik
Copy link

koichik commented Dec 16, 2011

@vincentcr: I agree.

@bnoordhuis: Can you review koichik/node@4d954b8?

@bnoordhuis
Copy link
Member

@koichik: LGTM

@koichik
Copy link

koichik commented Dec 16, 2011

@bnoordhuis: Thanks!

alexkwolfe pushed a commit to alexkwolfe/node that referenced this issue Dec 23, 2011
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

3 participants