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

Renamed WebRtcTransport webRtcServerClosed() to listenServerClosed() #1141

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

piranna
Copy link
Contributor

@piranna piranna commented Aug 10, 2023

I've found a typo where the WebRtcTransport.webRtcServerClosed() was never called. Instead, the Transport.listenServerClosed() method is being directly called by WebRtcServer, so the clean-ups done at webRtcServerClosed() are never done. Since listenServerClose event is at Transport level with the idea maybe of have other listener servers in the future (raw TCP? RTP?), I've renamed webRtcServerClosed() to the more generic one listenServerClosed(), calling to the parent class.

@ibc
Copy link
Member

ibc commented Aug 11, 2023

And here a new added test that indeed fails in v3 branch and will pass once this PR is merged:

a34ca02

@ibc
Copy link
Member

ibc commented Aug 11, 2023

@nazar-pc I am not brave enough to know how this is done in Rust, I couldn't find any similar code so the bug probably doesn't happen in Rust.

@ibc ibc merged commit 60f57d1 into versatica:v3 Aug 11, 2023
@nazar-pc
Copy link
Collaborator

Cleanups are done in Drop implementation in Rust, so most likely it works correctly there

@piranna piranna deleted the listenServerClosed branch August 11, 2023 17:02
@piranna
Copy link
Contributor Author

piranna commented Aug 11, 2023

And here a new added test that indeed fails in v3 branch and will pass once this PR is merged:

a34ca02

Glad you liked this PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants