peter-murray / node-hue-api

Node.js Library for interacting with the Philips Hue Bridge and Lights
Apache License 2.0
1.19k stars 145 forks source link

upnpSearch #55

Closed pl7y closed 9 years ago

pl7y commented 9 years ago

When performing some sequential upnpSearch (e.g. to force reconnection upon link going down), node.js EventEmitter gives up since it reaches the maxListeners limit.

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at process.addListener (events.js:160:15)
    at process.on.process.addListener (node.js:796:26)
    at new SSDPSearch (/.../node_modules/node-hue-api/hue-api/search.js:57:14)
    at Object.locateBridges (/.../node_modules/node-hue-api/hue-api/search.js:15:18)
    at Object.module.exports.networkSearch [as upnpSearch] (/.../node_modules/node-hue-api/hue-api/bridge-discovery.js:20:19)
    at Firefly.searchForBridge (/.../backend/server/firefly.js:15:13)
    at /.../backend/server/firefly.js:18:22
    at _fulfilled (/.../node_modules/node-hue-api/node_modules/q/q.js:787:54)
    at self.promiseDispatch.done (/.../node_modules/node-hue-api/node_modules/q/q.js:816:30)
    at Promise.promise.promiseDispatch (/.../node_modules/node-hue-api/node_modules/q/q.js:749:13)
    at /.../node_modules/node-hue-api/node_modules/q/q.js:509:49
    at flush (/.../node_modules/node-hue-api/node_modules/q/q.js:108:17)
    at process._tickCallback (node.js:442:13)
pl7y commented 9 years ago

I tracked down the cause to be a process.on statement at the end of SSDPSearch function in search.js. Commenting down lines 57-61 removes the warning, but I'd like to retain the 'exit' cleansing idea, how can I do it?

peter-murray commented 9 years ago

Thanks for the info, I need to fix this to only register the process.exit listener once, if it is called multiple times during execution. All the existing tests only perform a single lookup, so I missed this one.

I would suggest that you utilise the other search function, as this SSDP lookup is a throw back to before Phillips provided the discovery service, which should work better in most cases.

peter-murray commented 9 years ago

I am not at a computer with Node.js at the moment, but I think you could change the code at 57-61 to something like this;

if (!exitFunction) {
        exitFunction = function () {
            if (!self.closed) {
                _close(self);
            }
        };
        // Clean up if close is not called directly
        process.on("exit", exitFunction);
    }

And put a line at line 8 in the file with:

var exitFunction;

I think that would prevent it from registering multiple times in your app.

I need to put together a test case for this when I get back to a machine I can develop on.

pl7y commented 9 years ago

Thank you very much, tomorrow i will try it (it's night here)!

Can the other search function be used without a internet connection?

Il giorno mar 18 ago 2015 23:29 Peter Murray notifications@github.com ha scritto:

I am not at a computer with Node.js at the moment, but I think you could change the code at 57-61 to something like this;

if (!exitFunction) { exitFunction = function () { if (!self.closed) { _close(self); } }; // Clean up if close is not called directly process.on("exit", exitFunction); }

And put a line at line 8 in the file with:

var exitFunction;

I think that would prevent it from registering multiple times in your app.

I need to put together a test case for this when I get back to a machine I can develop on.

— Reply to this email directly or view it on GitHub https://github.com/peter-murray/node-hue-api/issues/55#issuecomment-132357247 .

peter-murray commented 9 years ago

Ah, no, the other function uses the Internet to connect with the hue discovery service, so you are stuck with the SSDP way then.