Event Listener Leak

[droghio]

In an web app I am making I continually spawn and destroy spooky instances using the built in ".destory()" method.

This works, but every eleventh spooky respawn I get a double EventEmitter memleak warning (see below for raw dump).

Poking around the spooky and tiny-json-rpc sources pinned this line in tiny-json-rpc as the source of the error:

        this._server.stream.on('data', this._onData.bind(this)); //Line 36 Of stream-client
        this._server.stream.on('drain', this._onDrain.bind(this));  //Line 37

What I think is happening is that when the spooky object is destroyed it doesn't clear these listeners from its _rpcClient object, thus causing these leaks.

I could be way off base, but I'm pretty sure that's what's going on.

Error:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at RequestStream.EventEmitter.addListener (events.js:160:15)
    at new StreamClient (/Users/jldrogo/Developer/repo/Crawler/node_modules/spooky/node_modules/tiny-jsonrpc/lib/tiny-jsonrpc/stream-client.js:36:29)
    at new Spooky (/Users/jldrogo/Developer/repo/Crawler/node_modules/spooky/lib/spooky.js:75:27)
    at resetSpooky (/Users/jldrogo/Developer/repo/Crawler/worker.js:45:14)
    at Promise.<anonymous> (/Users/jldrogo/Developer/repo/Crawler/links.js:83:20)
    at Promise.<anonymous> (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:177:8)
    at Promise.EventEmitter.emit (events.js:95:17)
    at Promise.emit (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:84:38)
    at Promise.fulfill (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:97:20)
    at Promise.resolve (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/lib/promise.js:108:15)
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at RequestStream.EventEmitter.addListener (events.js:160:15)
    at new StreamClient (/Users/jldrogo/Developer/repo/Crawler/node_modules/spooky/node_modules/tiny-jsonrpc/lib/tiny-jsonrpc/stream-client.js:37:29)
    at new Spooky (/Users/jldrogo/Developer/repo/Crawler/node_modules/spooky/lib/spooky.js:75:27)
    at resetSpooky (/Users/jldrogo/Developer/repo/Crawler/worker.js:45:14)
    at Promise.<anonymous> (/Users/jldrogo/Developer/repo/Crawler/links.js:83:20)
    at Promise.<anonymous> (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:177:8)
    at Promise.EventEmitter.emit (events.js:95:17)
    at Promise.emit (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:84:38)
    at Promise.fulfill (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/node_modules/mpromise/lib/promise.js:97:20)
    at Promise.resolve (/Users/jldrogo/Developer/repo/Crawler/node_modules/mongoose/lib/promise.js:108:15)

[lawnsea]

Yeah, that sounds like a bug, alright. Thanks for doing such a thorough investigation!

[droghio]

No problem, thanks for such a quick response!

Do you have any suggested patch in the mean time?
I suppose I could try manually clearing the listeners before destroying the spooky instance.

[lawnsea]

I think the fix would be to store a reference to the server option passed when creating the this._rpcClient as this._rpcServer and then call this._rpcServer.off in the destroy method.

If this works, please submit it as a pull request.

[lawnsea]

Not off, but rather removeAllListeners.

[pedrotanaka]

+1 hopefully the issue gets fixed soon

[bekzod]

+1

[joshbalfour]

+1, this is breaking our app.
Thanks @droghio for doing the legwork!

[fcuenca]

I ran into the same problem and implemented the proposed solution of calling removeAllListeners on the client stream, but it didn't work.

After doing some debugging, I figured that removeAllListeners doesn't seem to work unless you specify the event to remove (contrary to what the documentation says. But of course, this wouldn't be the first time documentations lies... :-)

In any case, I fixed the issue by explicitly removing all the events in the destroy function:

Spooky.prototype.destroy = function () {
    // -- FIX --- issue https://github.com/WaterfallEngineering/SpookyJS/issues/104
    this._clientStream.removeAllListeners('data');
    this._clientStream.removeAllListeners('drain');
    // --- FIX ---
    this._child.kill();
    delete Spooky._instances[this.options.child.port];
};

Prior to that, I capture the client stream when the _rpcClient variable is initialized (line 107):

        this._clientStream = new RequestStream({
            host: options.transport.http.host,
            port: options.child.port
        });
        this._rpcClient = new tinyjsonrpc.StreamClient({ server: this._clientStream });

[fcuenca]

Update: I then noticed that calling the destroy function, while preventing the event listener leak, it seems to cause the phantomjs process to stay after spooky is finished. I'm using it with cucumber.js, creating a Spooky instance at the beginning of each scenario, so I end up with multiple phantomjs process hanging around at the end.
If I don't call destroy, then there's no process leak, but I get back the listener leak warning.