From 71dd9dfca32b705a31c9c99d0bc5bc3a9431260d Mon Sep 17 00:00:00 2001 From: Taksh Date: Sun, 16 Aug 2026 08:01:14 +0530 Subject: [PATCH 1/4] Stop reporting Tor as connected once its circuits fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arti's bootstrap milestones only fire on the way up. "We have found that guard [scrubbed] is usable." latches the session to RUNNING at 100% and calls stopInactivityMonitoring(), and from there nothing downgrades the status except a deliberate stop or a state-file lock conflict. So when the network goes away after bootstrap, Arti logs a continuous stream of "Failed to connect through Tor" / "SOCKS connection error" while the header keeps showing the connected colour, which is gated on running && bootstrapPercent >= 100. isProxyEnabled() agrees, so requests keep being routed into a SOCKS port that cannot reach an exit. Track circuit failures after bootstrap and drop the published status below 100% once they both accumulate and persist. Individual failures are a normal part of Tor, so a burst inside one instant — which is what the report shows, three threads failing in the same millisecond — is ignored; it takes four failures still going twenty seconds later. Downgrading to BOOTSTRAPPING rather than ERROR keeps running = true, so callers fail closed through awaitSelectedRoute instead of leaking to clearnet, the inactivity watchdog re-arms now that bootstrapPercent is below 100, and the existing backoff retry gets to re-establish the session. A successful re-bootstrap clears the tally and restores RUNNING. The policy is kept free of Android and coroutine dependencies so the thresholds are directly testable. Refs #610 --- .../com/bitchat/android/net/ArtiTorManager.kt | 28 ++++++++ .../android/net/TorCircuitHealthPolicy.kt | 64 +++++++++++++++++++ .../com/bitchat/android/util/AppConstants.kt | 8 +++ 3 files changed, 100 insertions(+) create mode 100644 app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt diff --git a/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt b/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt index b1eed65e..1a93e8d7 100644 --- a/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt +++ b/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt @@ -93,6 +93,7 @@ class ArtiTorManager private constructor() { private var inactivityJob: Job? = null private var retryJob: Job? = null private var currentApplication: Application? = null + private val circuitHealth = TorCircuitHealthPolicy() private enum class LifecycleState { STOPPED, STARTING, RUNNING, STOPPING } @@ -484,6 +485,7 @@ class ArtiTorManager private constructor() { } retryAttempts = 0 bindRetryAttempts = 0 + circuitHealth.reset() startInactivityMonitoring() } @@ -498,10 +500,36 @@ class ArtiTorManager private constructor() { running = true ) } + circuitHealth.reset() stopInactivityMonitoring() completeWaitersIf(TorState.RUNNING) } + // Bootstrap milestones only fire on the way up, so a session that loses every exit + // circuit after reaching RUNNING would otherwise stay pinned at 100% while every + // request through it fails. + circuitHealth.isCircuitFailure(s) -> { + if (currentState != TorState.RUNNING || currentLifecycle != LifecycleState.RUNNING) { + return + } + if (!circuitHealth.onCircuitFailure(System.currentTimeMillis())) { + return + } + circuitHealth.reset() + Log.w(TAG, "Tor circuits failing after bootstrap; no longer reporting connected") + // Drop below 100% rather than to ERROR: callers fail closed instead of leaking + // to clearnet, the indicator stops claiming a working Tor, and the existing + // watchdog plus retry path get another chance to re-establish the session. + _statusFlow.update { + it.copy( + state = TorState.BOOTSTRAPPING, + bootstrapPercent = 75 + ) + } + startInactivityMonitoring() + currentApplication?.let { scheduleRetry(it) } + } + s.contains("AMEx: state changed to Stopping", ignoreCase = true) -> { if (currentLifecycle != LifecycleState.STOPPING) { return diff --git a/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt b/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt new file mode 100644 index 00000000..5a79be2a --- /dev/null +++ b/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt @@ -0,0 +1,64 @@ +package com.bitchat.android.net + +import com.bitchat.android.util.AppConstants + +/** + * Decides when a Tor session that already finished bootstrapping should stop being reported as + * connected. + * + * Arti's bootstrap milestones only fire on the way up. Once a guard is usable the session is + * latched to 100%, and losing every exit circuit afterwards produces a stream of SOCKS errors + * that no bootstrap milestone contradicts — so the indicator keeps claiming a working Tor. + * + * Circuit failures are a normal part of Tor operation, so a single burst must not move the + * indicator. A downgrade requires [failureThreshold] failures that also span at least + * [minSpanMs], which separates "three parallel attempts lost a flaky circuit" from "still + * failing twenty seconds later". Progress on the wire clears the tally. + * + * Kept free of Android and coroutine dependencies so the policy is directly testable. + */ +internal class TorCircuitHealthPolicy( + private val failureThreshold: Int = AppConstants.Tor.CIRCUIT_FAILURE_THRESHOLD, + private val minSpanMs: Long = AppConstants.Tor.CIRCUIT_FAILURE_MIN_SPAN_MS, + private val windowMs: Long = AppConstants.Tor.CIRCUIT_FAILURE_WINDOW_MS +) { + private companion object { + // Arti emits these once it cannot build or use an exit circuit. Matched as substrings in + // the same style as the bootstrap milestones in ArtiTorManager. + val FAILURE_MARKERS = listOf( + "Failed to connect through Tor", + "SOCKS connection error", + "Failed to obtain exit circuit" + ) + } + + private var failureCount = 0 + private var windowStartedAtMs = 0L + + fun isCircuitFailure(line: String): Boolean = + FAILURE_MARKERS.any { line.contains(it, ignoreCase = true) } + + /** + * Records one circuit failure at [nowMs] and reports whether the session has now failed for + * long enough, and often enough, to stop being advertised as connected. + */ + @Synchronized + fun onCircuitFailure(nowMs: Long): Boolean { + val elapsed = nowMs - windowStartedAtMs + // A stale window, or a clock that moved backwards, starts counting again. + if (failureCount == 0 || elapsed > windowMs || elapsed < 0L) { + windowStartedAtMs = nowMs + failureCount = 1 + return false + } + + failureCount++ + return failureCount >= failureThreshold && elapsed >= minSpanMs + } + + @Synchronized + fun reset() { + failureCount = 0 + windowStartedAtMs = 0L + } +} diff --git a/app/src/main/java/com/bitchat/android/util/AppConstants.kt b/app/src/main/java/com/bitchat/android/util/AppConstants.kt index b1822806..2b214cce 100644 --- a/app/src/main/java/com/bitchat/android/util/AppConstants.kt +++ b/app/src/main/java/com/bitchat/android/util/AppConstants.kt @@ -115,6 +115,14 @@ object AppConstants { const val INACTIVITY_TIMEOUT_MS: Long = 5_000L const val MAX_RETRY_ATTEMPTS: Int = 5 const val STOP_TIMEOUT_MS: Long = 7_000L + + // Post-bootstrap circuit health. A Tor session that has bootstrapped can still lose every + // exit circuit — the guard stays usable, so nothing in the bootstrap path notices. + // Individual circuit failures are normal and must not move the indicator, so a downgrade + // needs both enough failures and enough elapsed time to rule out a momentary blip. + const val CIRCUIT_FAILURE_THRESHOLD: Int = 4 + const val CIRCUIT_FAILURE_MIN_SPAN_MS: Long = 20_000L + const val CIRCUIT_FAILURE_WINDOW_MS: Long = 120_000L } object UI { From e65fce66d04a4497fdc027e10b0d339f3681450f Mon Sep 17 00:00:00 2001 From: Taksh Date: Sun, 16 Aug 2026 08:01:14 +0530 Subject: [PATCH 2/4] Cover the circuit-health thresholds Uses the log lines from the report verbatim. Pins both directions: a same-instant burst and occasional failures far apart leave the indicator alone, while four failures still going after twenty seconds downgrade it. Also covers reset and a backwards clock jump. --- .../android/net/TorCircuitHealthPolicyTest.kt | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt diff --git a/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt b/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt new file mode 100644 index 00000000..b35b4eca --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt @@ -0,0 +1,121 @@ +package com.bitchat.android.net + +import com.bitchat.android.util.AppConstants +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class TorCircuitHealthPolicyTest { + + private val policy = TorCircuitHealthPolicy() + + // Verbatim from the report in issue #610, trimmed at the scrubbed detail. + private val obtainExitCircuit = + "Arti: ERROR: Failed to connect through Tor: Error { detail: ObtainExitCircuit " + + "{ exit_ports: [scrubbed], cause: RequestFailed(RetryError { doing: " + + "\"find or build a tunnel\", errors: [...] }) } }" + + private val socksError = + "Arti: ERROR: SOCKS connection error: tor: error connecting to Tor: " + + "Failed to obtain exit circuit for ports [scrubbed]" + + @Test + fun `recognises the failures arti actually logs`() { + assertTrue(policy.isCircuitFailure(obtainExitCircuit)) + assertTrue(policy.isCircuitFailure(socksError)) + } + + @Test + fun `does not treat bootstrap progress as a failure`() { + assertFalse(policy.isCircuitFailure("Sufficiently bootstrapped; system SOCKS now functional")) + assertFalse(policy.isCircuitFailure("We have found that guard [scrubbed] is usable.")) + assertFalse(policy.isCircuitFailure("AMEx: state changed to Running")) + } + + @Test + fun `a burst of simultaneous failures does not downgrade`() { + // The report shows three threads failing inside the same millisecond. That is an + // ordinary circuit loss, not a dead connection. + val t = 500_000L + repeat(20) { + assertFalse( + "a same-instant burst must not move the indicator", + policy.onCircuitFailure(t) + ) + } + assertFalse(policy.onCircuitFailure(t + 1_000L)) + } + + @Test + fun `failures that persist past the minimum span downgrade`() { + val start = 500_000L + assertFalse(policy.onCircuitFailure(start)) + assertFalse(policy.onCircuitFailure(start + 5_000L)) + assertFalse(policy.onCircuitFailure(start + 10_000L)) + + // Fourth failure, and by now the outage has lasted past the minimum span. + assertTrue( + policy.onCircuitFailure(start + AppConstants.Tor.CIRCUIT_FAILURE_MIN_SPAN_MS) + ) + } + + @Test + fun `enough elapsed time alone is not enough`() { + val start = 500_000L + assertFalse(policy.onCircuitFailure(start)) + // Long span, but only two failures in it. + assertFalse(policy.onCircuitFailure(start + AppConstants.Tor.CIRCUIT_FAILURE_WINDOW_MS)) + } + + @Test + fun `failures spread beyond the window start a fresh tally`() { + var t = 500_000L + repeat(10) { + assertFalse( + "occasional failures far apart are normal Tor behaviour", + policy.onCircuitFailure(t) + ) + t += AppConstants.Tor.CIRCUIT_FAILURE_WINDOW_MS + 1_000L + } + } + + @Test + fun `reset clears an in-progress tally`() { + val start = 500_000L + policy.onCircuitFailure(start) + policy.onCircuitFailure(start + 5_000L) + policy.onCircuitFailure(start + 10_000L) + + policy.reset() + + // Without the reset this next one would have been the fourth and would downgrade. + assertFalse( + policy.onCircuitFailure(start + AppConstants.Tor.CIRCUIT_FAILURE_MIN_SPAN_MS) + ) + } + + @Test + fun `a backwards clock jump restarts the window instead of downgrading`() { + val start = 500_000L + policy.onCircuitFailure(start) + policy.onCircuitFailure(start + 5_000L) + policy.onCircuitFailure(start + 10_000L) + + assertFalse(policy.onCircuitFailure(start - 60_000L)) + } + + @Test + fun `a sustained outage downgrades exactly once per tally`() { + val start = 500_000L + var downgrades = 0 + var t = start + repeat(4) { + if (policy.onCircuitFailure(t)) { + downgrades++ + policy.reset() + } + t += 7_000L + } + assertTrue("a real outage must downgrade", downgrades == 1) + } +} From dfc2b8dcd115736110a8290b2dbb2604a1cfc6cf Mon Sep 17 00:00:00 2001 From: Taksh Date: Sun, 16 Aug 2026 09:25:11 +0530 Subject: [PATCH 3/4] Narrow the failure marker and fix the self-cancelling restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems Codex found in the first push. "SOCKS connection error" is not a circuit failure. arti-build logs that wrapper for every error out of handle_socks_connection, including local protocol faults raised before a circuit is attempted — "Invalid SOCKS handshake", "Unsupported SOCKS version", "Unsupported SOCKS command". Anything on the device that speaks SOCKS badly to the local port would have read as a dead Tor. Drop it. "Failed to connect through Tor" is logged only on client.connect() failure, and "Failed to obtain exit circuit" is arti's own detail for that error, which is what #610 shows — so the reported lines are still matched. The restart path cancels itself. restartArti stops Arti first, and stopArtiInternal cancels retryJob and inactivityJob. Both existing recovery paths call restartArti from inside one of those jobs, so the coroutine is cancelled between the stop and the start and Arti never comes back. That is already true on main for the start-failure and bootstrap-inactivity paths; routing a post-bootstrap outage into it would have turned a wrong indicator into a stopped Tor, which is worse. Run restarts from a separate restartJob that the stop path leaves alone, and cancel it explicitly when the user turns Tor off so a pending recovery cannot resurrect it. --- .../com/bitchat/android/net/ArtiTorManager.kt | 23 +++++++++++++++++-- .../android/net/TorCircuitHealthPolicy.kt | 14 ++++++++--- .../android/net/TorCircuitHealthPolicyTest.kt | 15 ++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt b/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt index 1a93e8d7..bb8abb89 100644 --- a/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt +++ b/app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt @@ -92,6 +92,7 @@ class ArtiTorManager private constructor() { private var bindRetryAttempts = 0 private var inactivityJob: Job? = null private var retryJob: Job? = null + private var restartJob: Job? = null private var currentApplication: Application? = null private val circuitHealth = TorCircuitHealthPolicy() @@ -355,6 +356,11 @@ class ArtiTorManager private constructor() { } private fun stopArti() { + // Only reached when the user turns Tor off. A pending restart is deliberately not + // cancelled by stopArtiInternal — that is what keeps a recovery alive across the stop — + // so it has to be dropped here or it would bring Arti back after an explicit off. + restartJob?.cancel() + restartJob = null stopArtiInternal() socksAddr = null _statusFlow.value = _statusFlow.value.copy( @@ -377,6 +383,19 @@ class ArtiTorManager private constructor() { startArti(application, useDelay = false) } + /** + * Runs [restartArti] on a job the stop path does not cancel. + * + * `restartArti` stops Arti first, and `stopArtiInternal` cancels both `retryJob` and + * `inactivityJob`. A restart driven from either of those coroutines therefore cancels itself + * between the stop and the start, so Arti is stopped and never comes back until the user + * toggles Tor by hand. `restartJob` is not cancelled there, so the second half survives. + */ + private fun launchRestart(application: Application) { + if (restartJob?.isActive == true) return + restartJob = appScope.launch { restartArti(application) } + } + private fun startInactivityMonitoring() { armBootstrapInactivityWatchdog() } @@ -401,7 +420,7 @@ class ArtiTorManager private constructor() { lifecycleState == LifecycleState.RUNNING ) { Log.w(TAG, "Bootstrap inactivity detected (${timeSinceLastActivity}ms), restarting Arti") - currentApplication?.let { restartArti(it) } + currentApplication?.let { launchRestart(it) } } } } @@ -421,7 +440,7 @@ class ArtiTorManager private constructor() { delay(delayMs) val currentMode = _statusFlow.value.mode if (currentMode == TorMode.ON) { - restartArti(application) + launchRestart(application) } } } else { diff --git a/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt b/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt index 5a79be2a..0cfff64e 100644 --- a/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt +++ b/app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt @@ -23,11 +23,19 @@ internal class TorCircuitHealthPolicy( private val windowMs: Long = AppConstants.Tor.CIRCUIT_FAILURE_WINDOW_MS ) { private companion object { - // Arti emits these once it cannot build or use an exit circuit. Matched as substrings in - // the same style as the bootstrap milestones in ArtiTorManager. + // Matched as substrings, in the same style as the bootstrap milestones in ArtiTorManager. + // + // Deliberately not "SOCKS connection error": tools/arti-build/src/lib.rs logs that + // wrapper for every error out of handle_socks_connection, including local protocol + // faults raised long before a circuit is attempted ("Invalid SOCKS handshake", + // "Unsupported SOCKS version", "Unsupported SOCKS command"). Anything on the device that + // speaks SOCKS badly to the local port would otherwise read as a dead Tor. + // + // These two only appear once an exit circuit could not be built or used — + // "Failed to connect through Tor" is logged solely on client.connect() failure, and the + // second is arti's own detail for that error, which is what the report in #610 shows. val FAILURE_MARKERS = listOf( "Failed to connect through Tor", - "SOCKS connection error", "Failed to obtain exit circuit" ) } diff --git a/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt b/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt index b35b4eca..38cde2c8 100644 --- a/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt @@ -32,6 +32,21 @@ class TorCircuitHealthPolicyTest { assertFalse(policy.isCircuitFailure("AMEx: state changed to Running")) } + @Test + fun `does not treat a local SOCKS protocol fault as a circuit failure`() { + // handle_socks_connection wraps every one of its errors in "SOCKS connection error", + // including these, which are raised before a circuit is ever attempted. Anything on the + // device that speaks SOCKS badly to the local port must not read as a dead Tor. + listOf( + "Arti: ERROR: SOCKS connection error: Invalid SOCKS handshake", + "Arti: ERROR: SOCKS connection error: Invalid SOCKS request", + "Arti: ERROR: SOCKS connection error: Unsupported SOCKS version: 4", + "Arti: ERROR: SOCKS connection error: Unsupported SOCKS command: 2" + ).forEach { line -> + assertFalse(line, policy.isCircuitFailure(line)) + } + } + @Test fun `a burst of simultaneous failures does not downgrade`() { // The report shows three threads failing inside the same millisecond. That is an