From 048aa821b655fe3c2a407a4c137e27f33f988bb3 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Thu, 18 Jun 2026 18:08:17 -0400 Subject: [PATCH] fix(tor): warm-reset on dead-circuit self-heal instead of wiping state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Tor is Active but every Tor-routed relay fails (the ExitTimeout / RESOLVEFAILED barrage), onTorCircuitsDead() now does a warm reset() — drop the in-process client to rebuild the circuit pool with a fresh exit draw, while keeping guards and the consensus cache — instead of resetWithCleanState(). The failure is exit-side, not entry-side: circuits build fine, but the exits can't reach the relays. Wiping arti/state/ + cache can't improve exit selection (exits aren't persisted) and only forces a ~60s cold bootstrap — exactly the blackout that strands users on the connection-failure dialog. A warm restart reconnects in ~5s. The poisoned-guards safety net is preserved: the next start() re-runs noUsableGuards(), so genuinely unusable guards are still wiped. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../amethyst/ui/tor/TorManager.kt | 25 ++++++++++++------- .../amethyst/ui/tor/TorManagerTest.kt | 20 +++++++++++---- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt index 0f7f31c84b..f55ccf315c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt @@ -313,13 +313,20 @@ class TorManager( * the per-relay success/failure outcome and the Tor-routing of each url) detects the * all-failing condition and pokes us here — analogous to [onNetworkChange]. * - * Recovery mirrors the post-Active stuck-Connecting path: drop the client and wipe - * `arti/state/` (we *did* bootstrap, so a fully-dead exit set behind a healthy-looking - * guards.json points at a bad persisted guard/circuit sample worth rebuilding), then bump - * [resetEpoch] so the status combine re-enters the INTERNAL branch and runs a full - * re-init. Shares [lastSelfHealAtMs]/[SELF_HEAL_COOLDOWN_MS] with the Connecting watchdog - * so the two can't thrash — at most one self-heal per cooldown window. If circuits are - * still dead after the reset, the cooldown suppresses further resets and the 60s + * The failure is exit-side, not entry-side: the dead circuits' *exits* can't reach the + * relays (`ExitTimeout` / `RESOLVEFAILED`), while the guards (entry) and the cached + * consensus are fine. So recovery is a **warm** [TorBackend.reset]: drop the in-process + * client so the next start rebuilds the circuit pool from scratch — a fresh exit draw — + * but keep `arti/state/` (the guards were never the problem) and the consensus cache, so + * the re-bootstrap is a ~5s warm restart, not a ~60s cold consensus re-download. We + * deliberately do NOT [TorBackend.resetWithCleanState] here: exits aren't persisted, so + * wiping guards + cache can't improve exit selection, and the 60s cold bootstrap it forces + * is exactly the blackout that strands users on the [connectionFailure] dialog. Then bump + * [resetEpoch] so the status combine re-enters the INTERNAL branch and re-inits. + * + * Shares [lastSelfHealAtMs]/[SELF_HEAL_COOLDOWN_MS] with the Connecting watchdog so the + * two can't thrash — at most one self-heal per cooldown window. If the fresh circuits are + * still dead after the rotation, the cooldown suppresses further resets and the 60s * [connectionFailure] dialog still offers the user the bypass. */ fun onTorCircuitsDead() { @@ -328,9 +335,9 @@ class TorManager( val now = nowMs() if (now - lastSelfHealAtMs < SELF_HEAL_COOLDOWN_MS) return lastSelfHealAtMs = now - Log.w("TorManager") { "Tor Active but all circuits failing — self-healing (drop client + wipe state)" } + Log.w("TorManager") { "Tor Active but all circuits failing — self-healing (drop client to rotate exits, keep state)" } scope.launch(ioDispatcher) { - service.resetWithCleanState() + service.reset() resetEpoch.update { it + 1 } } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/tor/TorManagerTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/tor/TorManagerTest.kt index c6ae747701..94f148500d 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/tor/TorManagerTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/tor/TorManagerTest.kt @@ -171,18 +171,22 @@ class TorManagerTest { // ------------------------------------------------------------------ @Test - fun `onTorCircuitsDead wipes state and re-inits when Active`() = + fun `onTorCircuitsDead rotates exits with a warm reset when Active`() = runTest(UnconfinedTestDispatcher()) { val backend = FakeTorBackend() val manager = buildManager(backend = backend, clock = { 1_000_000_000_000L }) advanceUntilIdle() backend.setActive(17392) advanceUntilIdle() + val resetCountBefore = backend.resetCount manager.onTorCircuitsDead() advanceUntilIdle() - assertEquals("Active-but-failing recovery wipes state", 1, backend.resetWithCleanStateCount) + // Exit failures are exit-side: warm reset (keep guards + cache) to rotate exits, + // never a state wipe (which only forces a 60s cold bootstrap). + assertEquals("Active-but-failing recovery rotates exits warm", resetCountBefore + 1, backend.resetCount) + assertEquals("must not wipe state for an exit-side failure", 0, backend.resetWithCleanStateCount) // resetEpoch bump re-enters INTERNAL → start() runs again. assertTrue("re-init should call start() again", backend.startCount >= 2) } @@ -195,10 +199,12 @@ class TorManagerTest { advanceUntilIdle() // Still Connecting (never reached Active). assertEquals(TorServiceStatus.Connecting, manager.status.value) + val resetCountBefore = backend.resetCount manager.onTorCircuitsDead() advanceUntilIdle() + assertEquals("no rotation while not Active", resetCountBefore, backend.resetCount) assertEquals(0, backend.resetWithCleanStateCount) } @@ -212,10 +218,12 @@ class TorManagerTest { advanceUntilIdle() manager.sessionBypass.value = true advanceUntilIdle() + val resetCountBefore = backend.resetCount manager.onTorCircuitsDead() advanceUntilIdle() + assertEquals("no rotation while bypassing", resetCountBefore, backend.resetCount) assertEquals(0, backend.resetWithCleanStateCount) } @@ -228,10 +236,11 @@ class TorManagerTest { advanceUntilIdle() backend.setActive(17392) advanceUntilIdle() + val resetCountBefore = backend.resetCount manager.onTorCircuitsDead() advanceUntilIdle() - assertEquals(1, backend.resetWithCleanStateCount) + assertEquals(resetCountBefore + 1, backend.resetCount) // Backend is Active again (re-init bootstrapped). A second call inside the cooldown // window must be suppressed. @@ -239,7 +248,7 @@ class TorManagerTest { advanceUntilIdle() manager.onTorCircuitsDead() advanceUntilIdle() - assertEquals("cooldown should suppress the second self-heal", 1, backend.resetWithCleanStateCount) + assertEquals("cooldown should suppress the second self-heal", resetCountBefore + 1, backend.resetCount) // Past the cooldown it can fire again. clockNow += TorManager.SELF_HEAL_COOLDOWN_MS + 1_000L @@ -247,7 +256,8 @@ class TorManagerTest { advanceUntilIdle() manager.onTorCircuitsDead() advanceUntilIdle() - assertEquals("after cooldown elapses, self-heal fires again", 2, backend.resetWithCleanStateCount) + assertEquals("after cooldown elapses, self-heal fires again", resetCountBefore + 2, backend.resetCount) + assertEquals("exit-side rotations never wipe state", 0, backend.resetWithCleanStateCount) } // ------------------------------------------------------------------