Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix EventEmitter memory leak #1365

Closed
wants to merge 1 commit into from
Closed

fix EventEmitter memory leak #1365

wants to merge 1 commit into from

Conversation

@bricewge
Copy link

bricewge commented Apr 26, 2018

On some torrents, after resuming, webtorrent-cli throw this warning:

(node:13649) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 ready listeners added. Use emitter.setMaxListeners() to increase limit
    at _addListener (events.js:188:19)
    at Torrent.addListener (events.js:205:10)
    at Torrent.once (events.js:236:8)
    at Request.onChunk [as callback] (/home/bricewge/tmp/webtorrent-cli/node_modules/webtorrent/lib/torrent.js:1504:34)
    at Wire._callback (/home/bricewge/tmp/webtorrent-cli/node_modules/bittorrent-protocol/index.js:608:11)
    at Wire._onPiece (/home/bricewge/tmp/webtorrent-cli/node_modules/bittorrent-protocol/index.js:512:8)
    at Wire._onMessage (/home/bricewge/tmp/webtorrent-cli/node_modules/bittorrent-protocol/index.js:678:19)
    at Wire._write (/home/bricewge/tmp/webtorrent-cli/node_modules/bittorrent-protocol/index.js:596:10)
    at doWrite (/home/bricewge/tmp/webtorrent-cli/node_modules/readable-stream/lib/_stream_writable.js:428:64)
    at writeOrBuffer (/home/bricewge/tmp/webtorrent-cli/node_modules/readable-stream/lib/_stream_writable.js:417:5)

This PR seems to fix that.

It's related to #1122 and webtorrent/webtorrent-cli/issues/55.

@feross

This comment has been minimized.

Copy link
Member

feross commented Apr 26, 2018

FYI, that warning is harmless, if annoying.

Honestly, I have no idea what the removed line does, hence the TODO. I'd need to re-read the code to make sure this won't break anything. We could also just merge it and see if there are new errors related to this in the WebTorrent Desktop telemetry.

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

DiegoRBaquero commented Jul 24, 2018

The line if just deferring the chunk management until Torrent fires ready. So if over 10 chunks get downloaded before the Torrent fires ready, warning will be shown.

But... do we really care if torrent is not ready to store the chunk and mark the piece received, etc? Apparently not. I would just remove it.

@feross

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

DiegoRBaquero commented Aug 24, 2018

This need to be fixed by correctly organizing the methods and events.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.