Calling killed the app, twice over

🔴 **Three separate defects, and calling could not work past any of them.**
Found by tapping the Call button, which is not covered by any test.

**1. The button did nothing.** `requestCall` sets `callPrivacyPrompt`, and the
only thing that renders it is `CallBanner` — which was composed inside
`SignedIn`, the HOME screen. The chat screen REPLACES that. So the only Call
button in the app set a flag nothing was on screen to show, and since the
prompt gates placing the call, calling could not be started at all. The button
looked simply dead.

That also means an incoming call arriving while somebody was reading a thread
displayed nothing — directly against the comment above it, which says an
incoming call must be visible whatever screen somebody is on. CallBanner is
now hoisted above the screen switch.

**2. Placing a call SIGABRTed the process.** The manifest never declared
ACCESS_NETWORK_STATE. WebRTC's NetworkMonitor calls
ConnectivityManager.getNetworkState() on its own network thread, that throws
without the permission, and the JNI layer immediately asserts
RTC_CHECK(!env->ExceptionCheck()) — turning a Java exception into abort().
No AndroidRuntime trace, nothing catchable, the process just vanishes:

    Check failed: !env->ExceptionCheck()
    Fatal signal 6 (SIGABRT) in tid network_thread

**3. Hanging up SIGABRTed it again, re-entrantly.** `onIceConnectionChange`
treated CLOSED as a reason to hang up — but CLOSED is exactly what our own
`close()` produces. So Hang up called hangUp, which closed the connection,
which fired CLOSED on the signaling thread, which re-entered hangUp, which did
a BLOCKING NETWORK SEND and disposed the connection it was running inside.

hangUp is now re-entrancy-guarded, CLOSED is a no-op (we caused it), FAILED
still hangs up, `dispose()` is guarded like the `close()` beside it already
was, and the courtesy "call ended" signal is best-effort rather than something
that can take the teardown down with it. The observer body cannot throw at all
now, because WebRTC treats any escaping exception as fatal.

Verified on the emulator: dialling brings up Mute/Hang up and the app survives
both dialling and hanging up.

⚠️ Still unproven: whether AUDIO actually flows. That needs two live endpoints
and a TURN server, and none is deployed. What is fixed is that calling is
reachable and no longer kills the app.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-08-26 09:05:31 -04:00
co-authored by Claude Opus 5
parent fb099442a0
commit f90c6477bb
3 changed files with 62 additions and 8 deletions
+11
View File
@@ -3,6 +3,17 @@
<uses-permission android:name="android.permission.INTERNET" /> <uses-permission android:name="android.permission.INTERNET" />
<!--
🔴 **WebRTC aborts the PROCESS without this.** Its NetworkMonitor calls
ConnectivityManager.getNetworkState() on its own network thread, and the
JNI layer asserts RTC_CHECK(!env->ExceptionCheck()) straight afterwards.
Missing this permission makes that call throw, and the check turns a Java
exception into abort() — SIGABRT inside libjingle_peerconnection_so, no
AndroidRuntime trace, no catchable failure. Placing a call simply killed
the app.
-->
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<!-- <!--
Voice notes. Requested at runtime as well: on this minSdk a dangerous Voice notes. Requested at runtime as well: on this minSdk a dangerous
permission declared here is not granted here, and a recorder started permission declared here is not granted here, and a recorder started
@@ -118,14 +118,26 @@ class CallManager(
override fun onIceCandidate(candidate: IceCandidate?) = Unit override fun onIceCandidate(candidate: IceCandidate?) = Unit
override fun onIceCandidatesRemoved(candidates: Array<out IceCandidate>?) = Unit override fun onIceCandidatesRemoved(candidates: Array<out IceCandidate>?) = Unit
override fun onSignalingChange(state: PeerConnection.SignalingState?) = Unit override fun onSignalingChange(state: PeerConnection.SignalingState?) = Unit
// 🔴 **Nothing may throw out of here, and nothing may call back
// into WebRTC from here.** This runs on WebRTC's signaling thread,
// and its JNI asserts RTC_CHECK(!env->ExceptionCheck()) on the way
// out — so ANY escaping Kotlin exception aborts the process:
// SIGABRT, no AndroidRuntime trace, nothing to catch.
override fun onIceConnectionChange(newState: PeerConnection.IceConnectionState?) { override fun onIceConnectionChange(newState: PeerConnection.IceConnectionState?) {
runCatching {
when (newState) { when (newState) {
PeerConnection.IceConnectionState.CONNECTED, PeerConnection.IceConnectionState.CONNECTED,
PeerConnection.IceConnectionState.COMPLETED -> state = State.ACTIVE PeerConnection.IceConnectionState.COMPLETED -> state = State.ACTIVE
PeerConnection.IceConnectionState.FAILED, // ⚠️ CLOSED is what our OWN close() produces. Treating it as
PeerConnection.IceConnectionState.CLOSED -> hangUp(notifyPeer = true) // a reason to hang up re-entered hangUp from inside this
// callback, which then did a blocking network send AND
// disposed the connection we are currently inside — and
// killed the app every single time somebody pressed Hang up.
PeerConnection.IceConnectionState.CLOSED -> Unit
PeerConnection.IceConnectionState.FAILED -> hangUp(notifyPeer = true)
else -> Unit else -> Unit
} }
}
} }
override fun onIceConnectionReceivingChange(receiving: Boolean) = Unit override fun onIceConnectionReceivingChange(receiving: Boolean) = Unit
override fun onIceGatheringChange(newState: PeerConnection.IceGatheringState?) = Unit override fun onIceGatheringChange(newState: PeerConnection.IceGatheringState?) = Unit
@@ -235,12 +247,34 @@ class CallManager(
* Closing one without disposing the other leaves the mic live with no call * Closing one without disposing the other leaves the mic live with no call
* attached — the worst possible failure for this particular feature. * attached — the worst possible failure for this particular feature.
*/ */
/**
* ⚠️ **Re-entrancy-safe on purpose.** Tearing a connection down makes
* WebRTC fire state callbacks, and a callback that calls back in here
* would dispose the very object it is running inside.
*/
private var tearingDown = false
fun hangUp(notifyPeer: Boolean) { fun hangUp(notifyPeer: Boolean) {
if (tearingDown) return
tearingDown = true
try {
hangUpInner(notifyPeer)
} finally {
tearingDown = false
}
}
private fun hangUpInner(notifyPeer: Boolean) {
val active = current val active = current
if (notifyPeer && active != null) { if (notifyPeer && active != null) {
signal(active.peerId, Content.CallSignal(Content.CallSignal.END, active.callId)) // Best effort. Telling the other end is worth doing and is not
// worth failing the teardown over — the relay can be rate limiting
// us, or simply unreachable.
runCatching {
signal(active.peerId, Content.CallSignal(Content.CallSignal.END, active.callId))
}
} }
connection?.let { runCatching { it.close() }; it.dispose() } connection?.let { runCatching { it.close() }; runCatching { it.dispose() } }
connection = null connection = null
audioTrack?.let { runCatching { it.dispose() } } audioTrack?.let { runCatching { it.dispose() } }
audioTrack = null audioTrack = null
@@ -78,6 +78,19 @@ private fun Root(model: MessengerViewModel = viewModel()) {
// the lists below manage their own scrolling. // the lists below manage their own scrolling.
is MessengerViewModel.Phase.SignedIn -> is MessengerViewModel.Phase.SignedIn ->
Column(modifier = frame, verticalArrangement = Arrangement.spacedBy(12.dp)) { Column(modifier = frame, verticalArrangement = Arrangement.spacedBy(12.dp)) {
// 🔴 **Above the screen switch, not inside one branch.** An
// incoming call has to be visible whatever screen somebody
// is on, and this used to live inside SignedIn — which the
// chat screen REPLACES. So a call arriving while you were
// reading a thread showed nothing at all.
//
// Worse, the same banner renders the first-call privacy
// prompt, and the only Call button is IN a chat. Tapping it
// set the prompt state, nothing was composed to show it,
// and the prompt gates placing the call — so calling could
// not be started at all. The button looked simply dead.
CallBanner(model, current)
val chat = current.openChat val chat = current.openChat
if (chat != null) { if (chat != null) {
ChatScreen(model, chat, current.error, current.buddies) ChatScreen(model, chat, current.error, current.buddies)
@@ -314,10 +327,6 @@ private fun ColumnScope.SignedIn(
color = Palette.SecondaryText, color = Palette.SecondaryText,
modifier = Modifier.testTag("device-id")) modifier = Modifier.testTag("device-id"))
// Above everything: an incoming call is the one thing that must be visible
// whatever screen somebody is on.
CallBanner(model, phase)
// ⚠️ **Screen level, not inside Contacts.** There is ONE error slot shared // ⚠️ **Screen level, not inside Contacts.** There is ONE error slot shared
// by everything on this screen, and it used to be rendered inside // by everything on this screen, and it used to be rendered inside
// BuddyList — so a story that failed to post, or a group that could not be // BuddyList — so a story that failed to post, or a group that could not be