You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Found while merging the changes from #988 into #986.
#988 serializes Room.connect's audio publish with setMicrophoneEnabled on the per-source publish lock, but the full reconnect republish path never takes that lock. republishTracks() republishes the same default track instance that setTrackEnabled operates on, so the two can overlap on one LocalAudioTrack. The cross-path guards don't compose safely: isTrackPublished in publishTrackImpl only sees publications that have already landed, and the duplicate-cid rejection in RTCEngine.addTrack does detect an in-flight publish but collapses it into the same failure as a real publish error, so the losing caller cleans up a track it does not own. The duplicate check and the resolver registration are also separate synchronized blocks, so even that detection is check-then-act.
The window is wide because prepareForFullReconnect() clears trackPublications at the start of the reconnect attempt, so for the whole join + ICE phase getTrackPublication(source) returns null and setMicrophoneEnabled(true) takes the create-and-publish branch with the cached default track. There is no connection-state gate above it during RECONNECTING.
I reproduced three interleavings with MockE2ETest-based Robolectric tests, holding a publish in flight by withholding the TrackPublished response via wsFactory.registerSignalRequestHandler:
App's AddTrackRequest is in flight when republishTracks runs. The republish publish fails with DuplicateTrackException and republishTracks stops the shared track (L1316-L1318). The app's publish then lands. Final state: setMicrophoneEnabled returned true, publication unmuted, rtcTrack disabled. Silent mic reported as success.
Republish's AddTrackRequest is in flight when the app calls setMicrophoneEnabled(true). The app's duplicate publish fails, the "concurrent publish won the race" guard from Fix silent mic race between Room.connect audio publish and setMicrophoneEnabled #988 (L382-L385) sees isTrackPublished == false because the republish has not landed yet, and stops the very track the republish is publishing (L386-L388). Final state: publication lands unmuted with a disabled track. Silent mic again.
Mic muted at disconnect, app enables mid-reconnect. No timing luck needed, pure program order: the app's fresh publish lands, then republishTracks unconditionally runs unpublishTrack(track, false) (L1308), which removes the app's new publication by track instance, then skips republishing because the old snapshot publication was muted (L1310). Final state: setMicrophoneEnabled returned true, no MICROPHONE publication left, and the track is left enabled and never stopped while unpublished.
In cases 1 and 2 retrying setMicrophoneEnabled(true) does not recover: it hits the unmute path and the muted setter early-returns on an already-unmuted publication (LocalTrackPublication.kt#L37-L39) without re-enabling the media track. A full off/on toggle does recover it.
A residual version of the #988 race also exists without any reconnect: publishAudioTrack(getOrCreateDefaultAudioTrack()) called directly, with setMicrophoneEnabled(true) arriving while that publish is in flight, ends the same way as case 2.
The camera paths have the same shape (setTrackEnabled camera branch plus the republishTracks video arm), with stopCapture() in the failure branches, so the video analog would stop capture on a live published camera track. I only tested audio.
I can put together a fix. The direction I would take: have republishTracks take sourcePubLocks for each republished source, skip snapshot entries whose track already has a live publication (the lock alone does not cover case 3, where the app's publish lands before republish acquires it), and make the failure branches distinguish a duplicate-in-flight rejection from a real publish failure before stopping a shared track. The repro tests can come along in the PR.
Found while merging the changes from #988 into #986.
#988 serializes
Room.connect's audio publish withsetMicrophoneEnabledon the per-source publish lock, but the full reconnect republish path never takes that lock.republishTracks()republishes the same default track instance thatsetTrackEnabledoperates on, so the two can overlap on oneLocalAudioTrack. The cross-path guards don't compose safely:isTrackPublishedinpublishTrackImplonly sees publications that have already landed, and the duplicate-cid rejection inRTCEngine.addTrackdoes detect an in-flight publish but collapses it into the same failure as a real publish error, so the losing caller cleans up a track it does not own. The duplicate check and the resolver registration are also separate synchronized blocks, so even that detection is check-then-act.The window is wide because
prepareForFullReconnect()clearstrackPublicationsat the start of the reconnect attempt, so for the whole join + ICE phasegetTrackPublication(source)returns null andsetMicrophoneEnabled(true)takes the create-and-publish branch with the cached default track. There is no connection-state gate above it during RECONNECTING.I reproduced three interleavings with MockE2ETest-based Robolectric tests, holding a publish in flight by withholding the
TrackPublishedresponse viawsFactory.registerSignalRequestHandler:App's
AddTrackRequestis in flight whenrepublishTracksruns. The republish publish fails withDuplicateTrackExceptionandrepublishTracksstops the shared track (L1316-L1318). The app's publish then lands. Final state:setMicrophoneEnabledreturned true, publication unmuted, rtcTrack disabled. Silent mic reported as success.Republish's
AddTrackRequestis in flight when the app callssetMicrophoneEnabled(true). The app's duplicate publish fails, the "concurrent publish won the race" guard from Fix silent mic race between Room.connect audio publish and setMicrophoneEnabled #988 (L382-L385) seesisTrackPublished == falsebecause the republish has not landed yet, and stops the very track the republish is publishing (L386-L388). Final state: publication lands unmuted with a disabled track. Silent mic again.Mic muted at disconnect, app enables mid-reconnect. No timing luck needed, pure program order: the app's fresh publish lands, then
republishTracksunconditionally runsunpublishTrack(track, false)(L1308), which removes the app's new publication by track instance, then skips republishing because the old snapshot publication was muted (L1310). Final state:setMicrophoneEnabledreturned true, no MICROPHONE publication left, and the track is left enabled and never stopped while unpublished.In cases 1 and 2 retrying
setMicrophoneEnabled(true)does not recover: it hits the unmute path and the muted setter early-returns on an already-unmuted publication (LocalTrackPublication.kt#L37-L39) without re-enabling the media track. A full off/on toggle does recover it.A residual version of the #988 race also exists without any reconnect:
publishAudioTrack(getOrCreateDefaultAudioTrack())called directly, withsetMicrophoneEnabled(true)arriving while that publish is in flight, ends the same way as case 2.The camera paths have the same shape (
setTrackEnabledcamera branch plus therepublishTracksvideo arm), withstopCapture()in the failure branches, so the video analog would stop capture on a live published camera track. I only tested audio.I can put together a fix. The direction I would take: have
republishTrackstakesourcePubLocksfor each republished source, skip snapshot entries whose track already has a live publication (the lock alone does not cover case 3, where the app's publish lands before republish acquires it), and make the failure branches distinguish a duplicate-in-flight rejection from a real publish failure before stopping a shared track. The repro tests can come along in the PR.