fix: call resource leaks and race conditions from out-of-order signaling
- Clear discoveredCalleePeers on transitionToEnded to prevent stale peers from triggering mesh setup in subsequent calls - Cap processedEventIds and completedCallIds with LRU eviction to prevent unbounded memory growth over long app sessions - Store and release SurfaceTextureHelper in startCamera/stopCamera to prevent GL thread and texture leaks - Wrap stopCamera's stopCapture in try-catch for InterruptedException to ensure camera resources are always released - Add Mutex to CallManager to serialize state mutations, preventing races when signaling events arrive concurrently from multiple relays - Add @Synchronized to CallController's toggleAudioMute, toggleVideo, and cleanup to prevent races with WebRTC callbacks https://claude.ai/code/session_01TTPrYjcz1eYEzdSV5N6rHi
This commit is contained in:
@@ -99,6 +99,7 @@ class CallController(
|
|||||||
private var localAudioTrackInternal: AudioTrack? = null
|
private var localAudioTrackInternal: AudioTrack? = null
|
||||||
private var localVideoTrackInternal: VideoTrack? = null
|
private var localVideoTrackInternal: VideoTrack? = null
|
||||||
private var cameraCapturer: CameraVideoCapturer? = null
|
private var cameraCapturer: CameraVideoCapturer? = null
|
||||||
|
private var surfaceTextureHelper: SurfaceTextureHelper? = null
|
||||||
|
|
||||||
private val callFactory = WebRtcCallFactory()
|
private val callFactory = WebRtcCallFactory()
|
||||||
val audioManager = CallAudioManager(context)
|
val audioManager = CallAudioManager(context)
|
||||||
@@ -530,12 +531,14 @@ class CallController(
|
|||||||
|
|
||||||
// ---- UI toggle controls ----
|
// ---- UI toggle controls ----
|
||||||
|
|
||||||
|
@Synchronized
|
||||||
fun toggleAudioMute() {
|
fun toggleAudioMute() {
|
||||||
val muted = !_isAudioMuted.value
|
val muted = !_isAudioMuted.value
|
||||||
_isAudioMuted.value = muted
|
_isAudioMuted.value = muted
|
||||||
localAudioTrackInternal?.setEnabled(!muted)
|
localAudioTrackInternal?.setEnabled(!muted)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Synchronized
|
||||||
fun toggleVideo() {
|
fun toggleVideo() {
|
||||||
val enabling = !_isVideoEnabled.value
|
val enabling = !_isVideoEnabled.value
|
||||||
|
|
||||||
@@ -634,21 +637,24 @@ class CallController(
|
|||||||
val frontCamera = enumerator.deviceNames.firstOrNull { enumerator.isFrontFacing(it) }
|
val frontCamera = enumerator.deviceNames.firstOrNull { enumerator.isFrontFacing(it) }
|
||||||
val camera = frontCamera ?: enumerator.deviceNames.firstOrNull() ?: return
|
val camera = frontCamera ?: enumerator.deviceNames.firstOrNull() ?: return
|
||||||
|
|
||||||
|
val helper = SurfaceTextureHelper.create("CaptureThread", egl.eglBaseContext)
|
||||||
|
surfaceTextureHelper = helper
|
||||||
cameraCapturer =
|
cameraCapturer =
|
||||||
enumerator.createCapturer(camera, null)?.also {
|
enumerator.createCapturer(camera, null)?.also {
|
||||||
it.initialize(
|
it.initialize(helper, context, source.capturerObserver)
|
||||||
SurfaceTextureHelper.create("CaptureThread", egl.eglBaseContext),
|
|
||||||
context,
|
|
||||||
source.capturerObserver,
|
|
||||||
)
|
|
||||||
it.startCapture(1280, 720, 30)
|
it.startCapture(1280, 720, 30)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun stopCamera() {
|
private fun stopCamera() {
|
||||||
|
try {
|
||||||
cameraCapturer?.stopCapture()
|
cameraCapturer?.stopCapture()
|
||||||
|
} catch (_: InterruptedException) {
|
||||||
|
}
|
||||||
cameraCapturer?.dispose()
|
cameraCapturer?.dispose()
|
||||||
cameraCapturer = null
|
cameraCapturer = null
|
||||||
|
surfaceTextureHelper?.dispose()
|
||||||
|
surfaceTextureHelper = null
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- Per-peer PeerConnection creation ----
|
// ---- Per-peer PeerConnection creation ----
|
||||||
@@ -664,7 +670,9 @@ class CallController(
|
|||||||
onIceCandidate = { candidate -> onLocalIceCandidate(peerPubKey, candidate) },
|
onIceCandidate = { candidate -> onLocalIceCandidate(peerPubKey, candidate) },
|
||||||
onPeerConnected = {
|
onPeerConnected = {
|
||||||
Log.d(TAG) { "Peer ${peerPubKey.take(8)} connected!" }
|
Log.d(TAG) { "Peer ${peerPubKey.take(8)} connected!" }
|
||||||
|
scope.launch {
|
||||||
callManager.onPeerConnected()
|
callManager.onPeerConnected()
|
||||||
|
}
|
||||||
if (!foregroundServiceStarted) {
|
if (!foregroundServiceStarted) {
|
||||||
foregroundServiceStarted = true
|
foregroundServiceStarted = true
|
||||||
startForegroundService()
|
startForegroundService()
|
||||||
@@ -780,7 +788,7 @@ class CallController(
|
|||||||
if (entry != null) {
|
if (entry != null) {
|
||||||
Log.d(TAG) { "disposePeerSession: closing session for ${peerPubKey.take(8)}" }
|
Log.d(TAG) { "disposePeerSession: closing session for ${peerPubKey.take(8)}" }
|
||||||
try {
|
try {
|
||||||
entry?.session?.dispose()
|
entry.session.dispose()
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.e(TAG, "disposePeerSession: dispose() failed for ${peerPubKey.take(8)}", e)
|
Log.e(TAG, "disposePeerSession: dispose() failed for ${peerPubKey.take(8)}", e)
|
||||||
}
|
}
|
||||||
@@ -807,6 +815,7 @@ class CallController(
|
|||||||
|
|
||||||
// ---- Cleanup ----
|
// ---- Cleanup ----
|
||||||
|
|
||||||
|
@Synchronized
|
||||||
fun cleanup() {
|
fun cleanup() {
|
||||||
Log.d(TAG) { "cleanup: disposing ${peerSessionMgr.allSessionKeys().size} peer sessions, state=${callManager.state.value::class.simpleName}" }
|
Log.d(TAG) { "cleanup: disposing ${peerSessionMgr.allSessionKeys().size} peer sessions, state=${callManager.state.value::class.simpleName}" }
|
||||||
// Each block is wrapped individually so that a failure in one
|
// Each block is wrapped individually so that a failure in one
|
||||||
|
|||||||
+59
-17
@@ -41,6 +41,8 @@ import kotlinx.coroutines.flow.MutableStateFlow
|
|||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
|
import kotlinx.coroutines.sync.Mutex
|
||||||
|
import kotlinx.coroutines.sync.withLock
|
||||||
|
|
||||||
class CallManager(
|
class CallManager(
|
||||||
private val signer: NostrSigner,
|
private val signer: NostrSigner,
|
||||||
@@ -50,6 +52,12 @@ class CallManager(
|
|||||||
) {
|
) {
|
||||||
private val factory = WebRtcCallFactory()
|
private val factory = WebRtcCallFactory()
|
||||||
|
|
||||||
|
/** Serializes all state-mutating operations. Signaling events can arrive
|
||||||
|
* from multiple relay coroutines concurrently; without this lock a hangup
|
||||||
|
* and an answer could race, causing the answer to overwrite an Ended
|
||||||
|
* transition and leaking WebRTC resources. */
|
||||||
|
private val stateMutex = Mutex()
|
||||||
|
|
||||||
private val _state = MutableStateFlow<CallState>(CallState.Idle)
|
private val _state = MutableStateFlow<CallState>(CallState.Idle)
|
||||||
val state: StateFlow<CallState> = _state.asStateFlow()
|
val state: StateFlow<CallState> = _state.asStateFlow()
|
||||||
|
|
||||||
@@ -69,13 +77,13 @@ class CallManager(
|
|||||||
|
|
||||||
private var timeoutJob: Job? = null
|
private var timeoutJob: Job? = null
|
||||||
private var resetJob: Job? = null
|
private var resetJob: Job? = null
|
||||||
private val processedEventIds = mutableSetOf<String>()
|
private val processedEventIds = LinkedHashSet<String>()
|
||||||
|
|
||||||
/** Call IDs for which we have seen a hangup, reject, or answer-elsewhere
|
/** Call IDs for which we have seen a hangup, reject, or answer-elsewhere
|
||||||
* signal. Checked before transitioning to [CallState.IncomingCall] so
|
* signal. Checked before transitioning to [CallState.IncomingCall] so
|
||||||
* that stale offer events replayed by relays after an app restart do not
|
* that stale offer events replayed by relays after an app restart do not
|
||||||
* trigger ringing for calls that already ended. */
|
* trigger ringing for calls that already ended. */
|
||||||
private val completedCallIds = mutableSetOf<String>()
|
private val completedCallIds = LinkedHashSet<String>()
|
||||||
|
|
||||||
/** Timestamp (epoch seconds) when this CallManager was created. Events
|
/** Timestamp (epoch seconds) when this CallManager was created. Events
|
||||||
* created before this are from a previous app session and should not
|
* created before this are from a previous app session and should not
|
||||||
@@ -90,6 +98,23 @@ class CallManager(
|
|||||||
const val CALL_TIMEOUT_MS = 60_000L // 60 seconds ringing timeout
|
const val CALL_TIMEOUT_MS = 60_000L // 60 seconds ringing timeout
|
||||||
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_COMPLETED_CALL_IDS = 200 // cap completed-call set
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Adds [value] to a [LinkedHashSet], evicting the oldest entries when
|
||||||
|
* the set exceeds [maxSize]. Insertion-order iteration of LinkedHashSet
|
||||||
|
* ensures oldest entries are removed first. */
|
||||||
|
private fun <T> cappedAdd(
|
||||||
|
set: LinkedHashSet<T>,
|
||||||
|
value: T,
|
||||||
|
maxSize: Int,
|
||||||
|
) {
|
||||||
|
set.add(value)
|
||||||
|
while (set.size > maxSize) {
|
||||||
|
val oldest = set.iterator().next()
|
||||||
|
set.remove(oldest)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun isEventTooOld(event: Event): Boolean {
|
private fun isEventTooOld(event: Event): Boolean {
|
||||||
@@ -109,11 +134,11 @@ class CallManager(
|
|||||||
* Sets state to Offering. Called by CallController before creating
|
* Sets state to Offering. Called by CallController before creating
|
||||||
* per-peer offers in group calls.
|
* per-peer offers in group calls.
|
||||||
*/
|
*/
|
||||||
fun beginOffering(
|
suspend fun beginOffering(
|
||||||
callId: String,
|
callId: String,
|
||||||
calleePubKeys: Set<HexKey>,
|
calleePubKeys: Set<HexKey>,
|
||||||
callType: CallType,
|
callType: CallType,
|
||||||
) {
|
) = stateMutex.withLock {
|
||||||
_state.value = CallState.Offering(callId, calleePubKeys, callType)
|
_state.value = CallState.Offering(callId, calleePubKeys, callType)
|
||||||
startTimeout(callId)
|
startTimeout(callId)
|
||||||
}
|
}
|
||||||
@@ -237,15 +262,22 @@ class CallManager(
|
|||||||
}
|
}
|
||||||
|
|
||||||
suspend fun acceptCall(sdpAnswer: String) {
|
suspend fun acceptCall(sdpAnswer: String) {
|
||||||
val current = _state.value
|
val current: CallState.IncomingCall
|
||||||
if (current !is CallState.IncomingCall) {
|
val discovered: Set<HexKey>
|
||||||
Log.d("CallManager") { "acceptCall: state is ${current::class.simpleName}, not IncomingCall — ignoring" }
|
stateMutex.withLock {
|
||||||
|
val s = _state.value
|
||||||
|
if (s !is CallState.IncomingCall) {
|
||||||
|
Log.d("CallManager") { "acceptCall: state is ${s::class.simpleName}, not IncomingCall — ignoring" }
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
current = s
|
||||||
|
|
||||||
Log.d("CallManager") { "acceptCall: callId=${current.callId}, transitioning to Connecting, sdpAnswerLength=${sdpAnswer.length}" }
|
Log.d("CallManager") { "acceptCall: callId=${current.callId}, transitioning to Connecting, sdpAnswerLength=${sdpAnswer.length}" }
|
||||||
_state.value = CallState.Connecting(current.callId, current.peerPubKeys() - signer.pubKey, current.callType)
|
_state.value = CallState.Connecting(current.callId, current.peerPubKeys() - signer.pubKey, current.callType)
|
||||||
cancelTimeout()
|
cancelTimeout()
|
||||||
|
discovered = discoveredCalleePeers.toSet()
|
||||||
|
discoveredCalleePeers.clear()
|
||||||
|
}
|
||||||
|
|
||||||
val allRecipients = current.groupMembers + signer.pubKey
|
val allRecipients = current.groupMembers + signer.pubKey
|
||||||
Log.d("CallManager") { "acceptCall: publishing answer to ${allRecipients.size} recipients" }
|
Log.d("CallManager") { "acceptCall: publishing answer to ${allRecipients.size} recipients" }
|
||||||
@@ -255,8 +287,6 @@ class CallManager(
|
|||||||
|
|
||||||
// Trigger callee-to-callee mesh connections with peers we discovered
|
// Trigger callee-to-callee mesh connections with peers we discovered
|
||||||
// while still ringing (their answers arrived before we accepted).
|
// while still ringing (their answers arrived before we accepted).
|
||||||
val discovered = discoveredCalleePeers.toSet()
|
|
||||||
discoveredCalleePeers.clear()
|
|
||||||
if (discovered.isNotEmpty()) {
|
if (discovered.isNotEmpty()) {
|
||||||
Log.d("CallManager") { "acceptCall: triggering mesh setup with ${discovered.size} discovered peers: ${discovered.map { it.take(8) }}" }
|
Log.d("CallManager") { "acceptCall: triggering mesh setup with ${discovered.size} discovered peers: ${discovered.map { it.take(8) }}" }
|
||||||
for (peer in discovered) {
|
for (peer in discovered) {
|
||||||
@@ -266,10 +296,13 @@ class CallManager(
|
|||||||
}
|
}
|
||||||
|
|
||||||
suspend fun rejectCall() {
|
suspend fun rejectCall() {
|
||||||
val current = _state.value
|
val current: CallState.IncomingCall
|
||||||
if (current !is CallState.IncomingCall) return
|
stateMutex.withLock {
|
||||||
|
val s = _state.value
|
||||||
|
if (s !is CallState.IncomingCall) return
|
||||||
|
current = s
|
||||||
transitionToEnded(current.callId, current.peerPubKeys(), EndReason.REJECTED)
|
transitionToEnded(current.callId, current.peerPubKeys(), EndReason.REJECTED)
|
||||||
|
}
|
||||||
|
|
||||||
val allRecipients = current.groupMembers + signer.pubKey
|
val allRecipients = current.groupMembers + signer.pubKey
|
||||||
val result = factory.createGroupReject(allRecipients, current.callId, signer = signer)
|
val result = factory.createGroupReject(allRecipients, current.callId, signer = signer)
|
||||||
@@ -455,7 +488,8 @@ class CallManager(
|
|||||||
publishEvent(result.wrap)
|
publishEvent(result.wrap)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun onPeerConnected() {
|
suspend fun onPeerConnected() {
|
||||||
|
stateMutex.withLock {
|
||||||
val current = _state.value
|
val current = _state.value
|
||||||
if (current !is CallState.Connecting) {
|
if (current !is CallState.Connecting) {
|
||||||
Log.d("CallManager") { "onPeerConnected: state is ${current::class.simpleName}, not Connecting — ignoring" }
|
Log.d("CallManager") { "onPeerConnected: state is ${current::class.simpleName}, not Connecting — ignoring" }
|
||||||
@@ -472,6 +506,7 @@ class CallManager(
|
|||||||
pendingPeerPubKeys = current.pendingPeerPubKeys,
|
pendingPeerPubKeys = current.pendingPeerPubKeys,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
suspend fun invitePeer(
|
suspend fun invitePeer(
|
||||||
peerPubKey: HexKey,
|
peerPubKey: HexKey,
|
||||||
@@ -509,6 +544,7 @@ class CallManager(
|
|||||||
suspend fun hangup() {
|
suspend fun hangup() {
|
||||||
val peerPubKeys: Set<HexKey>
|
val peerPubKeys: Set<HexKey>
|
||||||
val callId: String
|
val callId: String
|
||||||
|
stateMutex.withLock {
|
||||||
when (val current = _state.value) {
|
when (val current = _state.value) {
|
||||||
is CallState.Offering -> {
|
is CallState.Offering -> {
|
||||||
peerPubKeys = current.peerPubKeys
|
peerPubKeys = current.peerPubKeys
|
||||||
@@ -533,6 +569,7 @@ class CallManager(
|
|||||||
// Transition immediately so the UI stops ringing/ringback before
|
// Transition immediately so the UI stops ringing/ringback before
|
||||||
// the (potentially slow) signing + relay publish completes.
|
// the (potentially slow) signing + relay publish completes.
|
||||||
transitionToEnded(callId, peerPubKeys, EndReason.HANGUP)
|
transitionToEnded(callId, peerPubKeys, EndReason.HANGUP)
|
||||||
|
}
|
||||||
|
|
||||||
val result = factory.createGroupHangup(peerPubKeys, callId, signer = signer)
|
val result = factory.createGroupHangup(peerPubKeys, callId, signer = signer)
|
||||||
result.wraps.forEach { publishEvent(it) }
|
result.wraps.forEach { publishEvent(it) }
|
||||||
@@ -613,12 +650,15 @@ class CallManager(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun onSignalingEvent(event: Event) {
|
suspend fun onSignalingEvent(event: Event) {
|
||||||
if (isEventTooOld(event)) {
|
if (isEventTooOld(event)) {
|
||||||
Log.d("CallManager") { "Discarding old event kind=${event.kind} age=${TimeUtils.now() - event.createdAt}s" }
|
Log.d("CallManager") { "Discarding old event kind=${event.kind} age=${TimeUtils.now() - event.createdAt}s" }
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if (!processedEventIds.add(event.id)) return
|
|
||||||
|
stateMutex.withLock {
|
||||||
|
if (event.id in processedEventIds) return
|
||||||
|
cappedAdd(processedEventIds, event.id, MAX_PROCESSED_EVENT_IDS)
|
||||||
|
|
||||||
// Filter out our own ICE candidates and hangups echoed back from relays.
|
// Filter out our own ICE candidates and hangups echoed back from relays.
|
||||||
// These are never useful: ICE candidates are for the remote peer, and
|
// These are never useful: ICE candidates are for the remote peer, and
|
||||||
@@ -643,7 +683,7 @@ class CallManager(
|
|||||||
else -> null
|
else -> null
|
||||||
}
|
}
|
||||||
if (terminatedCallId != null) {
|
if (terminatedCallId != null) {
|
||||||
completedCallIds.add(terminatedCallId)
|
cappedAdd(completedCallIds, terminatedCallId, MAX_COMPLETED_CALL_IDS)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -656,6 +696,7 @@ class CallManager(
|
|||||||
is CallRenegotiateEvent -> onRenegotiate(event)
|
is CallRenegotiateEvent -> onRenegotiate(event)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fun currentCallId(): String? =
|
fun currentCallId(): String? =
|
||||||
when (val s = _state.value) {
|
when (val s = _state.value) {
|
||||||
@@ -704,7 +745,8 @@ class CallManager(
|
|||||||
peerPubKeys: Set<HexKey>,
|
peerPubKeys: Set<HexKey>,
|
||||||
reason: EndReason,
|
reason: EndReason,
|
||||||
) {
|
) {
|
||||||
completedCallIds.add(callId)
|
cappedAdd(completedCallIds, callId, MAX_COMPLETED_CALL_IDS)
|
||||||
|
discoveredCalleePeers.clear()
|
||||||
_state.value = CallState.Ended(callId, peerPubKeys, reason)
|
_state.value = CallState.Ended(callId, peerPubKeys, reason)
|
||||||
cancelTimeout()
|
cancelTimeout()
|
||||||
resetJob?.cancel()
|
resetJob?.cancel()
|
||||||
|
|||||||
Reference in New Issue
Block a user