Merge 690b60e20ffcb1cc4ccdd31de298860ca4526d66 into c127eb83ab94c069c32d37530d2faecd381cd2a8

app/src/main/java/com/bitchat/android/net/ArtiTorManager.kt

@@ -92,7 +92,9 @@ 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()
 
     private enum class LifecycleState { STOPPED, STARTING, RUNNING, STOPPING }
 
@@ -354,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(
@@ -376,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()
     }
@@ -400,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) }
             }
         }
     }
@@ -420,7 +440,7 @@ class ArtiTorManager private constructor() {
                 delay(delayMs)
                 val currentMode = _statusFlow.value.mode
                 if (currentMode == TorMode.ON) {
-                    restartArti(application)
+                    launchRestart(application)
                 }
             }
         } else {
@@ -484,6 +504,7 @@ class ArtiTorManager private constructor() {
                 }
                 retryAttempts = 0
                 bindRetryAttempts = 0
+                circuitHealth.reset()
                 startInactivityMonitoring()
             }
 
@@ -498,10 +519,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

app/src/main/java/com/bitchat/android/net/TorCircuitHealthPolicy.kt

@@ -0,0 +1,72 @@
+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 {
+        // 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",
+            "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
+    }
+}

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 {

app/src/test/kotlin/com/bitchat/android/net/TorCircuitHealthPolicyTest.kt

@@ -0,0 +1,136 @@
+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 `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
+        // 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)
+    }
+}