Skip to content

Renamed WebRtcTransport webRtcServerClosed() to listenServerClosed() - #1141

Merged
ibc merged 1 commit into
versatica:v3from
Mafalda-SFU:listenServerClosed
Aug 11, 2023
Merged

ibc merged 1 commit into
versatica:v3from
Mafalda-SFU:listenServerClosed

Conversation

@piranna

@piranna piranna commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

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

ibc commented Aug 11, 2023

Copy link
Copy Markdown
Member

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

a34ca02

@ibc

ibc commented Aug 11, 2023

Copy link
Copy Markdown
Member

@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
Copy Markdown
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

piranna commented Aug 11, 2023

Copy link
Copy Markdown
Contributor Author

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