thibauts / node-castv2

An implementation of the Chromecast CASTV2 protocol
MIT License
765 stars 99 forks source link

EventEmitter memory leak #24

Closed angelnu closed 8 years ago

angelnu commented 8 years ago
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at Client.EventEmitter.addListener (events.js:160:15)
    at new Channel (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/node_modules/castv2/lib/channel.js:16:12)
    at Client.createChannel (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/node_modules/castv2/lib/client.js:132:10)
    at MediaController.Controller (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/controllers/controller.js:8:25)
    at MediaController.JsonController (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/controllers/json.js:6:14)
    at MediaController.RequestResponseController (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/controllers/request-response.js:6:18)
    at new MediaController (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/controllers/media.js:6:29)
    at MediaController.fn (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/senders/sender.js:29:23)
    at construct (/home/anunez/Downloads/node_modules/iobroker/node_modules/iobroker.chromecast/node_modules/castv2-client/lib/senders/sender.js:32:10)
angelnu commented 8 years ago

This happened after several client.join() calls on the same client instance. This results in the same function being registered each time:

Channel.js

this.bus.on('message', onmessage);

The root reason seems to be that the old player object is not destroyed before a new own is created. Should this be handled automatically by the client code or is it the caller responsibility to close old player?

My code currently assumes a player disconnect after it receives a client status package where there is no applications object. The player does not seem to be closed automatically when this happens. I could easily try to close the player but I wonder why this does not happen. Going though the code trying to figure out why.

angelnu commented 8 years ago

I found the following suspicious code: node-chromecast-client/lib/controllers/controller.js

this.channel.once('close', onclose);
...
Controller.prototype.close = function() {
  this.channel.close();
};

This would explain why a close from the channel does not result on a close from the application. Should this be changed to:

Controller.prototype.close = function() {
  self.emit("close");
};

?

thibauts commented 8 years ago

You're right in assuming it's the caller responsibility to explicitly close the objects it creates.

When you call close on a Controller, it closes its channel, the channel emits a close event on itself that is caught by the controller which then remove its listeners on the channel.

If I'm not mistaken everything should behave as expected.

angelnu commented 8 years ago

You are right: if the channel closes then the controller will also close itself. So here everything seems ok: maybe effects from the new year :-)

What I still see is that I get RECEIVER_STATUS packages without any applications on them and the media player object does not notice this. Therefore I assume that the Chromecast does not send a close message for the channel OR it is OK for the chromecast to send RECEIVER_STATUS packages without applications even if the application is still there.

{"volume":{"level":1,
         *            "muted":false}}

Maybe I am to paranoid by closing the player application when I get the package above...

What do you think?