From fe13b77e22b9be5592ab44377acbdce6e6ec6226 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Apr 2026 18:16:09 +0000 Subject: [PATCH] fix: harden call cleanup to guarantee nothing is left running Three issues addressed: 1. Make cleanup() exception-safe: wrap each WebRTC dispose call in its own try-catch so that a failure in one (e.g. native crash disposing a PeerConnection) does not skip releasing the camera, audio mode, foreground service, or EGL context. 2. Upgrade the Idle safety net to call cleanup() instead of only stopping ringing/notifications. If the Ended state is missed due to StateFlow conflation, the Idle handler now performs full resource cleanup (camera, WebRTC, audio mode, foreground service, proximity wake lock). cleanup() is idempotent since all resources are null-checked and nulled out. 3. Call cleanup() in AccountViewModel.onCleared() so that WebRTC resources, camera, audio mode, and foreground service are released when the ViewModel is destroyed (e.g. logout, activity recreation). 4. Clear processedEventIds when transitioning to Idle to prevent unbounded memory growth across calls. https://claude.ai/code/session_01GWRdrVAa29BsDkv8R7Z3Y9 --- .../amethyst/service/call/CallController.kt | 80 +++++++++++++++---- .../ui/screen/loggedIn/AccountViewModel.kt | 1 + .../amethyst/commons/call/CallManager.kt | 1 + 3 files changed, 65 insertions(+), 17 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 1c86493d9..7788e9a39 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 @@ -193,12 +193,11 @@ class CallController( } is CallState.Idle -> { - // Safety net: ensure ringing and notifications are - // stopped even if the Ended state was missed due to - // StateFlow conflation. - audioManager.stopRinging() - audioManager.stopRingbackTone() - NotificationUtils.cancelCallNotification(context) + // Safety net: full cleanup in case the Ended state + // was missed due to StateFlow conflation. cleanup() + // is idempotent — calling it twice is harmless because + // each resource is null-checked and nulled out. + cleanup() } } } @@ -737,24 +736,71 @@ class CallController( // ---- Cleanup ---- fun cleanup() { - audioManager.release() - stopForegroundService() + // Each block is wrapped individually so that a failure in one + // (e.g. a WebRTC native crash) does not prevent the rest from + // running. Without this, a single exception could leave the + // camera open, audio mode stuck, or the foreground service alive. + try { + audioManager.release() + } catch (e: Exception) { + Log.e(TAG, "cleanup: audioManager.release() failed", e) + } + try { + stopForegroundService() + } catch (e: Exception) { + Log.e(TAG, "cleanup: stopForegroundService() failed", e) + } foregroundServiceStarted = false NotificationUtils.cancelCallNotification(context) stopRemoteVideoMonitor() // Dispose all peer sessions - peerSessions.values.forEach { it.session.dispose() } + for (ps in peerSessions.values) { + try { + ps.session.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: PeerSession.dispose() failed", e) + } + } peerSessions.clear() - // Dispose shared resources - stopCamera() - localAudioTrackInternal?.dispose() - localVideoTrackInternal?.dispose() - localAudioSource?.dispose() - localVideoSource?.dispose() - peerConnectionFactory?.dispose() - sharedEglBase?.release() + // Dispose shared resources — each in its own try-catch so one + // failure does not prevent the others from being released. + try { + stopCamera() + } catch (e: Exception) { + Log.e(TAG, "cleanup: stopCamera() failed", e) + } + try { + localAudioTrackInternal?.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: localAudioTrack.dispose() failed", e) + } + try { + localVideoTrackInternal?.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: localVideoTrack.dispose() failed", e) + } + try { + localAudioSource?.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: localAudioSource.dispose() failed", e) + } + try { + localVideoSource?.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: localVideoSource.dispose() failed", e) + } + try { + peerConnectionFactory?.dispose() + } catch (e: Exception) { + Log.e(TAG, "cleanup: peerConnectionFactory.dispose() failed", e) + } + try { + sharedEglBase?.release() + } catch (e: Exception) { + Log.e(TAG, "cleanup: sharedEglBase.release() failed", e) + } localAudioTrackInternal = null localVideoTrackInternal = null diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 0227d79ee..73c0f12f8 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1429,6 +1429,7 @@ class AccountViewModel( override fun onCleared() { Log.d("AccountViewModel", "onCleared") + callController?.cleanup() feedStates.destroy() super.onCleared() } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt index faac810c4..a872d5230 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/call/CallManager.kt @@ -606,6 +606,7 @@ class CallManager( delay(ENDED_DISPLAY_MS) if (_state.value is CallState.Ended) { _state.value = CallState.Idle + processedEventIds.clear() } } }