diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt b/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt index 4d059e0..fbc509e 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/CallManager.kt @@ -87,6 +87,45 @@ class CallManager( /** Gives up an unanswered outgoing call, like iOS's 45 s dial timeout. */ private var dialGuard: Runnable? = null + /** + * Stops an unanswered incoming call ringing forever. + * + * Every other state had a guard and this one did not, so a call nobody + * picked up stayed RINGING until the app was killed. Nothing showed it: the + * notification can be swiped off the top of the screen while the state + * behind it survives. The next caller then got "busy" from a phone that + * looked idle, and its owner had no way to find out why. + * + * The caller gives up after DIAL_TIMEOUT_MS. This end waits a little longer + * than that, so a ring is released by its own timer only when the caller's + * hang-up never arrived. + */ + private var ringGuard: Runnable? = null + + private fun armRingGuard() { + cancelRingGuard() + val guard = + Runnable { + if (state == State.RINGING) { + android.util.Log.i( + "fnmsg-call", + "ring expired with no answer, releasing so the next call is not refused", + ) + // "timeout", not a new word: the far end already renders + // that as "No answer", which is what this is. An unknown + // reason falls through to a flat "Call ended". + hangUp(notifyPeer = true, reason = "timeout") + } + } + ringGuard = guard + main.postDelayed(guard, RING_TIMEOUT_MS) + } + + private fun cancelRingGuard() { + ringGuard?.let { main.removeCallbacks(it) } + ringGuard = null + } + /** * Gives up a call that was answered but whose media never connected. */ @@ -411,6 +450,7 @@ class CallManager( video = incoming.video, ) state = State.RINGING + armRingGuard() } Content.CallSignal.ANSWER -> { @@ -420,6 +460,7 @@ class CallManager( if (incoming.callId != current?.callId) return dialGuard?.let { main.removeCallbacks(it) } dialGuard = null + cancelRingGuard() state = State.CONNECTING armConnectGuard() connection?.setRemoteDescription( @@ -599,6 +640,12 @@ class CallManager( companion object { /** How long an unanswered outgoing call rings before giving up. */ const val DIAL_TIMEOUT_MS = 45_000L + + /** + * A little past the caller's own patience, so this end releases a ring + * only when their hang-up never reached us. + */ + const val RING_TIMEOUT_MS = 60_000L const val CONNECT_TIMEOUT_MS = 30_000L /** diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt index a5af607..93818c1 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt @@ -168,6 +168,25 @@ class MainActivity : androidx.fragment.app.FragmentActivity() { @Composable private fun Root(model: MessengerViewModel = viewModel()) { val phase by model.phase.collectAsStateWithLifecycle() + + // The other half of the handover above. The service is started when this + // screen stops; this stops the loop that would otherwise keep reading the + // same inbox beside it. An envelope is consumed once, so two pollers means + // one of them loses, and the one that loses is sometimes the only one that + // could have rung the phone. + val owner = androidx.lifecycle.compose.LocalLifecycleOwner.current + DisposableEffect(owner) { + val observer = + androidx.lifecycle.LifecycleEventObserver { _, event -> + when (event) { + androidx.lifecycle.Lifecycle.Event.ON_STOP -> model.pausePolling() + androidx.lifecycle.Lifecycle.Event.ON_START -> model.resumePolling() + else -> Unit + } + } + owner.lifecycle.addObserver(observer) + onDispose { owner.lifecycle.removeObserver(observer) } + } Scaffold { padding -> val frame = Modifier.fillMaxSize().padding(padding).padding(20.dp) when (val current = phase) { diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt index eb1d8d3..d0c8f51 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MessengerViewModel.kt @@ -3415,6 +3415,31 @@ class MessengerViewModel( /** * Poll for mail while signed in. */ + /** + * Stop reading the inbox while the app is off screen. + * + * `MainActivity` starts the background service on the way out and stops it + * on the way back, under a comment reading "exactly one poller at a time". + * Only half of that was ever true: the service started, and this loop kept + * running beside it. Two sessions then polled the same inbox, and an + * envelope is consumed once, so whichever asked first won it. + * + * When this loop won a call offer the phone rang inside a UI nobody was + * looking at, and the service, which is the only half that can post a + * notification, never saw the call at all. Observed on a device: an offer + * arrived, `CallManager` moved to RINGING, and the only notification posted + * was the service's own "running" notice. + */ + fun pausePolling() { + polling?.cancel() + polling = null + } + + /** Read the inbox again, once the app is back on screen. */ + fun resumePolling() { + if (polling == null && session != null) startPolling() + } + private fun startPolling() { polling?.cancel() // So an incoming call rings, see installCallSignalHandler. diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/OnePollerAtATimeTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/OnePollerAtATimeTest.kt new file mode 100644 index 0000000..31c17a6 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/OnePollerAtATimeTest.kt @@ -0,0 +1,76 @@ +package io.flamenet.messenger.app + +import java.io.File +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Exactly one thing reads the inbox at a time. + * + * `MainActivity` starts the background service when the screen stops and stops + * it when the screen starts, under a comment saying "exactly one poller at a + * time". Only half of that was ever true: the service started, and the view + * model's loop kept running beside it. + * + * An envelope is consumed once. With two sessions polling the same inbox + * whichever asked first won, and when the view model won a call offer the phone + * rang inside a UI nobody was looking at. The service is the only half that can + * post a notification, and it never saw the call. Observed on a device: the + * offer arrived, `CallManager` went to RINGING, and the only notification + * posted was the service's own "running" notice. + */ +class OnePollerAtATimeTest { + private fun source(path: String): String { + val file = File("src/main/kotlin/io/flamenet/messenger/app/$path") + assertTrue("missing $path", file.exists()) + return file.readText() + } + + /** The service half: started on the way out, stopped on the way back. */ + @Test + fun theServiceHalfIsWired() { + val main = source("MainActivity.kt") + assertTrue(main.contains("MessengerService.stop(this)")) + assertTrue(main.contains("MessengerService.start(this)")) + } + + /** The half that was missing. */ + @Test + fun theViewModelHalfIsWired() { + val vm = source("MessengerViewModel.kt") + assertTrue("nothing could stop the loop", vm.contains("fun pausePolling()")) + assertTrue("and nothing could start it again", vm.contains("fun resumePolling()")) + assertTrue( + "pausing must actually cancel the job, not just drop the reference", + vm.contains("polling?.cancel()"), + ) + } + + /** Wired to the lifecycle, not merely defined. */ + @Test + fun theViewModelHalfIsDrivenByTheLifecycle() { + val main = source("MainActivity.kt") + assertTrue( + "ON_STOP must stop the loop", + main.contains("Lifecycle.Event.ON_STOP -> model.pausePolling()"), + ) + assertTrue( + "ON_START must start it again, or the app comes back deaf", + main.contains("Lifecycle.Event.ON_START -> model.resumePolling()"), + ) + } + + /** + * Resuming twice must not leave two loops running, which would recreate the + * race inside a single process. + */ + @Test + fun resumingTwiceDoesNotStartTwoLoops() { + val vm = source("MessengerViewModel.kt") + val resume = vm.substringAfter("fun resumePolling()").substringBefore("}") + assertTrue( + "resume must check the loop is not already running", + resume.contains("polling == null"), + ) + } +}