From 0fbe61274aa24369e37f1d3aadd4e6eb3df9fdd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Apr 2026 22:16:17 +0000 Subject: [PATCH] fix(call): release WebRTC resources synchronously on CallActivity teardown The normal cleanup path routes through CallManager -> state=Ended -> the state-collector in viewModelScope -> CallController.cleanup(). If the Activity is destroyed before the collector finishes (task removal, PiP dismissal, system kill), the async path can leave PeerConnections, the camera capturer, the proximity wake lock, and the audio-mode change alive until AccountViewModel.onCleared runs later. CallActivity.onDestroy / onStop now call CallController.cleanup() directly as a synchronous safety net, in addition to publishing the hangup/reject signaling on a detached scope. To make that safe, CallController.cleanup() is now truly idempotent within a call session: the cleanedUp flag stays sticky once set and is re-armed only when a new call starts via initiateGroupCall() or acceptIncomingCall(). This prevents a second sequential cleanup() call from double-disposing WebRTC objects when both the state-collector path and the Activity safety net run. https://claude.ai/code/session_01XSPDbahLwHs9sdF5XfRvHB --- .../amethyst/service/call/CallController.kt | 22 ++++++++++- .../amethyst/ui/call/CallActivity.kt | 37 +++++++++++++++---- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallController.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallController.kt index 6132fed28..c38d161a8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallController.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/call/CallController.kt @@ -212,6 +212,9 @@ class CallController( val callId = UUID.randomUUID().toString() _errorMessage.value = null + // A new call session begins — arm cleanup() so it will run again + // for this session's Ended transition. + cleanedUp.set(false) applyCallSettings() try { withContext(Dispatchers.IO) { mediaManager.initialize(callType) } @@ -258,6 +261,9 @@ class CallController( scope.launch { _errorMessage.value = null + // A new call session begins — arm cleanup() so it will run again + // for this session's Ended transition. + cleanedUp.set(false) applyCallSettings() try { withContext(Dispatchers.IO) { mediaManager.initialize(state.callType) } @@ -612,6 +618,17 @@ class CallController( // ---- Cleanup ---- + /** + * Releases all WebRTC, audio, and foreground-service resources for the + * current call session. + * + * Idempotent within a call session: calling this multiple times in a row + * (e.g. once from the Ended state collector and once from [CallActivity]'s + * onDestroy safety net) executes the body at most once. The guard is + * re-armed when a new call starts via [initiateGroupCall] or + * [acceptIncomingCall], so subsequent call sessions still clean up + * correctly. + */ fun cleanup() { if (!cleanedUp.compareAndSet(false, true)) return unregisterNetworkCallback() @@ -643,7 +660,10 @@ class CallController( videoPausedByProximity = false videoSenders.clear() pendingRenegotiation.clear() - cleanedUp.set(false) + // NOTE: cleanedUp intentionally stays true here so that a second, + // sequential cleanup() in the same call session is a no-op. The flag + // is re-armed in initiateGroupCall() / acceptIncomingCall() when a new + // call session begins. } // ---- Foreground service ---- diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/CallActivity.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/CallActivity.kt index 3a1fbb8ae..e6560b7bd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/CallActivity.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/call/CallActivity.kt @@ -202,28 +202,40 @@ class CallActivity : AppCompatActivity() { if (wasInPipMode && !isInPictureInPictureMode) { hangupInitiated = true val manager = CallSessionBridge.callManager + val controller = CallSessionBridge.callController val state = manager?.state?.value if (state is CallState.Connected || state is CallState.Connecting || state is CallState.Offering) { + // Publish the hangup signaling event on a detached scope so + // it survives activity teardown. CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate).launch { manager.hangup() - finishAndRemoveTask() } - } else { - finishAndRemoveTask() } + // Synchronously release all WebRTC/audio resources before the + // activity is torn down so we don't leak a PeerConnection, + // camera capturer, wake lock, or audio mode change. cleanup() is + // idempotent within a call session, so the state-collector path + // triggered by hangup() above will be a no-op. + controller?.cleanup() + finishAndRemoveTask() } } override fun onDestroy() { unregisterPipReceiver() + val manager = CallSessionBridge.callManager + val controller = CallSessionBridge.callController + // Safety net: if the Activity is destroyed while a call is still - // ringing/offering, ensure the call is hung up so audio stops. - // Skip if onStop already initiated the hangup to avoid double signaling. - if (!hangupInitiated) { - val manager = CallSessionBridge.callManager - when (manager?.state?.value) { + // ringing/offering, publish the reject/hangup signaling so the other + // side stops ringing. Skip if onStop already initiated the hangup to + // avoid double signaling. + if (!hangupInitiated && manager != null) { + when (manager.state.value) { is CallState.IncomingCall -> { + // Publish on a detached scope so the reject event goes out + // even after the activity is gone. CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate).launch { manager.rejectCall() } @@ -242,6 +254,15 @@ class CallActivity : AppCompatActivity() { } } + // Ultimate safety net: synchronously release all WebRTC/audio + // resources before the Activity reference goes away. The normal + // Ended → state-collector → cleanup() path runs asynchronously on + // viewModelScope and is not guaranteed to finish before this method + // returns. CallController.cleanup() is idempotent within a call + // session, so calling it here in addition to the state-collector path + // is safe. + controller?.cleanup() + super.onDestroy() }