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 race condition can cause IdAssigned event to be missed in browser #469

Open
caspark opened this issue Dec 10, 2024 · 0 comments
Open

Comments

@caspark
Copy link

caspark commented Dec 10, 2024

I seem to be consistently running into a race condition in receiving the IdAssigned event:

  • I'm using matchbox_socket 0.10.0 (latest as of writing) on linux and wasm with macroquad and ggrs, polling the future via macroquad's coroutines (which approximately polls the future every on vsync), running signaling server and two clients on my linux desktop
  • As a linux build, everything works just fine
  • As a wasm build, ICE gathering and data channel establishment work fine (at least on chromium - Firefox is hit and miss), but when I call WebRtcSocket::id() then it always returns None; without that I can't work out which peer is the local peer (and so can't establish a ggrs session).
  • I turned up the (and added some extra) logging to various bits of matchbox as well as ws_stream_wasm and it looks like the Rust code never sees the IdAssigned event on the websocket at all.
  • Yet I can see the websocket IdAssigned event arrive on the websocket in the browser's devtools.

Hence, I suspect it must be a race condition caused by a sequence of events like this:

  1. Websocket connection is opened via ws_stream_wasm creating the websocket
  2. Websocket connection is established
  3. Signaling server sends IdAssigned message, which is received by the browser
  4. ws_stream_wasm adds the onmessage handler for the websocket, too late to handle the IdAssigned message

Presumably this issue is caused by less-frequent-than-normal polling of the future, on account of using macroquad's coroutines, but theoretically I believe it would be possible for the same thing to happen to e.g. a bevy user.

(I theorize that the desktop build of my game doesn't exhibit this issue because of tungstenite buffering events until they are read - but I haven't confirmed that's the case.)

A few fixes come to mind:

  • Patch ws_stream_wasm to use some set of web APIs that allow setting event handlers before the websocket connection is established
    • Unfortunately, it doesn't seem like any such browser APIs exist :/
  • Have the wasm variant of matchbox_socket send a "give me my peerid" message to the server as soon as the websocket is established - which guarantees that it will be able to see the reply.
  • Have the signaling server respond to keep alive events with IdAssigned events - basically this is the same as the previous bullet, but it adds some redundant signaling-server -> client traffic and also requires the client to wait 10 seconds for the first keepalive message to be issued.

I have a working version of the second fix at caspark@3c479d2 and could polish it up into a PR if that would be desired - but figured I'd raise the issue first in case I'm missing something obvious.

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

No branches or pull requests

1 participant