diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivity.kt index 3fc0aeeca..ec919d284 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivity.kt @@ -38,6 +38,7 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.ui.theme.AmethystTheme import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.asSharedFlow /** @@ -72,7 +73,7 @@ class AudioRoomActivity : AppCompatActivity() { // and forwarded to the VM. Replaces an earlier // process-wide singleton that could leak edges across // activity instances. - toggleMuteSignal.tryEmit(Unit) + _toggleMuteSignal.tryEmit(Unit) } ACTION_PIP_LEAVE -> { @@ -85,8 +86,12 @@ class AudioRoomActivity : AppCompatActivity() { private val _toggleMuteSignal = MutableSharedFlow(replay = 0, extraBufferCapacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) - /** Emitted when the PIP-overlay mute action is tapped. */ - val toggleMuteSignal: MutableSharedFlow get() = _toggleMuteSignal + /** + * Emitted when the PIP-overlay mute action is tapped. Read-only so + * external code can't tryEmit into it from outside the Activity + * (audit round-2 Android #2). + */ + val toggleMuteSignal: SharedFlow get() = _toggleMuteSignal.asSharedFlow() override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -144,7 +149,7 @@ class AudioRoomActivity : AppCompatActivity() { } }, onConnectedChange = { isConnected.value = it }, - pipMuteSignal = toggleMuteSignal.asSharedFlow(), + pipMuteSignal = toggleMuteSignal, onLeave = { finish() }, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivityContent.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivityContent.kt index a1f3604f1..1627f063e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivityContent.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/audiorooms/room/AudioRoomActivityContent.kt @@ -184,9 +184,16 @@ private fun AudioRoomActivityBody( // PRESENCE_DEBOUNCE_MS for further changes before sending a fresh // presence event. The LaunchedEffect's auto-cancel on key change // serves as the debounce mechanism. - LaunchedEffect(micMutedTag) { - delay(PRESENCE_DEBOUNCE_MS) - publishPresence(account, event, handRaised, micMutedTag) + // + // Skip when `micMutedTag` is null (we're not broadcasting) — otherwise + // the FIRST composition would publish twice within 500 ms (heartbeat + // immediately + debounce-publisher 500 ms later, both with muted=null). + // Audit round-2 Android #10. + if (micMutedTag != null) { + LaunchedEffect(micMutedTag) { + delay(PRESENCE_DEBOUNCE_MS) + publishPresence(account, event, handRaised, micMutedTag) + } } DisposableEffect(event.address().toValue()) { onDispose { diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/AudioRoomViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/AudioRoomViewModel.kt index f8847ee8a..a9e9087ed 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/AudioRoomViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/AudioRoomViewModel.kt @@ -76,6 +76,17 @@ import kotlin.coroutines.cancellation.CancellationException * [playerFactory] so commonMain doesn't have to know which platform's * MediaCodec / AudioTrack is in play. M1 wires Android-only — desktop * passes nothing here yet. + * + * **Threading contract:** all public methods (`connect`, `disconnect`, + * `updateSpeakers`, `setMuted`, `setMicMuted`, `startBroadcast`, + * `stopBroadcast`) MUST be called on the main thread. Internal mutation + * of `activeSubscriptions` / `speakingExpiryJobs` / `requestedSpeakers` + * is only thread-safe under that assumption — they're plain HashMaps + * confined to `viewModelScope`'s dispatcher (Dispatchers.Main.immediate + * on Android, which is the same dispatcher the MoQ flow's `onEach` + * callback runs on because the player launch lives in viewModelScope). + * If a future caller needs to invoke from a background thread, marshal + * via `viewModelScope.launch(Dispatchers.Main.immediate) { ... }` first. */ @Stable class AudioRoomViewModel( @@ -112,6 +123,12 @@ class AudioRoomViewModel( private var connectJob: Job? = null private var stateObserverJob: Job? = null + // Last in-flight `listener.close()` launched by teardown(). A subsequent + // connect() awaits this before opening a fresh transport so two QUIC + // sessions (the closing old one and the opening new one) don't briefly + // coexist. Some servers dedupe by client pubkey and reject the new one. + private var pendingCloseJob: Job? = null + private val activeSubscriptions = mutableMapOf() private val speakingExpiryJobs = mutableMapOf() private var requestedSpeakers: Set = emptySet() @@ -175,34 +192,7 @@ class AudioRoomViewModel( teardown(targetState = ConnectionUiState.Idle, finalCleanup = false) } - _uiState.update { it.copy(connection = ConnectionUiState.Connecting(ConnectionUiState.Step.ResolvingRoom)) } - - connectJob = - viewModelScope.launch { - try { - val l = - connector.connect( - httpClient = httpClient, - transport = transport, - scope = viewModelScope, - serviceBase = serviceBase, - roomId = roomId, - signer = signer, - ) - if (closed) { - runCatching { l.close() } - return@launch - } - listener = l - observeListenerState(l) - } catch (ce: CancellationException) { - throw ce - } catch (t: Throwable) { - _uiState.update { - it.copy(connection = ConnectionUiState.Failed(t.message ?: t::class.simpleName ?: "connect failed")) - } - } - } + launchConnect(triggerRetryOnFailure = false) } fun setMuted(muted: Boolean) { @@ -293,19 +283,22 @@ class AudioRoomViewModel( if (closed) return val handle = broadcastHandle ?: return viewModelScope.launch { - handle - .runCatching { setMuted(muted) } - .onSuccess { - if (closed) return@onSuccess - _uiState.update { - val current = it.broadcast - if (current is BroadcastUiState.Broadcasting) { - it.copy(broadcast = current.copy(isMuted = muted)) - } else { - it - } - } + val result = handle.runCatching { setMuted(muted) } + if (closed) return@launch + _uiState.update { + val current = it.broadcast + if (current !is BroadcastUiState.Broadcasting) return@update it + if (result.isSuccess) { + it.copy(broadcast = current.copy(isMuted = muted, muteError = null)) + } else { + // Surface the failure so the UI can show a toast / inline + // message instead of silently swallowing (audit round-2 + // VM #8b). We keep the broadcast running with the prior + // mute state — only the mute toggle failed. + val why = result.exceptionOrNull()?.message ?: "mute failed" + it.copy(broadcast = current.copy(muteError = why)) } + } } } @@ -462,12 +455,32 @@ class AudioRoomViewModel( if (closed) return val current = _uiState.value.connection if (current is ConnectionUiState.Connecting || current is ConnectionUiState.Connected) return + launchConnect(triggerRetryOnFailure = true) + } + /** + * Shared connect-launch body for [connect] (user-driven, no retry on + * failure) and [connectInternal] (auto-retry path, schedules another + * retry on failure). Awaits any in-flight `listener.close()` from a + * previous teardown so two QUIC sessions don't briefly coexist + * (audit round-2 VM #10), then runs the connector and observes the + * resulting listener. + */ + private fun launchConnect(triggerRetryOnFailure: Boolean) { _uiState.update { it.copy(connection = ConnectionUiState.Connecting(ConnectionUiState.Step.ResolvingRoom)) } + val priorClose = pendingCloseJob + pendingCloseJob = null + connectJob = viewModelScope.launch { try { + // Drain the previous listener's close before opening a + // new transport. This is fast (< 100 ms typical: send + // UNSUBSCRIBE / UNANNOUNCE / WT_CLOSE_SESSION + drain). + if (priorClose?.isActive == true) { + runCatching { priorClose.join() } + } val l = connector.connect( httpClient = httpClient, @@ -489,7 +502,7 @@ class AudioRoomViewModel( _uiState.update { it.copy(connection = ConnectionUiState.Failed(t.message ?: t::class.simpleName ?: "connect failed")) } - scheduleAutoRetry() + if (triggerRetryOnFailure) scheduleAutoRetry() } } } @@ -629,7 +642,11 @@ class AudioRoomViewModel( // before the QUIC transport drops. Same scope rule as the // player stop above: viewModelScope when alive, cleanupScope // for onCleared so the wire teardown survives. - scope.launch { runCatching { l.close() } } + // Track the close so the next connect() can await it (audit + // round-2 VM #10). If `finalCleanup` is true we don't bother + // — the VM is going away and no future connect() will run. + val closeLaunch = scope.launch { runCatching { l.close() } } + if (!finalCleanup) pendingCloseJob = closeLaunch } _uiState.update { it.copy( @@ -787,6 +804,13 @@ sealed class BroadcastUiState { data class Broadcasting( val isMuted: Boolean, + /** + * Non-null when the most recent mute toggle failed (e.g. handle + * setMuted threw). UI surfaces this as an inline message; the + * broadcast itself stays running with the previous mute state. + * Cleared on the next successful mute toggle. + */ + val muteError: String? = null, ) : BroadcastUiState() data class Failed( diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/MoqSession.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/MoqSession.kt index 9273cf7e8..eea2bc215 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/MoqSession.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/MoqSession.kt @@ -834,10 +834,11 @@ class MoqSession private constructor( .onSuccess { anySent = true } } // Roll the objectId back if the entire fan-out failed (e.g. - // transport down). Audio-rooms NIP wants strictly contiguous - // object ids per group; a gap from a fully-failed send would - // trip strict subscribers. We roll back only if the next send - // hasn't already grabbed an id past us. + // transport down). The audio-rooms NIP wants object ids + // monotonic; gaps from real network loss are acceptable per + // spec, but a gap caused by a fully-failed local send is + // pure noise we can avoid. We only roll back if no + // concurrent send has already grabbed an id past us. if (!anySent) { stateMutex.withLock { if (nextObjectId == objectId + 1) nextObjectId = objectId