fix: add closed guards to WebRTC callbacks, connecting watchdog, account-switch safety
Three additional hardening fixes: 1. WebRTC callback guards: Every callback passed to WebRtcCallSession (onIceCandidate, onPeerConnected, onRemoteVideoTrack, onDisconnected, onError, onRenegotiationNeeded, onIceRestartOffer) now checks `closed` before touching any session resource. Prevents native crashes from libwebrtc callbacks firing after close() has disposed PeerConnections. 2. Connecting-state watchdog: New 30-second timer armed when entering Connecting state, disarmed on Connected/Ended/reset. If ICE negotiation hangs (broken TURN, restrictive NAT), the call ends with TIMEOUT instead of leaving the user on a "Connecting..." screen forever. 3. Account-switch safety: CallSessionBridge.clear() now calls callManager.hangup() before nulling references. Prevents a stale CallSession from invoking signing/publishing lambdas on a disposed Account after logout or account switch. https://claude.ai/code/session_019yNnDjGKmJb19gadmojq54
This commit is contained in:
@@ -21,7 +21,12 @@
|
|||||||
package com.vitorpamplona.amethyst.service.call
|
package com.vitorpamplona.amethyst.service.call
|
||||||
|
|
||||||
import com.vitorpamplona.amethyst.commons.call.CallManager
|
import com.vitorpamplona.amethyst.commons.call.CallManager
|
||||||
|
import com.vitorpamplona.amethyst.commons.call.CallState
|
||||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
|
import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
|
||||||
|
import kotlinx.coroutines.CoroutineScope
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
import kotlinx.coroutines.launch
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Process-level singleton that bridges the active [CallManager] and
|
* Process-level singleton that bridges the active [CallManager] and
|
||||||
@@ -47,7 +52,26 @@ object CallSessionBridge {
|
|||||||
this.accountViewModel = accountViewModel
|
this.accountViewModel = accountViewModel
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Terminates any active call and clears all references. Called from
|
||||||
|
* [AccountViewModel.onCleared] during logout or account switch so
|
||||||
|
* that a stale CallSession cannot invoke signing/publishing lambdas
|
||||||
|
* on a disposed Account.
|
||||||
|
*/
|
||||||
fun clear() {
|
fun clear() {
|
||||||
|
val mgr = callManager
|
||||||
|
if (mgr != null) {
|
||||||
|
val state = mgr.state.value
|
||||||
|
if (state is CallState.IncomingCall ||
|
||||||
|
state is CallState.Offering ||
|
||||||
|
state is CallState.Connecting ||
|
||||||
|
state is CallState.Connected
|
||||||
|
) {
|
||||||
|
CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate).launch {
|
||||||
|
mgr.hangup()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
callManager = null
|
callManager = null
|
||||||
accountViewModel = null
|
accountViewModel = null
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -589,22 +589,36 @@ class CallSession(
|
|||||||
WebRtcCallSession(
|
WebRtcCallSession(
|
||||||
peerConnectionFactory = factory,
|
peerConnectionFactory = factory,
|
||||||
iceServers = IceServerConfig.buildIceServers(settingsProvider().callTurnServers),
|
iceServers = IceServerConfig.buildIceServers(settingsProvider().callTurnServers),
|
||||||
onIceCandidate = { candidate -> onLocalIceCandidate(peerPubKey, candidate) },
|
onIceCandidate = { candidate ->
|
||||||
|
if (!closed) onLocalIceCandidate(peerPubKey, candidate)
|
||||||
|
},
|
||||||
onPeerConnected = {
|
onPeerConnected = {
|
||||||
|
if (closed) return@WebRtcCallSession
|
||||||
Log.d(TAG) { "Peer ${peerPubKey.take(8)} connected!" }
|
Log.d(TAG) { "Peer ${peerPubKey.take(8)} connected!" }
|
||||||
scope.launch {
|
scope.launch {
|
||||||
|
if (closed) return@launch
|
||||||
callManager.onPeerConnected()
|
callManager.onPeerConnected()
|
||||||
if (callManager.state.value is CallState.Connected) {
|
if (callManager.state.value is CallState.Connected) {
|
||||||
ensureForegroundService()
|
ensureForegroundService()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
onRemoteVideoTrack = { track -> videoMonitor.onRemoteVideoTrack(peerPubKey, track) },
|
onRemoteVideoTrack = { track ->
|
||||||
onDisconnected = { scope.launch { onPeerDisconnected(peerPubKey) } },
|
if (!closed) videoMonitor.onRemoteVideoTrack(peerPubKey, track)
|
||||||
onError = { error -> _errorMessage.value = error },
|
},
|
||||||
onRenegotiationNeeded = { performRenegotiation(peerPubKey) },
|
onDisconnected = {
|
||||||
|
if (!closed) scope.launch { onPeerDisconnected(peerPubKey) }
|
||||||
|
},
|
||||||
|
onError = { error ->
|
||||||
|
if (!closed) _errorMessage.value = error
|
||||||
|
},
|
||||||
|
onRenegotiationNeeded = {
|
||||||
|
if (!closed) performRenegotiation(peerPubKey)
|
||||||
|
},
|
||||||
onIceRestartOffer = { sdp ->
|
onIceRestartOffer = { sdp ->
|
||||||
scope.launch { callManager.sendRenegotiation(sdp.description, peerPubKey) }
|
if (!closed) {
|
||||||
|
scope.launch { callManager.sendRenegotiation(sdp.description, peerPubKey) }
|
||||||
|
}
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
try {
|
try {
|
||||||
|
|||||||
+37
-1
@@ -113,6 +113,7 @@ class CallManager(
|
|||||||
*/
|
*/
|
||||||
private val watchdogScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
private val watchdogScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||||
private var ringingWatchdogJob: Job? = null
|
private var ringingWatchdogJob: Job? = null
|
||||||
|
private var connectingWatchdogJob: Job? = null
|
||||||
|
|
||||||
/** Per-peer invite timeout jobs. A separate 30-second timer is scheduled
|
/** Per-peer invite timeout jobs. A separate 30-second timer is scheduled
|
||||||
* for each peer we are waiting on (initial group-call offerees and
|
* for each peer we are waiting on (initial group-call offerees and
|
||||||
@@ -152,6 +153,7 @@ class CallManager(
|
|||||||
companion object {
|
companion object {
|
||||||
const val CALL_TIMEOUT_MS = 60_000L // 60 seconds ringing timeout (callee side)
|
const val CALL_TIMEOUT_MS = 60_000L // 60 seconds ringing timeout (callee side)
|
||||||
const val PEER_INVITE_TIMEOUT_MS = 30_000L // 30 seconds per-peer invite timeout (caller side)
|
const val PEER_INVITE_TIMEOUT_MS = 30_000L // 30 seconds per-peer invite timeout (caller side)
|
||||||
|
const val CONNECTING_TIMEOUT_MS = 30_000L // 30 seconds to establish ICE connection
|
||||||
const val ENDED_DISPLAY_MS = 2_000L // show "call ended" briefly before resetting
|
const val ENDED_DISPLAY_MS = 2_000L // show "call ended" briefly before resetting
|
||||||
const val MAX_EVENT_AGE_SECONDS = 20L // discard signaling events older than this
|
const val MAX_EVENT_AGE_SECONDS = 20L // discard signaling events older than this
|
||||||
const val MAX_PROCESSED_EVENT_IDS = 2_000 // cap dedup set to prevent unbounded growth
|
const val MAX_PROCESSED_EVENT_IDS = 2_000 // cap dedup set to prevent unbounded growth
|
||||||
@@ -415,6 +417,7 @@ class CallManager(
|
|||||||
)
|
)
|
||||||
cancelTimeout()
|
cancelTimeout()
|
||||||
disarmRingingWatchdog()
|
disarmRingingWatchdog()
|
||||||
|
armConnectingWatchdog(current.callId)
|
||||||
// Local watchdog timers only — we are not the inviter for these
|
// Local watchdog timers only — we are not the inviter for these
|
||||||
// peers, so [handlePeerTimeout] will silently drop them without
|
// peers, so [handlePeerTimeout] will silently drop them without
|
||||||
// publishing any hangup (see `peersInvitedByUs`).
|
// publishing any hangup (see `peersInvitedByUs`).
|
||||||
@@ -512,7 +515,8 @@ class CallManager(
|
|||||||
// still in `pending` keep theirs (scheduled in beginOffering).
|
// still in `pending` keep theirs (scheduled in beginOffering).
|
||||||
cancelPeerTimeout(answeringPeer)
|
cancelPeerTimeout(answeringPeer)
|
||||||
disarmRingingWatchdog()
|
disarmRingingWatchdog()
|
||||||
Log.d("CallManager") { "onCallAnswered: Offering -> Connecting, forwarding answer to CallController" }
|
armConnectingWatchdog(current.callId)
|
||||||
|
Log.d("CallManager") { "onCallAnswered: Offering -> Connecting, forwarding answer to CallSession" }
|
||||||
onAnswerReceived?.invoke(event)
|
onAnswerReceived?.invoke(event)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -710,6 +714,7 @@ class CallManager(
|
|||||||
}
|
}
|
||||||
|
|
||||||
Log.d("CallManager") { "onPeerConnected: Connecting -> Connected! callId=${current.callId}" }
|
Log.d("CallManager") { "onPeerConnected: Connecting -> Connected! callId=${current.callId}" }
|
||||||
|
disarmConnectingWatchdog()
|
||||||
_state.value =
|
_state.value =
|
||||||
CallState.Connected(
|
CallState.Connected(
|
||||||
callId = current.callId,
|
callId = current.callId,
|
||||||
@@ -998,6 +1003,7 @@ class CallManager(
|
|||||||
cancelTimeout()
|
cancelTimeout()
|
||||||
cancelAllPeerTimeouts()
|
cancelAllPeerTimeouts()
|
||||||
disarmRingingWatchdog()
|
disarmRingingWatchdog()
|
||||||
|
disarmConnectingWatchdog()
|
||||||
resetJob?.cancel()
|
resetJob?.cancel()
|
||||||
resetJob = null
|
resetJob = null
|
||||||
processedEventIds.clear()
|
processedEventIds.clear()
|
||||||
@@ -1017,6 +1023,7 @@ class CallManager(
|
|||||||
cancelTimeout()
|
cancelTimeout()
|
||||||
cancelAllPeerTimeouts()
|
cancelAllPeerTimeouts()
|
||||||
disarmRingingWatchdog()
|
disarmRingingWatchdog()
|
||||||
|
disarmConnectingWatchdog()
|
||||||
resetJob?.cancel()
|
resetJob?.cancel()
|
||||||
resetJob =
|
resetJob =
|
||||||
scope.launch {
|
scope.launch {
|
||||||
@@ -1065,12 +1072,41 @@ class CallManager(
|
|||||||
ringingWatchdogJob = null
|
ringingWatchdogJob = null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---- Connecting watchdog ----
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Arms a timer for [callId] in `Connecting` state. If ICE negotiation
|
||||||
|
* does not promote the state to `Connected` within
|
||||||
|
* [CONNECTING_TIMEOUT_MS], the call ends with [EndReason.TIMEOUT].
|
||||||
|
* Disarmed when entering `Connected`, `Ended`, or `Idle`.
|
||||||
|
*/
|
||||||
|
private fun armConnectingWatchdog(callId: String) {
|
||||||
|
connectingWatchdogJob?.cancel()
|
||||||
|
connectingWatchdogJob =
|
||||||
|
watchdogScope.launch {
|
||||||
|
delay(CONNECTING_TIMEOUT_MS)
|
||||||
|
stateMutex.withLock {
|
||||||
|
val cur = _state.value
|
||||||
|
if (cur is CallState.Connecting && cur.callId == callId) {
|
||||||
|
Log.d("CallManager") { "Connecting watchdog fired for $callId — forcing Ended(TIMEOUT)" }
|
||||||
|
transitionToEnded(callId, cur.peerPubKeys + cur.pendingPeerPubKeys, EndReason.TIMEOUT)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun disarmConnectingWatchdog() {
|
||||||
|
connectingWatchdogJob?.cancel()
|
||||||
|
connectingWatchdogJob = null
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Cancels all long-lived coroutines owned by this manager. Called when
|
* Cancels all long-lived coroutines owned by this manager. Called when
|
||||||
* the owning account scope is torn down (logout / process shutdown).
|
* the owning account scope is torn down (logout / process shutdown).
|
||||||
*/
|
*/
|
||||||
fun dispose() {
|
fun dispose() {
|
||||||
disarmRingingWatchdog()
|
disarmRingingWatchdog()
|
||||||
|
disarmConnectingWatchdog()
|
||||||
watchdogScope.cancel()
|
watchdogScope.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user