livekit / rust-sdks

LiveKit realtime and server SDKs for Rust
https://livekit.io
Apache License 2.0
185 stars 44 forks source link

livekit: wait for disconnect before callback #370

Open nbsp opened 2 months ago

nbsp commented 2 months ago

should fix a race condition where in node-sdks await room.disconnect() doesn't wait for the room status update to change.

draft because it doesn't work reliably yet: patched node-sdks still fails approximately one in fifty times.

node-sdks diff ```diff diff --git a/packages/livekit-rtc/src/room.ts b/packages/livekit-rtc/src/room.ts index 143774f..f9bebeb 100644 --- a/packages/livekit-rtc/src/room.ts +++ b/packages/livekit-rtc/src/room.ts @@ -16,6 +16,7 @@ import type { ConnectResponse, ConnectionQuality, DataPacketKind, + DisconnectCallback, DisconnectResponse, IceServer, RoomInfo, @@ -148,7 +149,7 @@ export class Room extends (EventEmitter as new () => TypedEmitter return; } - FfiClient.instance.request({ + const res = FfiClient.instance.request({ message: { case: 'disconnect', value: { @@ -157,6 +158,10 @@ export class Room extends (EventEmitter as new () => TypedEmitter }, }); + await FfiClient.instance.waitFor((ev: FfiEvent) => { + return ev.message.case == 'disconnect' && ev.message.value.asyncId == res.asyncId; + }); + FfiClient.instance.removeAllListeners(); this.removeAllListeners(); } ```
nbsp commented 1 month ago

fixed on the node-sdks side. merge this before merging livekit/node-sdks#223

theomonnom commented 1 month ago

I think just removing close_tx and waiting for engine_tx to ends should do the work (we can close engine_tx on EngineInner)

nbsp commented 1 month ago

I think just removing close_tx and waiting for engine_tx to ends should do the work (we can close engine_tx on EngineInner)

i don't think this works, await room.disconnect() on Node doesn't unblock