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

Websocket response + disconnect race condition? #1260

Closed
SeanMollet opened this issue Sep 26, 2024 · 4 comments · Fixed by #1266
Closed

Websocket response + disconnect race condition? #1260

SeanMollet opened this issue Sep 26, 2024 · 4 comments · Fixed by #1266

Comments

@SeanMollet
Copy link

I'm shipping video via WebRTC to a "server" with a unique websocket behavior:

Connect, send SDP, it immediately responds with an answer and then disconnects.

Most of the time, onClosed ends up getting called and onMessage never is. Wireshark shows that the answer was received. Digging into PollService::runLoop, it seems like this is caused by a race. If the close is executed before the runLoop comes back around, the message ends up dropped on the floor.

Am I reading the code correctly?

From onClose, is there any easy way to get the data? receive() gives me nothing.

Happy to dig in and solve this, but wanted to get some thoughts from others on the best approach and/or validation that I'm reading all of this correctly.

Thanks

@SeanMollet
Copy link
Author

I'm using TLS and it seems the problem is that the socket close triggers TlsTransport::stop(), which unregisters the incoming callback., thus eating the received message.

@paullouisageneau
Copy link
Owner

Thank you for reporting, you are right, data can still be waiting for TLS decryption when TCP disconnect. Could you please try with #1266 ?

@SeanMollet
Copy link
Author

@paullouisageneau Testing now, will report back in a few minutes

@SeanMollet
Copy link
Author

Tested with #1266, fixed.

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

Successfully merging a pull request may close this issue.

2 participants