test(audio-rooms) + fix: round-trip test + audit pass
Adds an end-to-end MoQ round-trip test and lands the highest-severity
findings from a 3-agent audit (protocol / ViewModel / Android lifecycle)
of the M5–M7 + Activity work.
Round-trip test (`:nestsClient` MoqRoundTripTest):
- Two MoqSession.client() instances (publisher + subscriber) talk
through a hand-rolled in-test relay coroutine that mirrors a real
MoQ relay's wire behavior (forwards CLIENT_SETUP / SERVER_SETUP /
ANNOUNCE / SUBSCRIBE / SUBSCRIBE_OK / OBJECT_DATAGRAM).
- 100-Opus-frame test exercises the full publisher → wire →
subscriber path, asserting payload bytes + monotonic group/object
ids round-trip correctly. Catches any drift between our publisher
and our own subscriber's wire format.
- Second test verifies SUBSCRIBE_ERROR(TRACK_DOES_NOT_EXIST) flows
back as MoqProtocolException when the publisher hasn't openTrack'd.
MoQ protocol fixes (CRITICAL audit findings):
- SubscribeDoneStatus constants were inverted: had UNSUBSCRIBED=0x01
(peer reads as INTERNAL_ERROR) and TRACK_ENDED=0x00 (peer reads as
UNSUBSCRIBED). Swapped to draft-stable values: UNSUBSCRIBED=0x00,
TRACK_ENDED=0x03.
- Pending CompletableDeferreds for in-flight SUBSCRIBE / ANNOUNCE on
session close were `cancel()`-ed, which propagates as
CancellationException — caller's entire scope cancels instead of
catching a domain MoqProtocolException. Switched all sites to
`completeExceptionally(MoqProtocolException("session closed"))`
including unsubscribe-while-pending-OK.
ViewModel fixes:
- openSubscription race: re-check `activeSubscriptions[pubkey] === slot
&& !closed` AFTER the suspending `subscribeSpeaker` returns; if the
user removed the speaker mid-flight, fire-and-forget UNSUBSCRIBE
rather than attaching a leaked SubscribeHandle + AudioRoomPlayer to
a discarded slot.
- Server-initiated `Closed` listener state now triggers
`teardown(targetState=Closed)` and resets the auto-retry counter,
so a transport-died-mid-handshake doesn't leave stale subscriptions
in the VM map until the Activity finishes.
- Cleanup-scope split: `disconnect()` (user-driven) routes the close
through `viewModelScope` (still alive); `onCleared()` routes it
through a process-lived `cleanupScope` (default GlobalScope, tests
pass backgroundScope) so MoQ control frames (UNSUBSCRIBE,
UNANNOUNCE, SUBSCRIBE_DONE) actually land before the QUIC
transport drops, instead of being eaten by the cancelled
viewModelScope.
Android lifecycle fixes:
- AudioRoomBridge.clear() wired into AccountViewModel.onCleared next
to the existing CallSessionBridge.clear() — no more cross-account
AccountViewModel leak after logout/switch.
- registerReceiver flag now gated on Build.VERSION.SDK_INT
TIRAMISU+ — RECEIVER_NOT_EXPORTED on pre-33 devices collides with
RECEIVER_VISIBLE_TO_INSTANT_APPS. PendingIntents stay
package-scoped via setPackage(packageName).
- onUserLeaveHint no longer enters PIP unconditionally: gated on
ui.connection == Connected AND PackageManager
FEATURE_PICTURE_IN_PICTURE present, so PIP-from-lobby /
PIP-on-incompatible-device doesn't leave a frozen full-screen
card in Recents.
- Replaced the process-wide singleton AudioRoomPipActions toggle
Boolean with a per-Activity MutableSharedFlow<Unit> so a stale
emission from a torn-down Activity can't leak into a new one.
ViewModel test was extended with `cleanupScope = backgroundScope`
plumbing so the post-disconnect `listener.close()` assertion remains
observable in the test scheduler.
Verified: spotlessApply clean; :commons:jvmTest (9 tests),
:nestsClient:jvmTest (80 tests including 2 new round-trip), and
:amethyst:compilePlayDebugKotlin all green.
Audit findings deferred to follow-up commits (none ship-blocking):
- Wire layout pinning vs draft-17 vs draft-11 (currently emits a
draft-11-style OBJECT_DATAGRAM; we advertise draft-17). Resolve
via the M4 manual interop pass against nostrnests.
- UNANNOUNCE racing inbound SUBSCRIBE; control-pump cancel half-write
(audit MoQ #5, #6) — small ordering tweaks.
- Two retry coroutines stacking under fast Failed bursts (audit VM #4).
- AudioRoomForegroundService startForeground always-first contract
(audit Android #8).
This commit is contained in:
+51
-11
@@ -44,6 +44,7 @@ import kotlinx.collections.immutable.ImmutableSet
|
||||
import kotlinx.collections.immutable.persistentSetOf
|
||||
import kotlinx.collections.immutable.toPersistentSet
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.GlobalScope
|
||||
import kotlinx.coroutines.Job
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
@@ -96,6 +97,13 @@ class AudioRoomViewModel(
|
||||
// listener whose state they can drive directly.
|
||||
private val connector: NestsListenerConnector = DefaultNestsListenerConnector,
|
||||
private val speakerConnector: NestsSpeakerConnector = DefaultNestsSpeakerConnector,
|
||||
// Scope used for fire-and-forget MoQ cleanup (UNANNOUNCE,
|
||||
// SUBSCRIBE_DONE, MoQ session close) that needs to outlive the VM's
|
||||
// own scope. Production passes [GlobalScope] so onCleared can finish
|
||||
// sending control frames before the QUIC transport drops; tests pass
|
||||
// their `backgroundScope` so assertions can observe the close.
|
||||
@OptIn(kotlinx.coroutines.DelicateCoroutinesApi::class)
|
||||
private val cleanupScope: CoroutineScope = GlobalScope,
|
||||
) : ViewModel() {
|
||||
private val _uiState = MutableStateFlow(AudioRoomUiState())
|
||||
val uiState: StateFlow<AudioRoomUiState> = _uiState.asStateFlow()
|
||||
@@ -278,16 +286,16 @@ class AudioRoomViewModel(
|
||||
autoRetryJob?.cancel()
|
||||
autoRetryJob = null
|
||||
autoRetryAttempts = 0
|
||||
teardownBroadcast(BroadcastUiState.Idle)
|
||||
teardown(targetState = ConnectionUiState.Idle)
|
||||
teardownBroadcast(BroadcastUiState.Idle, finalCleanup = false)
|
||||
teardown(targetState = ConnectionUiState.Idle, finalCleanup = false)
|
||||
}
|
||||
|
||||
override fun onCleared() {
|
||||
closed = true
|
||||
autoRetryJob?.cancel()
|
||||
autoRetryJob = null
|
||||
teardownBroadcast(BroadcastUiState.Idle)
|
||||
teardown(targetState = ConnectionUiState.Closed)
|
||||
teardownBroadcast(BroadcastUiState.Idle, finalCleanup = true)
|
||||
teardown(targetState = ConnectionUiState.Closed, finalCleanup = true)
|
||||
super.onCleared()
|
||||
}
|
||||
|
||||
@@ -313,7 +321,10 @@ class AudioRoomViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
private fun teardownBroadcast(targetState: BroadcastUiState) {
|
||||
private fun teardownBroadcast(
|
||||
targetState: BroadcastUiState,
|
||||
finalCleanup: Boolean = false,
|
||||
) {
|
||||
speakerConnectJob?.cancel()
|
||||
speakerConnectJob = null
|
||||
speakerStateJob?.cancel()
|
||||
@@ -323,7 +334,12 @@ class AudioRoomViewModel(
|
||||
broadcastHandle = null
|
||||
speaker = null
|
||||
if (handle != null || s != null) {
|
||||
viewModelScope.launch {
|
||||
// User-driven disconnect can use viewModelScope (still alive);
|
||||
// onCleared must use cleanupScope because viewModelScope is
|
||||
// already cancelled and a launch on it would no-op without
|
||||
// emitting the UNANNOUNCE / SUBSCRIBE_DONE wire frames.
|
||||
val scope = if (finalCleanup) cleanupScope else viewModelScope
|
||||
scope.launch {
|
||||
handle?.runCatching { close() }
|
||||
s?.runCatching { close() }
|
||||
}
|
||||
@@ -349,6 +365,14 @@ class AudioRoomViewModel(
|
||||
scheduleAutoRetry()
|
||||
}
|
||||
|
||||
// Server-initiated Closed: reset the retry counter and
|
||||
// tear down stale local state so any later user-driven
|
||||
// reconnect starts fresh.
|
||||
NestsListenerState.Closed -> {
|
||||
autoRetryAttempts = 0
|
||||
teardown(targetState = ConnectionUiState.Closed)
|
||||
}
|
||||
|
||||
else -> { /* no extra side effect */ }
|
||||
}
|
||||
}
|
||||
@@ -467,6 +491,15 @@ class AudioRoomViewModel(
|
||||
if (closed || activeSubscriptions[pubkey] !== slot) return
|
||||
try {
|
||||
val handle = l.subscribeSpeaker(pubkey)
|
||||
// Re-check after the suspending subscribeSpeaker — the user
|
||||
// may have removed this speaker via updateSpeakers / disconnected
|
||||
// while the SUBSCRIBE was in flight. If so, abandon the handle
|
||||
// cleanly (fire-and-forget UNSUBSCRIBE) instead of attaching it
|
||||
// to a slot the reconcile loop has already discarded.
|
||||
if (closed || activeSubscriptions[pubkey] !== slot) {
|
||||
viewModelScope.launch { runCatching { handle.unsubscribe() } }
|
||||
return
|
||||
}
|
||||
val decoder = decoderFactory()
|
||||
val player = playerFactory()
|
||||
val isMuted = _uiState.value.isMuted
|
||||
@@ -491,7 +524,10 @@ class AudioRoomViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
private fun teardown(targetState: ConnectionUiState) {
|
||||
private fun teardown(
|
||||
targetState: ConnectionUiState,
|
||||
finalCleanup: Boolean = false,
|
||||
) {
|
||||
connectJob?.cancel()
|
||||
connectJob = null
|
||||
stateObserverJob?.cancel()
|
||||
@@ -505,10 +541,14 @@ class AudioRoomViewModel(
|
||||
val l = listener
|
||||
listener = null
|
||||
if (l != null) {
|
||||
// Closing the listener is suspending; fire-and-forget on the VM
|
||||
// scope is fine — even if the scope is cancelled (onCleared), the
|
||||
// underlying transport's own cleanup runs.
|
||||
viewModelScope.launch { runCatching { l.close() } }
|
||||
// Listener.close() sends MoQ control frames (UNSUBSCRIBE etc.)
|
||||
// before the QUIC transport drops. User-driven disconnect uses
|
||||
// the still-alive viewModelScope; onCleared has already had its
|
||||
// viewModelScope cancelled, so it routes through cleanupScope
|
||||
// (a process-lived scope) so the peer sees a clean teardown
|
||||
// rather than a hard QUIC drop.
|
||||
val scope = if (finalCleanup) cleanupScope else viewModelScope
|
||||
scope.launch { runCatching { l.close() } }
|
||||
}
|
||||
_uiState.update {
|
||||
it.copy(
|
||||
|
||||
Reference in New Issue
Block a user