Skip to content
This repository has been archived by the owner on Oct 7, 2020. It is now read-only.

net::Server.unref() failed on cluster mode #73

Closed
kyriosli opened this issue Jul 30, 2015 · 3 comments
Closed

net::Server.unref() failed on cluster mode #73

kyriosli opened this issue Jul 30, 2015 · 3 comments

Comments

@kyriosli
Copy link

This can be easily reproduced by the following code:

var cluster = require('cluster');
if (cluster.isMaster) {
    cluster.fork();
}
else {
    require('net').createServer().listen(8081, function () {
        this.unref();
    })
}

In node v0.12.7 it says:

net.js:1440
    this._handle.unref();
                 ^
TypeError: undefined is not a function
    at Server.unref (net.js:1440:18)
    at Server.<anonymous> (/home/kyrios.li/test.js:7:8)
    at Server.g (events.js:199:16)
    at Server.emit (events.js:129:20)
    at net.js:1171:12
    at process._tickCallback (node.js:355:11)

and the same code is ok when NODE_CLUSTER_SCHED_POLICY is set to none. So I thought maybe it is a problem with RoundRobin?

@kyriosli
Copy link
Author

I simplified the code to to the following:

var cluster = require('cluster');
if (cluster.isMaster) {
    cluster.fork();
}

but the child process is not exiting because there is internal message listeners. So Making unref work well will not make a change.

But I think we should at least avoid the error to be thrown, maybe a patch to the net module that ignores the unref call when cluster.isSlave is true will do?

@bnoordhuis
Copy link
Member

nodejs/node#2274

bnoordhuis referenced this issue in bnoordhuis/io.js Jul 30, 2015
Add ref() and unref() stub methods to the faux handle in round-robin
mode.  Fixes the following TypeError when calling `server.unref()` in
the worker:

    net.js:1521
        this._handle.unref();
                     ^
    TypeError: this._handle.unref is not a function
        at Server.unref (net.js:1521:18)

No actual reference counting is implemented.  It would effectively be
a no-op because the control channel would still keep the worker alive.

Fixes: nodejs#73
PR-URL: nodejs#2274
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@bnoordhuis
Copy link
Member

Fixed by nodejs/node@fa98b97.

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