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: connection leaks #420

Merged
merged 1 commit into from
Mar 25, 2022
Merged

fix: connection leaks #420

merged 1 commit into from
Mar 25, 2022

Conversation

lookis
Copy link
Contributor

@lookis lookis commented Mar 23, 2022

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[X] Bug fix
[ ] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)
clear up the peer resources when destroy

Which issue (if any) does this pull request address?
this PR MAY related to these issue:
webtorrent/webtorrent#1079
webtorrent/webtorrent-hybrid#119

Is there anything you'd like reviewers to focus on?
There is another connection leak when tracker(WebRTC mode) interval is too small, for example 5 seconds, when webtorrent-hybrid as a seeder will announce too fast, and every time hybrid announce, It will create 5 offer peers here:
https://github.com/webtorrent/bittorrent-tracker/blob/master/lib/client/websocket-tracker.js#L63

and these peers only get destroy after 50 seconds:
https://github.com/webtorrent/bittorrent-tracker/blob/master/lib/client/websocket-tracker.js#L392

in this case, there will be more than 50 Peers, and make the hybrid hang forever (osx 12.2.1)

@welcome
Copy link

welcome bot commented Mar 23, 2022

🙌 Thanks for opening this pull request! You're awesome.

@lookis
Copy link
Contributor Author

lookis commented Mar 25, 2022

are these changes OK for merge? @DiegoRBaquero @feross

Copy link
Member

@DiegoRBaquero DiegoRBaquero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a good cleanup happening and this is consistent with other parts

@DiegoRBaquero DiegoRBaquero merged commit f7928cf into webtorrent:master Mar 25, 2022
@welcome
Copy link

welcome bot commented Mar 25, 2022

🎉 Congrats on getting your first pull request landed!

webtorrent-bot pushed a commit that referenced this pull request Mar 25, 2022
## [9.18.5](v9.18.4...v9.18.5) (2022-03-25)

### Bug Fixes

* connection leaks ([#420](#420)) ([f7928cf](f7928cf))
@webtorrent-bot
Copy link

🎉 This PR is included in version 9.18.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

DiegoRBaquero added a commit that referenced this pull request May 11, 2022
webtorrent-bot pushed a commit that referenced this pull request May 11, 2022
## [9.18.6](v9.18.5...v9.18.6) (2022-05-11)

### Bug Fixes

* revert [#420](#420) ([8d54938](8d54938))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants