Concurrent transport teardown: cleanUp() nulls publisher/subscriber after awaiting dispose()

[lennartvdd]

Environment

livekit_client 2.11.0 · flutter_webrtc 1.6.0 · reproduced in the package's own test harness (no device needed)

Summary

Engine.cleanUp() nulls the transport fields after awaiting dispose():

await publisher?.dispose();   // engine.dart:289 — suspends inside pc.close()
publisher = null;             // engine.dart:290 — only after it returns

A concurrent caller entering during that await still sees publisher non-null and calls dispose() on the same object. Disposable correctly guards the second dispose of the same transport — but that guard returns immediately, so the second cleanUp() overtakes the first and tears down the other transport while the first is still inside pc.close(). Two RTCPeerConnection.close() calls end up in flight at once, which never happens when the paths run sequentially.

Three uncoordinated writers touch those fields: cleanUp() (:289), restartConnection() (:1211-1217, a near-copy), and Room._cleanUp() via :1079. Room's engine listener is created with createListener() (room.dart:183) — unsynchronized — so Room's handlers do not serialize either.

Reproduction

test/race/teardown_race_test.dart on the branch below. Real Room + real Engine, mock socket and peer connection; the tracing peer connection's close() spans several event-loop turns to model the blocking native close.

scenario	concurrent pc.close	double-dispose
S1 give-up branch alone	1	0
S2 give-up ‖ room.dispose()	2	2
S3 pending reconnect + disconnect()	1	0
S4 two EngineDisconnectedEvents	2	2

S2 is ordinary public API: the SDK gives up reconnecting while the app disposes the Room. S4 is reachable because the give-up branch never sets _isClosed = true (unlike :1014 and :1473), leaving the engine free to emit a second disconnect.

Your own log line is the tell — the baseline run prints !! [Transport#…] unnecessary dispose() called. (disposable.dart:55).

A fix that looks sufficient and is not

Locking cleanUp() alone still fails S2, because restartConnection() bypasses it. Confirmed by intervention, not by reasoning.

Suggested fix (branch ready, happy to open a PR)

Extract _disposeTransportsLocked() that detaches before awaiting, route cleanUp() and restartConnection() through one Engine lock, and give Room._cleanUp() its own. synchronized is already a direct dependency (used by event.dart).

The probe fails on pristine v2.11.0, passes with the change, and the full suite stays green (402 passed).

Branch: https://github.com/Kyra-H-I/client-sdk-flutter/tree/fix/serialize-transport-teardown

What we have not established

Whether this causes a specific native crash. We hit an iOS EXC_BAD_ACCESS that turned out to be a separate, already-fixed WebRTC-SDK bug (webrtc-sdk/webrtc@f47af7bc9658, shipped in 144.7559.10). This race may be a trigger for that class of fault — the faulting path requires a redundant stop — but we cannot prove it, and we are reporting the race on its own merits.