diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/tor/TorBootstrapInstrumentedTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/tor/TorBootstrapInstrumentedTest.kt index 9ae1a3b302..d1f110264e 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/tor/TorBootstrapInstrumentedTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/tor/TorBootstrapInstrumentedTest.kt @@ -25,7 +25,10 @@ import androidx.test.filters.LargeTest import androidx.test.platform.app.InstrumentationRegistry import com.vitorpamplona.amethyst.ui.tor.TorService import com.vitorpamplona.amethyst.ui.tor.TorServiceStatus +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout @@ -58,7 +61,7 @@ import kotlin.system.measureTimeMillis * 3. `./gradlew :amethyst:connectedPlayDebugAndroidTest -P android.testInstrumentationRunnerArguments.class=com.vitorpamplona.amethyst.tor.TorBootstrapInstrumentedTest` * * **What it covers that [TorManagerTest] does not:** - * - Real `ArtiNative.initialize` → `create_bootstrapped` → SOCKS listener bind. + * - Real `ArtiNative.initialize` → `create_unbootstrapped_async` → SOCKS listener bind. * - Real rustls `CryptoProvider` install (regression check after the arti-v2.3.0 bump). * - Real `destroy()` releasing the state file lock so a second `initialize()` succeeds. * - OkHttp routing traffic through the SOCKS port and Arti exiting through the @@ -73,7 +76,14 @@ import kotlin.system.measureTimeMillis @Ignore("Tier-3 integration test — requires on-device network access to Tor. See class kdoc to enable.") class TorBootstrapInstrumentedTest { private val context = InstrumentationRegistry.getInstrumentation().targetContext - private val torService = TorService(context) + + /** + * [TorService] promotes Bootstrapping -> Active from a coroutine on this scope, so the test + * must own one and cancel it — without a live scope `status` would never reach Active and every + * assertion below would hang until its timeout. + */ + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + private val torService = TorService(context, scope) @After fun tearDown() = @@ -81,11 +91,12 @@ class TorBootstrapInstrumentedTest { // Drop the native client so this test's state file lock doesn't bleed into // the next instrumented run on the same device. torService.reset() + scope.cancel() } /** * Cold-start bootstrap. The whole point of the custom Arti build is that this - * works at all — if create_bootstrapped panics (e.g., because we forgot to install + * works at all — if client creation panics (e.g., because we forgot to install * a rustls CryptoProvider after an arti bump) the test catches it. */ @Test diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt index 3f55fd2cdd..b327657ac2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt @@ -130,7 +130,6 @@ import com.vitorpamplona.amethyst.ui.screen.AccountState import com.vitorpamplona.amethyst.ui.screen.UiSettingsState import com.vitorpamplona.amethyst.ui.tor.TorManager import com.vitorpamplona.amethyst.ui.tor.TorService -import com.vitorpamplona.amethyst.ui.tor.TorServiceStatus import com.vitorpamplona.quartz.nip01Core.core.Address import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient @@ -282,7 +281,7 @@ class AppModules( UiSettingsState(uiPrefs.value, connManager.isMobileOrFalse, applicationIOScope) } - private val torService = TorService(appContext) + private val torService = TorService(appContext, applicationIOScope) val torManager = TorManager(torPrefs, torService, applicationIOScope) // Network identity change (wifi↔cellular, regained from offline, captive portal @@ -414,7 +413,11 @@ class AppModules( init { applicationIOScope.launch { torService.status - .map { it is TorServiceStatus.Active } + // Battery ledger: Tor is doing work from the moment the client exists — the + // directory download is the most expensive part of a launch — so this tracks + // "running", not "bootstrapped". Keying it on Active alone would silently omit the + // 12-34s download from every cold start. + .map { it.socksPort != null } .distinctUntilChanged() .collect { torSession.setActive(it) } } @@ -647,7 +650,7 @@ class AppModules( // proxy during bootstrap. RelayProxyClientConnector reconnects them (with // ignoreRetryDelays=true) the instant Tor flips to Active. canDial = { url -> - !torEvaluatorFlow.shouldUseTorForRelay(url) || torManager.isSocksReady() + !torEvaluatorFlow.shouldUseTorForRelay(url) || torManager.isTorReady() }, ) @@ -716,7 +719,7 @@ class AppModules( TorCircuitHealthTracker( client = client, isTorRouted = { torEvaluatorFlow.shouldUseTorForRelay(it) }, - isTorActive = { torManager.isSocksReady() }, + isTorActive = { torManager.isTorReady() }, isConnectivityActive = { connManager.status.value is ConnectivityStatus.Active }, onCircuitsDead = { torManager.onTorCircuitsDead() }, ).also { it.register() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt index b7283b37d2..d0630e2cab 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/RelayProxyClientConnector.kt @@ -152,7 +152,7 @@ class RelayProxyClientConnector( onTrigger(UsageKeys.TRIGGER_OFF) client.disconnect() } - if (infra.torStatus is TorServiceStatus.Active) { + if (infra.torStatus.isFullyBootstrapped) { Log.d("ManageRelayServices", "Connectivity off, Tor idle") } // disconnect() already cleared every relay's backoff. Forget the network @@ -163,7 +163,7 @@ class RelayProxyClientConnector( infra.connectivity is ConnectivityStatus.Active && !client.isActive() -> { Log.d("ManageRelayServices", "Connectivity On: Resuming Relay Services") - if (infra.torStatus is TorServiceStatus.Active) { + if (infra.torStatus.isFullyBootstrapped) { Log.d("ManageRelayServices", "Connectivity resumed, Tor active") } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt index 571e328c9a..8e042af60d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/publicChannels/relayGroup/RelayGroupChannelListScreen.kt @@ -98,7 +98,6 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayG import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.relayGroup.datasource.RelayGroupsOnRelaySubscription import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.warningColor -import com.vitorpamplona.amethyst.ui.tor.TorServiceStatus import com.vitorpamplona.quartz.buzz.workspace.BUZZ_CHANNEL_TYPE_DM import com.vitorpamplona.quartz.buzz.workspace.BUZZ_CHANNEL_TYPE_FORUM import com.vitorpamplona.quartz.nip01Core.core.HexKey @@ -351,7 +350,7 @@ fun RelayGroupChannelListScreen( // own dialog), so don't let it read as "this relay blocks Tor exits". val torStatus by Amethyst.instance.torManager.status .collectAsStateWithLifecycle() - val torIsUp = torStatus is TorServiceStatus.Active + val torIsUp = torStatus.isFullyBootstrapped // The offer adds the relay to the kind-10089 Trusted list, which only moves it to clearnet while // trusted relays are *off* Tor. Under the Small-Payloads / Full-Privacy presets they are on Tor, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/ArtiNative.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/ArtiNative.kt index 9a1551f77e..fd1ab6b0f7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/ArtiNative.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/ArtiNative.kt @@ -49,6 +49,30 @@ object ArtiNative { */ external fun initialize(dataDir: String): Int + /** + * Whether Tor can carry traffic right now. + * + * [initialize] returns as soon as the client exists (the proxy is routable immediately and each + * stream waits for its own circuit), so this is the separate signal for "circuits can be built + * now". Polled rather than pushed: driving state off Arti's log strings is what caused the + * Connecting->Active race this wrapper already had to fix once. + * + * Reports Arti's *live* readiness rather than the outcome of the initial download, so a + * bootstrap that failed once and then succeeded on a later stream is picked up. + * + * @return 1 when ready for traffic, 0 when not yet, -1 when there is no client. + */ + external fun isBootstrapped(): Int + + /** + * Directory-download progress in permille (0..1000), or -1 when there is no client. + * + * Lets the lifecycle tell a slow download from a stalled one, which a timeout cannot: measured + * cold downloads ran 12.6-34.4s on the same hardware, so any fixed patience is either short + * enough to kill healthy ones or long enough to sit on a dead one. + */ + external fun bootstrapProgressPermille(): Int + /** * Start the SOCKS5 proxy on the given port. * Can be called multiple times — stops any existing listener first. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorBackend.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorBackend.kt index ff75857e79..9987867ace 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorBackend.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorBackend.kt @@ -31,6 +31,27 @@ import kotlinx.coroutines.flow.StateFlow interface TorBackend { val status: StateFlow + /** + * True while a native bootstrap attempt is actually running. + * + * [TorServiceStatus.Connecting] conflates two states that need opposite responses: a bootstrap + * that is working through a cold consensus download (leave it alone) and a lifecycle that has + * stopped trying (reset it). The stuck-Connecting watchdog cannot tell them apart from status + * alone, and a fresh install spends its first minute in the first one — so the watchdog used to + * queue a reset behind the in-flight attempt's lifecycle lock and tear the client down the + * moment it succeeded. This flag is the missing half of the signal. + */ + val bootstrapInFlight: StateFlow + + /** + * Directory-download progress in permille while [TorServiceStatus.Bootstrapping]; -1 when there + * is no client or nothing is being downloaded. + * + * Emits only on change (it is a [StateFlow]), which is exactly the signal the stall detector + * needs: the timestamp of the last distinct value is the last time Tor made forward progress. + */ + val bootstrapProgress: StateFlow + suspend fun start() suspend fun stop() 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 932743bb65..fd6449d309 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 @@ -26,6 +26,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -34,6 +35,7 @@ import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -103,6 +105,36 @@ class TorManager( */ @Volatile private var hasEverBootstrapped: Boolean = false + /** + * Epoch-millis of the first moment Tor was expected to work and didn't, spanning the self-heal + * retries in between. 0 while Tor is working or off. See [connectionFailure]. + */ + @Volatile private var tryingSinceMs: Long = 0L + + /** + * Epoch-millis of the most recent transition INTO a trying state. Distinct from + * [tryingSinceMs], which deliberately spans self-heal retries: this one restarts on every + * attempt, because the patience owed to a directory download is per-attempt. + */ + @Volatile private var lastTryingTransitionMs: Long = 0L + + /** + * Epoch-millis of the last time the directory download moved. Seeded when a download starts so + * a fresh attempt is never mistaken for a stalled one, then stamped by the collector below on + * every distinct progress value. + */ + @Volatile private var lastProgressAtMs: Long = 0L + + /** + * Whether the bypass prompt is currently raised. Survives the transient [TorServiceStatus.Off] + * a self-heal reset passes through, so the dialog stays up instead of blinking. Cleared + * wherever [tryingSinceMs] is. + */ + @Volatile private var failureRaised: Boolean = false + + /** Consecutive gentle (state-preserving) self-heals with no successful bootstrap in between. */ + @Volatile private var consecutiveGentleResets: Int = 0 + init { // Seed hasEverBootstrapped from persisted on-disk evidence before the watchdog can fire // (well under SELF_HEAL_AFTER_MS), so a stuck bootstrap on a previously-working install @@ -129,6 +161,8 @@ class TorManager( .onEach { sessionBypass.value = false lastBypassApprovalMs = 0L + tryingSinceMs = 0L + failureRaised = false torPrefs.saveLastBypassApprovalMs(0L) }.launchIn(scope) } @@ -150,8 +184,19 @@ class TorManager( } when (torType) { TorType.INTERNAL -> { - service.start() - emitAll(service.status) + // Subscribe to the backend's status BEFORE start(), not after. + // + // start() awaits a blocking JNI bootstrap that runs to its own timeout, so + // awaiting it first meant nothing observed Connecting until that whole attempt + // had already finished. Every timer keyed on the Connecting span therefore + // started one full attempt late: on device the stuck-watchdog fired at 105s + // instead of 45s, and the connection-failure dialog measured its 60s from the + // wrong instant. Running start() alongside the emitAll makes the status the + // app reacts to the status the service is actually in. + coroutineScope { + launch { service.start() } + emitAll(service.status) + } } TorType.OFF -> { @@ -181,11 +226,11 @@ class TorManager( val activePortOrNull: StateFlow = status .map { - (it as? TorServiceStatus.Active)?.port + it.socksPort }.stateIn( scope, SharingStarted.WhileSubscribed(2000), - (status.value as? TorServiceStatus.Active)?.port, + status.value.socksPort, ) /** @@ -198,17 +243,45 @@ class TorManager( val connectionFailure: StateFlow = status .transformLatest { s -> - if (s is TorServiceStatus.Connecting) { + if (!s.isTryingToConnect()) { + // Deliberately does NOT clear [tryingSinceMs]: the self-heal watchdog's own + // reset passes through Off on its way back to Bootstrapping, and clearing here + // would let the retry cycle rearm the timer forever. It is cleared where the + // outage genuinely ends — on a bootstrapped Tor, or on a user intent change. + // + // For the same reason this re-emits [failureRaised] rather than a flat false: + // that transient Off would otherwise dismiss the dialog, and the following + // Bootstrapping would re-raise it immediately (its deadline has already + // passed), so a stuck Tor blinked a modal at the user on every watchdog tick. + emit(failureRaised) + return@transformLatest + } + + // Measure from when Tor STOPPED WORKING, not from this status span. + // + // `transformLatest` restarts on every status change, and the self-heal watchdog + // deliberately bounces Off -> Bootstrapping every SELF_HEAL_AFTER_MS (45s) while + // stuck — less than this 60s timeout. Keyed on the span, the timer was reset by its + // own watchdog before it could ever expire, so the user was never offered the + // bypass no matter how long Tor stayed broken. The question being asked is "has Tor + // been down for a minute", which spans those retries. + if (tryingSinceMs == 0L) tryingSinceMs = nowMs() + emit(false) + + val remaining = BOOTSTRAP_TIMEOUT_MS - (nowMs() - tryingSinceMs) + if (remaining > 0) delay(remaining) + + // Never offer to give up on a bootstrap attempt that is still running: a cold + // install legitimately outlasts this timeout, and prompting mid-attempt asks the + // user to abandon something that is working. + service.bootstrapInFlight.first { !it } + + if (rememberedApprovalActive()) { + sessionBypass.value = true emit(false) - delay(BOOTSTRAP_TIMEOUT_MS) - if (rememberedApprovalActive()) { - sessionBypass.value = true - emit(false) - } else { - emit(true) - } } else { - emit(false) + failureRaised = true + emit(true) } }.stateIn( scope, @@ -217,16 +290,26 @@ class TorManager( ) /** - * Fires once after [SELF_HEAL_AFTER_MS] of continuous [TorServiceStatus.Connecting]. - * Drives the watchdog wired up below. `transformLatest` cancels the pending delay - * whenever the status changes, so a brief Connecting blip never fires. + * Fires every [SELF_HEAL_AFTER_MS] for as long as status stays [TorServiceStatus.Connecting]. + * Drives the watchdog wired up below. `transformLatest` cancels the pending delay whenever the + * status changes, so a brief Connecting blip never fires. + * + * It **repeats** rather than firing once per Connecting span, and that is the whole point. The + * retry loop is driven by status transitions, but the failure it has to recover from produces + * no transition: when the native bootstrap hits its own timeout, [TorService.start] gives up + * and deliberately leaves status at Connecting for this watchdog to retry. A one-shot signal + * has already been consumed by then, so nothing ever re-armed and Tor sat at Connecting + * forever — no retry, no dialog change, no recovery short of a network-identity change or a + * process restart. Repeating means every stuck span is re-examined until it stops being stuck. */ @OptIn(ExperimentalCoroutinesApi::class) private val selfHealSignal = status.transformLatest { s -> - if (s is TorServiceStatus.Connecting) { - delay(SELF_HEAL_AFTER_MS) - emit(Unit) + if (s.isTryingToConnect()) { + while (true) { + delay(SELF_HEAL_AFTER_MS) + emit(Unit) + } } } @@ -247,7 +330,21 @@ class TorManager( // state from a different network needs to go. status .onEach { - if (it is TorServiceStatus.Active) hasEverBootstrapped = true + if (it.isFullyBootstrapped) { + hasEverBootstrapped = true + tryingSinceMs = 0L + lastTryingTransitionMs = 0L + failureRaised = false + consecutiveGentleResets = 0 + } else if (it.isTryingToConnect() && lastTryingTransitionMs == 0L) { + lastTryingTransitionMs = nowMs() + // A download that has just begun has not stalled, whatever the last attempt did. + lastProgressAtMs = nowMs() + } else if (it == TorServiceStatus.Off) { + // A reset passes through Off on its way to a fresh attempt; the next + // Bootstrapping earns a full patience window of its own. + lastTryingTransitionMs = 0L + } }.launchIn(scope) // Rotten guard sample while Tor is otherwise UP. The watchdog above only fires on a status @@ -258,6 +355,13 @@ class TorManager( // AllGuardsDown log is the only reliable signal, so route it through the same rate-limited // wipe. Always a clean-state reset: the whole point is that the persisted sample is the // problem. + // Stamps the moment Tor last moved. A StateFlow only emits distinct values, so this fires + // exactly when progress changes — making `lastProgressAtMs` the age of the last real + // advance rather than the age of the attempt. + service.bootstrapProgress + .onEach { lastProgressAtMs = nowMs() } + .launchIn(scope) + service.guardsDownSignal .onEach { val now = nowMs() @@ -270,20 +374,91 @@ class TorManager( selfHealSignal .onEach { + // Re-check: the signal repeats, so by the time it lands the status may have moved + // on. Resetting a client that just reached Active is the opposite of self-healing. + if (!status.value.isTryingToConnect()) return@onEach + + // A native attempt that is still running is not stuck — it is working. (Under + // on-demand bootstrap `initialize` returns in ~130ms, so this only covers client + // creation; the directory download is covered by the patience window below.) + if (service.bootstrapInFlight.value) return@onEach + + val downloading = status.value is TorServiceStatus.Bootstrapping + + // A running directory download is judged on forward progress, not elapsed time. + // + // A timer cannot tell slow from stalled: measured cold downloads ran 12.6-34.4s on + // this same hardware and network, so any fixed patience is either short enough to + // kill healthy ones — and a reset discards the partial consensus, so firing early + // can stop the download EVER completing — or long enough to sit uselessly on a dead + // one. Progress separates them exactly: a download that is still advancing is left + // alone indefinitely, and one that has not moved at all is reset promptly. + if (downloading && nowMs() - lastProgressAtMs < BOOTSTRAP_STALL_MS) return@onEach + val now = nowMs() - if (now - lastSelfHealAtMs < SELF_HEAL_COOLDOWN_MS) return@onEach + if (now - lastSelfHealAtMs < selfHealCooldownMs()) return@onEach lastSelfHealAtMs = now - if (hasEverBootstrapped) { - Log.w("TorManager") { "Tor stuck Connecting >${SELF_HEAL_AFTER_MS}ms — self-healing (drop client + wipe state)" } + + // Never wipe while downloading. `resetWithCleanState` deletes `arti/cache`, which + // is precisely the consensus this state is in the middle of fetching: wiping it + // guarantees the next attempt restarts from zero, and on a slow link that loops + // forever. The clean-state hammer is for a lifecycle that cannot even get a client + // up, where the persisted state is the prime suspect. + // Escalate a fresh install that cannot even get a client up. + // + // The inline `clearAllArtiData()` retry used to cover corrupt on-disk state; it was + // removed because it fired on every failure, including "no network". But with + // `hasEverBootstrapped` false there is no confirmed guard on disk, so the gentle + // branch below would drop the client forever without ever wiping — and + // `noUsableGuards()` only inspects `guards.json`, so a corrupt `cache/` or the rest + // of `state/` is invisible to it. Escalate once the gentle path has demonstrably + // failed several times in a row. + val exhaustedGentleRetries = !downloading && consecutiveGentleResets >= GENTLE_RESETS_BEFORE_WIPE + + if ((hasEverBootstrapped || exhaustedGentleRetries) && !downloading) { + Log.w("TorManager") { "Tor stuck with no client for >${SELF_HEAL_AFTER_MS}ms — self-healing (drop client + wipe state)" } + consecutiveGentleResets = 0 service.resetWithCleanState() } else { - Log.w("TorManager") { "Tor stuck Connecting >${SELF_HEAL_AFTER_MS}ms on first bootstrap — self-healing (drop client only)" } + consecutiveGentleResets++ + val what = + if (downloading) { + "directory download stuck at ${service.bootstrapProgress.value}/1000 for >${BOOTSTRAP_STALL_MS}ms" + } else { + "stuck with no client >${SELF_HEAL_AFTER_MS}ms" + } + Log.w("TorManager") { "Tor $what — self-healing (drop client only, keeping the consensus cache)" } service.reset() } resetEpoch.update { it + 1 } }.launchIn(scope) } + /** + * How long to wait between self-heals. + * + * Once Tor has bootstrapped on this install, a reset is expensive and rarely the answer, so the + * full [SELF_HEAL_COOLDOWN_MS] applies — a permanently broken network must not put us in a + * reset loop. Before the first successful bootstrap the trade is reversed: there is no working + * state to protect, retrying is nearly free (Arti's directory cache persists across attempts, + * so each retry resumes rather than restarts), and the alternative is a brand-new install + * sitting on a dead Tor for five minutes at a time. So a fresh install retries on + * [FIRST_BOOTSTRAP_RETRY_COOLDOWN_MS] instead. + */ + private fun selfHealCooldownMs(): Long = if (hasEverBootstrapped) SELF_HEAL_COOLDOWN_MS else FIRST_BOOTSTRAP_RETRY_COOLDOWN_MS + + /** + * Tor is meant to be working and isn't yet — the span both the stuck watchdog and the + * connection-failure dialog exist to bound. + * + * It is deliberately NOT `is Connecting`. Under on-demand bootstrap the client is created in + * ~130ms, so status leaves Connecting almost immediately and spends the entire 12-34s directory + * download in [TorServiceStatus.Bootstrapping]. Keying on Connecting alone would have made both + * safety nets unreachable: a download that never completes would sit at Bootstrapping forever + * with nothing watching it. + */ + private fun TorServiceStatus.isTryingToConnect() = this != TorServiceStatus.Off && !isFullyBootstrapped + fun rememberedApprovalActive(): Boolean { val ts = lastBypassApprovalMs return ts > 0 && (nowMs() - ts) < APPROVAL_REMEMBER_MS @@ -292,6 +467,8 @@ class TorManager( /** Called when the user picks "Use regular connection". Starts a fresh 1-hour window. */ fun approveBypassForOneHour() { val now = nowMs() + tryingSinceMs = 0L + failureRaised = false lastBypassApprovalMs = now sessionBypass.value = true scope.launch(ioDispatcher) { @@ -311,6 +488,8 @@ class TorManager( fun onNetworkChange() { sessionBypass.value = false lastBypassApprovalMs = 0L + tryingSinceMs = 0L + failureRaised = false // Prevent the stuck-Connecting watchdog from firing a second reset while the // network-change bootstrap is still legitimately in progress (initial bootstrap // on a new network can take ~10–30s, sometimes longer). @@ -349,7 +528,7 @@ class TorManager( */ fun onTorCircuitsDead() { if (sessionBypass.value) return - if (status.value !is TorServiceStatus.Active) return + if (!status.value.isFullyBootstrapped) return val now = nowMs() if (now - lastSelfHealAtMs < SELF_HEAL_COOLDOWN_MS) return lastSelfHealAtMs = now @@ -360,9 +539,28 @@ class TorManager( } } - fun isSocksReady() = status.value is TorServiceStatus.Active + /** + * Whether a Tor-routed dial has somewhere to go. Both callers + * (`AppModules`' relay gate and the media-http `isTorActive` probe) are asking "can I send this + * through Tor", not "is the directory ready" — a dial during the download queues on its own + * circuit, which is strictly better than the alternative of refusing it or sending it in clear. + */ + fun isSocksReady() = status.value.socksPort != null - fun socksPort(): Int = (status.value as? TorServiceStatus.Active)?.port ?: 17392 + /** + * Tor can carry traffic now — the gate for "start using Tor", as opposed to [isSocksReady]'s + * "route through it if you do". + * + * Dialling merely because the port exists costs more than it saves: measured, relays dialled + * during the download simply time out (Tor connect timeout is 30s, inside the 12-34s window) + * and enter exponential backoff, and because the port is identical either side of + * Bootstrapping -> Active the transport never "changes", so `RelayProxyClientConnector` never + * calls `resetBackoff()` to forgive them. Time-to-first-socket was unchanged by dialling early + * (n=3), so the backoff is pure loss. + */ + fun isTorReady() = status.value.isFullyBootstrapped + + fun socksPort(): Int = status.value.socksPort ?: 17392 companion object { const val BOOTSTRAP_TIMEOUT_MS: Long = 60_000L @@ -371,5 +569,22 @@ class TorManager( /** Self-heal kicks in BEFORE the 60s [BOOTSTRAP_TIMEOUT_MS] dialog so most users never see it. */ const val SELF_HEAL_AFTER_MS: Long = 45_000L const val SELF_HEAL_COOLDOWN_MS: Long = 5L * 60L * 1000L + + /** Cooldown before Tor has ever bootstrapped on this install. See [selfHealCooldownMs]. */ + const val FIRST_BOOTSTRAP_RETRY_COOLDOWN_MS: Long = 30_000L + + /** + * How long a directory download may make **no forward progress at all** before it counts as + * stalled. Time spent downloading does not count against it — only time spent not moving. + */ + const val BOOTSTRAP_STALL_MS: Long = 60_000L + + /** + * Gentle self-heals to try before wiping on-disk state on an install that has never + * bootstrapped. Corrupt `cache/`/`state/` is invisible to [ArtiGuardState.hasNoUsableGuards] + * (it only reads `guards.json`), so without this a fresh install with a bad cache would + * drop-and-retry the client forever and never clear the thing actually blocking it. + */ + const val GENTLE_RESETS_BEFORE_WIPE: Int = 3 } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorService.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorService.kt index d270967de4..e8566a2275 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorService.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorService.kt @@ -24,14 +24,18 @@ import android.content.Context import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -47,16 +51,12 @@ private const val GUARDS_DOWN_THRESHOLD = 40 /** Window for [GUARDS_DOWN_THRESHOLD]. Wide enough that ordinary transient churn never trips it. */ private const val GUARDS_DOWN_WINDOW_MS = 60_000L +/** Cheap: an atomic read against a download that takes 12-34s. */ +private const val BOOTSTRAP_POLL_MS = 500L + private const val DEFAULT_SOCKS_PORT = 17392 private const val MAX_PORT_RETRIES = 10 -/** - * Return code from [ArtiNative.initialize] when the native bootstrap exceeds its - * internal timeout. The native side has already torn down the half-built client; - * we treat this differently from a hard failure (see [TorService.start]). - */ -private const val ARTI_ERROR_BOOTSTRAP_TIMEOUT = -4 - /** * Manages the Arti Tor client via custom JNI bindings. * @@ -70,6 +70,7 @@ private const val ARTI_ERROR_BOOTSTRAP_TIMEOUT = -4 */ class TorService( val context: Context, + private val scope: CoroutineScope, ) : TorBackend { private var socksPort = DEFAULT_SOCKS_PORT private val initialized = AtomicBoolean(false) @@ -82,6 +83,16 @@ class TorService( */ @Volatile private var bootstrapStartedAtMs: Long = -1L + /** + * How many native bootstrap attempts this process has made. Logged at INFO on every attempt so + * a boot log answers "did Tor retry, and how often" — the question that separates a genuinely + * slow first bootstrap from a lifecycle that stopped retrying altogether. + */ + @Volatile private var bootstrapAttempts: Int = 0 + + /** Poller promoting Bootstrapping -> Active. Cancelled by every reset so it can't outlive its client. */ + private var bootstrapWatcher: Job? = null + /** * Serializes every native lifecycle transition ([start], [stop], [reset], * [resetWithCleanState]). [ArtiNative] is a process-global singleton over a @@ -98,6 +109,17 @@ class TorService( private val _status = MutableStateFlow(TorServiceStatus.Off) override val status: StateFlow = _status.asStateFlow() + /** + * True for exactly as long as [ArtiNative.initialize] is running. See + * [TorBackend.bootstrapInFlight] — this is what lets the stuck-Connecting watchdog tell a + * bootstrap that is still working from a lifecycle that has given up. + */ + private val _bootstrapInFlight = MutableStateFlow(false) + override val bootstrapInFlight: StateFlow = _bootstrapInFlight.asStateFlow() + + private val _bootstrapProgress = MutableStateFlow(-1) + override val bootstrapProgress: StateFlow = _bootstrapProgress.asStateFlow() + /** * Every status change goes through here so the transition is logged exactly once, at INFO, with * the time since bootstrap started. @@ -159,7 +181,14 @@ class TorService( private fun artiDataDir() = File(context.filesDir, "arti") - /** Diagnostic: total bytes of the consensus/descriptor cache, to correlate with bootstrap time. */ + /** + * Diagnostic: total bytes of the consensus/descriptor cache, to correlate with bootstrap time — + * a cold 0-byte cache costs 12-34s where a warm one costs ~6s, so it is the first thing you + * want beside a slow bootstrap. + * + * Walks the whole cache directory, so it is called exactly once per attempt, from the INFO line + * below. Read it there rather than adding another call. + */ private fun cacheSizeBytes(): Long { val cacheDir = File(artiDataDir(), "cache") if (!cacheDir.exists()) return 0 @@ -235,8 +264,20 @@ class TorService( override suspend fun start() = lifecycleMutex.withLock { if (proxyRunning.get()) { - if (_status.value is TorServiceStatus.Active) return@withLock - setStatus(TorServiceStatus.Connecting) + // The proxy is already bound, so re-assert the state that matches reality rather + // than falling back to Connecting. + // + // Connecting reports no port. Downgrading to it here would strand a perfectly good + // listener: `activePortOrNull` goes null, every Tor-routed dial drops to the Orbot + // default 9050 where nothing listens, and — because this returns without arming + // [watchBootstrap] — nothing would ever promote it back. `TorManager` re-enters + // this branch on any combine re-fire (torType/port/bypass change, resetEpoch bump, + // or the status flow restarting after WhileSubscribed's 30s timeout), so it is very + // much reachable. + if (_status.value !is TorServiceStatus.Active) { + setStatus(TorServiceStatus.Bootstrapping(socksPort)) + watchBootstrap(socksPort) + } return@withLock } @@ -272,7 +313,7 @@ class TorService( // in state/ — not cache/ — and Arti already validates consensus freshness // and refetches whatever has expired. The reset/clean-state paths still call // clearAllArtiData() for genuine corruption recovery. - Log.d("TorService") { "Preserving Arti cache for warm bootstrap (cache size: ${cacheSizeBytes()} bytes)" } + Log.d("TorService") { "Preserving Arti cache for warm bootstrap" } // Self-heal the wedged guard sample (see [noUsableGuards]): if // the persisted sample has no usable guard left, Arti can @@ -288,29 +329,37 @@ class TorService( Log.d("TorService") { "Initializing Arti with data dir: $dataDir" } bootstrapStartedAtMs = System.currentTimeMillis() - var initResult = ArtiNative.initialize(dataDir) - - if (initResult == ARTI_ERROR_BOOTSTRAP_TIMEOUT) { - // The native bootstrap hit its timeout (hostile network) and - // already tore down the half-built client. Don't wipe state or - // retry inline — that would hold lifecycleMutex for another full - // timeout. Drop the init flag and leave status at Connecting so - // TorManager's self-heal watchdog resets and retries on its own - // cadence (and the connection-failure dialog can still surface). - Log.w("TorService") { "Arti bootstrap timed out — leaving Connecting for the self-heal watchdog to retry" } - initialized.set(false) - return@withContext - } + bootstrapAttempts++ + Log.i("TorService") { "Bootstrapping Arti (attempt $bootstrapAttempts, cache ${cacheSizeBytes()} bytes)" } + _bootstrapInFlight.value = true + val initResult = + try { + ArtiNative.initialize(dataDir) + } finally { + _bootstrapInFlight.value = false + } + Log.i("TorService") { "Arti bootstrap attempt $bootstrapAttempts returned $initResult after ${System.currentTimeMillis() - bootstrapStartedAtMs}ms" } if (initResult != 0) { - Log.e("TorService") { "Failed to initialize Arti: error $initResult, clearing data and retrying" } - clearAllArtiData() - initResult = ArtiNative.initialize(dataDir) - } - if (initResult != 0) { - Log.e("TorService") { "Failed to initialize Arti on retry: error $initResult" } + // Every failure mode ends the same way: don't decide recovery here. + // + // A timeout has already torn down its half-built client natively, and + // retrying inline would hold lifecycleMutex for another full timeout. A + // hard failure used to be treated differently — wipe all Arti data, retry + // inline, then fall back to status Off — and both halves of that were + // wrong. The wipe treated every failure as corruption, so a bootstrap that + // failed for the most ordinary reason there is (no network) threw away a + // guard sample that was working fine. And Off is a terminal state for this + // lifecycle: the watchdog and the connection-failure dialog both only arm + // while status is Connecting, so a hard failure left Tor switched off with + // nothing ever retrying it. + // + // Leaving Connecting hands the decision to [TorManager], which already + // owns the escalation: a gentle client-drop before Tor has ever + // bootstrapped here, a clean-state wipe once a confirmed guard on disk + // proves the persisted state used to work and is therefore suspect. + Log.w("TorService") { "Arti client creation failed (error $initResult) — leaving Connecting for the self-heal watchdog to retry" } initialized.set(false) - setStatus(TorServiceStatus.Off) return@withContext } } @@ -330,8 +379,12 @@ class TorService( } if (!started) { - Log.e("TorService") { "Failed to start SOCKS proxy after $MAX_PORT_RETRIES attempts" } - setStatus(TorServiceStatus.Off) + // Same reasoning as the init-failure branch: Off is terminal for this + // lifecycle — neither the watchdog nor the connection-failure dialog arms on + // it — so reporting Off here would leave Tor silently disabled with nothing + // retrying and no way for the user to find out. Stay Connecting and let the + // watchdog retry; a port collision is usually transient. + Log.w("TorService") { "Failed to bind a SOCKS port after $MAX_PORT_RETRIES attempts — leaving Connecting for the self-heal watchdog to retry" } return@withContext } @@ -350,11 +403,54 @@ class TorService( // reset/stop can't clobber it. val startedAt = bootstrapStartedAtMs val elapsed = if (startedAt > 0) System.currentTimeMillis() - startedAt else -1 - setStatus(TorServiceStatus.Active(socksPort)) - Log.d("TorService") { "Arti SOCKS proxy active on port $socksPort (bootstrap took ${elapsed}ms)" } + // Routable, not yet bootstrapped. The proxy is bound so dials belong here rather + // than at the dead 9050 fallback, but circuits cannot be built until the directory + // download lands — which [watchBootstrap] turns into Active. + setStatus(TorServiceStatus.Bootstrapping(socksPort)) + Log.i("TorService") { "Arti SOCKS proxy routable on port $socksPort after ${elapsed}ms (directory still downloading)" } + watchBootstrap(socksPort) } } + /** + * Polls the native directory-download result and promotes [TorServiceStatus.Bootstrapping] to + * [TorServiceStatus.Active] once circuits can actually be built. + * + * Polling rather than reacting to a log line is deliberate: the previous log-callback-driven + * transition raced `startSocksProxy` returning and silently dropped the Active transition, + * stranding Tor at Connecting until the 60s dialog. The poll reads Arti's live readiness, so a + * download that fails and is retried by a later stream still promotes. + */ + private fun watchBootstrap(port: Int) { + bootstrapWatcher?.cancel() + bootstrapWatcher = + scope.launch(Dispatchers.IO) { + while (true) { + // Publish progress on the same tick we check readiness — one extra cheap JNI + // read, and it is what lets TorManager distinguish slow from stalled. + _bootstrapProgress.value = ArtiNative.bootstrapProgressPermille() + when (ArtiNative.isBootstrapped()) { + 1 -> { + // Only promote if this is still the run we started watching for: a + // reset in between will have moved us to Off/Connecting already. + if (_status.value == TorServiceStatus.Bootstrapping(port)) { + setStatus(TorServiceStatus.Active(port)) + } + return@launch + } + -1 -> { + // No native client behind the proxy — a reset is in flight, or init + // failed. Nothing to promote; leave the status where it is so + // TorManager's watchdog treats it as stuck and retries. + Log.w("TorService") { "No Arti client while watching bootstrap — leaving status for the self-heal watchdog" } + return@launch + } + else -> delay(BOOTSTRAP_POLL_MS) + } + } + } + } + /** * Stop the SOCKS proxy and release the port. * The TorClient stays alive — no file lock issues on restart. @@ -363,6 +459,10 @@ class TorService( lifecycleMutex.withLock { if (!proxyRunning.compareAndSet(true, false)) return@withLock + bootstrapWatcher?.cancel() + bootstrapWatcher = null + _bootstrapProgress.value = -1 + withContext(Dispatchers.IO) { ArtiNative.stopSocksProxy() Log.d("TorService") { "SOCKS proxy stopped" } @@ -409,6 +509,9 @@ class TorService( */ private suspend fun resetLocked() = withContext(Dispatchers.IO) { + bootstrapWatcher?.cancel() + bootstrapWatcher = null + _bootstrapProgress.value = -1 if (proxyRunning.compareAndSet(true, false)) { ArtiNative.stopSocksProxy() } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorServiceStatus.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorServiceStatus.kt index 6eec458095..f358672d3f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorServiceStatus.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorServiceStatus.kt @@ -20,12 +20,52 @@ */ package com.vitorpamplona.amethyst.ui.tor +/** + * Two independent facts about Tor that used to coincide and no longer do. + * + * The SOCKS proxy binds within ~130ms of process start, but the directory download it needs before + * it can build a circuit takes 12-34s on a cold install. While `create_bootstrapped` blocked until + * both were true, one "Active" could honestly mean both. Under `BootstrapBehavior::OnDemand` the + * proxy is usable immediately and streams wait for their own circuits, so the two facts diverge by + * that whole window — and callers want different ones. Read [socksPort] to route bytes and + * [isFullyBootstrapped] to tell a user (or a watchdog) whether Tor is actually working; matching on + * the variants directly is how you end up with the wrong one, because both look like "Active". + */ sealed class TorServiceStatus { + /** Proxy bound and the directory is ready: circuits build immediately. */ data class Active( val port: Int, ) : TorServiceStatus() + /** + * Proxy bound and routable, directory still downloading. Dials sent here are not lost — each + * stream waits for its own circuit — but they will not complete until the download lands. + */ + data class Bootstrapping( + val port: Int, + ) : TorServiceStatus() + object Off : TorServiceStatus() + /** No proxy yet: the native client is still being created. Nothing is routable. */ object Connecting : TorServiceStatus() + + /** + * Where to send bytes, or null if there is nowhere to send them. + * + * [Bootstrapping] counts. Routing to it queues the dial behind the directory download, which is + * what we want; treating it as "no proxy" is what made every dial fall back to the Orbot + * default port 9050, where nothing listens, and fail instantly into backoff. + */ + val socksPort: Int? + get() = + when (this) { + is Active -> port + is Bootstrapping -> port + else -> null + } + + /** Tor can build circuits right now. What a user is told, and what the watchdogs judge. */ + val isFullyBootstrapped: Boolean + get() = this is Active } diff --git a/amethyst/src/main/jniLibs/arm64-v8a/libarti_android.so b/amethyst/src/main/jniLibs/arm64-v8a/libarti_android.so index de4fae5b06..8b4a4b828b 100755 Binary files a/amethyst/src/main/jniLibs/arm64-v8a/libarti_android.so and b/amethyst/src/main/jniLibs/arm64-v8a/libarti_android.so differ diff --git a/amethyst/src/main/jniLibs/x86_64/libarti_android.so b/amethyst/src/main/jniLibs/x86_64/libarti_android.so index 129ca7a67e..a3825bccd0 100755 Binary files a/amethyst/src/main/jniLibs/x86_64/libarti_android.so and b/amethyst/src/main/jniLibs/x86_64/libarti_android.so differ 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 f699969167..a6ea38384c 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 @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.ui.tor import com.vitorpamplona.amethyst.commons.tor.TorType import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow @@ -199,8 +200,8 @@ class TorManagerTest { val backend = FakeTorBackend() val manager = buildManager(backend = backend, clock = { 1_000_000_000_000L }) advanceUntilIdle() - // Still Connecting (never reached Active). - assertEquals(TorServiceStatus.Connecting, manager.status.value) + // Started but never reached a working Tor. + assertFalse(manager.status.value.isFullyBootstrapped) val resetCountBefore = backend.resetCount manager.onTorCircuitsDead() @@ -270,11 +271,14 @@ class TorManagerTest { fun `watchdog uses gentle reset before first Active`() = runTest(UnconfinedTestDispatcher()) { val backend = FakeTorBackend() + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true // Big constant clock so (now - lastSelfHealAtMs=0) is well past cooldown. val manager = buildManager(backend = backend, clock = { 1_000_000_000_000L }) advanceUntilIdle() - assertEquals(TorServiceStatus.Connecting, manager.status.value) + // Big constant clock so (now - lastSelfHealAtMs=0) is well past cooldown. + assertFalse(manager.status.value.isFullyBootstrapped) assertEquals(0, backend.resetCount) advanceTimeBy(TorManager.SELF_HEAL_AFTER_MS + 1_000L) @@ -288,11 +292,14 @@ class TorManagerTest { fun `watchdog wipes state on first stuck-Connecting when guards prove prior bootstrap`() = runTest(UnconfinedTestDispatcher()) { val backend = FakeTorBackend().apply { bootstrappedBefore = true } + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true // Never reaches Active in this session, but on-disk state proves a prior bootstrap. val manager = buildManager(backend = backend, clock = { 1_000_000_000_000L }) advanceUntilIdle() - assertEquals(TorServiceStatus.Connecting, manager.status.value) + // Never reaches Active in this session, but on-disk state proves a prior bootstrap. + assertFalse(manager.status.value.isFullyBootstrapped) advanceTimeBy(TorManager.SELF_HEAL_AFTER_MS + 1_000L) runCurrent() @@ -348,6 +355,8 @@ class TorManagerTest { fun `watchdog cooldown blocks a second fire within the window`() = runTest(UnconfinedTestDispatcher()) { val backend = FakeTorBackend() + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true var clockNow = 1_000_000_000_000L val manager = buildManager(backend = backend, clock = { clockNow }) advanceUntilIdle() @@ -368,6 +377,8 @@ class TorManagerTest { fun `watchdog can fire again once the cooldown elapses`() = runTest(UnconfinedTestDispatcher()) { val backend = FakeTorBackend() + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true var clockNow = 1_000_000_000_000L val manager = buildManager(backend = backend, clock = { clockNow }) advanceUntilIdle() @@ -430,7 +441,7 @@ class TorManagerTest { val manager = buildManager(backend = backend) advanceUntilIdle() - assertEquals(TorServiceStatus.Connecting, manager.status.value) + assertEquals(TorServiceStatus.Bootstrapping(17392), manager.status.value) backend.setActive(17392) advanceUntilIdle() @@ -470,6 +481,396 @@ class TorManagerTest { assertTrue(backend.stopCount >= 1) } + // ------------------------------------------------------------------ + // fresh install: the bootstrap-timeout retry loop + // ------------------------------------------------------------------ + + /** + * The regression that stranded brand-new installs on "Connecting" indefinitely. + * + * On a native bootstrap timeout `TorService.start()` deliberately leaves status at Connecting + * and delegates the retry to this watchdog. The watchdog used to fire once per Connecting + * *span* — and a timeout produces no status change, so no new span ever began. Two attempts + * were made and then the app stopped trying, permanently: no retry, no recovery short of a + * network-identity change or a process restart. + */ + @Test + fun `keeps retrying when the bootstrap keeps timing out`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + advanceTimeBy(10L * 60_000L) + runCurrent() + + assertTrue( + "expected repeated bootstrap retries over 10 stuck minutes, got ${backend.startCount}", + backend.startCount >= 4, + ) + sub.cancel() + } + + /** + * A bootstrap that is still running is not stuck. The native call can hold its lifecycle lock + * for its full timeout, so a reset issued while it runs queues behind it and lands the instant + * the attempt finishes — tearing down a client that may have just succeeded. On a fresh install + * with a cold consensus cache that window is the common case, not a corner case. + */ + @Test + fun `watchdog leaves an in-flight bootstrap alone`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + backend.holdBootstrapInFlight = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + // Well past SELF_HEAL_AFTER_MS, but the attempt is still running. + advanceTimeBy(3L * TorManager.SELF_HEAL_AFTER_MS) + runCurrent() + + assertEquals(0, backend.resetCount) + assertEquals(0, backend.resetWithCleanStateCount) + assertEquals(1, backend.startCount) + + // The attempt returns without reaching Active — now it is genuinely stuck. + backend.finishBootstrapAttempt() + advanceTimeBy(TorManager.SELF_HEAL_AFTER_MS + 1_000L) + runCurrent() + + assertTrue("watchdog should fire once the attempt returned", backend.resetCount >= 1) + sub.cancel() + } + + /** A fresh install has no working state to protect, so it must not wait out the 5-min cooldown. */ + @Test + fun `first-bootstrap retries use the short cooldown`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + // No client at all: this is the fast 45s no-client cadence, not a running download. + backend.startFailsToConnect = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + // Two watchdog windows: with the 5-min cooldown only one reset could land. + advanceTimeBy(3L * TorManager.SELF_HEAL_AFTER_MS) + runCurrent() + + assertTrue( + "expected more than one retry inside 3 watchdog windows, got ${backend.resetCount}", + backend.resetCount >= 2, + ) + sub.cancel() + } + + /** Once Tor has worked, resets stay rate-limited — a broken network must not cause a reset loop. */ + @Test + fun `after a successful bootstrap the long cooldown still applies`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + backend.setActive(17392) + advanceUntilIdle() + backend.setConnecting() + advanceUntilIdle() + + val before = backend.resetWithCleanStateCount + advanceTimeBy(3L * TorManager.SELF_HEAL_AFTER_MS) + runCurrent() + + assertEquals( + "post-bootstrap self-heal must stay on the long cooldown", + 1, + backend.resetWithCleanStateCount - before, + ) + sub.cancel() + } + + /** + * `start()` blocks for a whole native bootstrap attempt. Awaiting it before subscribing to the + * backend's status meant nothing observed Connecting until that attempt was already over, so + * every timer keyed on the Connecting span — the stuck watchdog, the connection-failure + * dialog — started one full attempt late. + */ + @Test + fun `status is observable while the first bootstrap is still running`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + backend.startNeverReturns = true + val manager = buildManager(backend = backend) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + assertFalse(manager.status.value.isFullyBootstrapped) + assertNotEquals(TorServiceStatus.Off, manager.status.value) + sub.cancel() + } + + /** + * The bypass prompt asks the user to give up on Tor. Asking that while the first bootstrap + * attempt is still downloading a cold consensus offers to abandon something that is working. + */ + @Test + fun `connection-failure prompt waits for the bootstrap attempt to return`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + backend.holdBootstrapInFlight = true + val manager = buildManager(backend = backend) + val sub = launch { manager.status.collect { } } + val fail = mutableListOf() + val subFail = launch { manager.connectionFailure.collect { fail.add(it) } } + advanceUntilIdle() + + advanceTimeBy(TorManager.BOOTSTRAP_TIMEOUT_MS * 2) + runCurrent() + assertFalse("must not prompt while the attempt is still running", fail.contains(true)) + + backend.finishBootstrapAttempt() + advanceTimeBy(1_000L) + runCurrent() + assertTrue("must prompt once the attempt returned without connecting", fail.contains(true)) + + sub.cancel() + subFail.cancel() + } + + // ------------------------------------------------------------------ + // Bootstrapping: routable, not yet ready + // ------------------------------------------------------------------ + + /** + * The regression this state exists to prevent. On-demand bootstrap leaves Connecting in ~130ms + * and spends the whole 12-34s directory download in Bootstrapping, so a watchdog keyed on + * Connecting would never fire again — a download that never completes would sit there forever + * with nothing watching it. + */ + @Test + fun `watchdog still fires while stuck Bootstrapping`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + assertTrue( + "start() should leave us routable but not bootstrapped", + manager.status.value is TorServiceStatus.Bootstrapping, + ) + + advanceTimeBy(TorManager.BOOTSTRAP_STALL_MS + 2L * TorManager.SELF_HEAL_AFTER_MS) + runCurrent() + + assertTrue("watchdog must arm on Bootstrapping, not just Connecting", backend.resetCount >= 1) + sub.cancel() + } + + /** The bypass prompt must also survive the state it now spends its time in. */ + @Test + fun `connection-failure prompt fires from a stuck Bootstrapping span`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + // Virtual clock, not the default constant one: the timer now measures elapsed + // wall-clock across the watchdog's retries, so a frozen clock makes it un-expirable. + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + val fail = mutableListOf() + val subFail = launch { manager.connectionFailure.collect { fail.add(it) } } + advanceUntilIdle() + + advanceTimeBy(TorManager.BOOTSTRAP_TIMEOUT_MS + 1_000L) + runCurrent() + + assertTrue( + "the prompt must survive the self-heal watchdog restarting the status span at 45s", + fail.contains(true), + ) + sub.cancel() + subFail.cancel() + } + + /** + * The whole point of the split: a dial issued during the download must be routed through the + * proxy, not dropped to the Orbot default port where nothing listens. + */ + @Test + fun `port is routable while still bootstrapping`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + val manager = buildManager(backend = backend) + val sub = launch { manager.status.collect { } } + val port = launch { manager.activePortOrNull.collect { } } + advanceUntilIdle() + + backend.setBootstrapping(17392) + advanceUntilIdle() + + assertEquals(17392, manager.activePortOrNull.value) + assertTrue(manager.isSocksReady()) + sub.cancel() + port.cancel() + } + + /** ...but it must not be reported to the user, or to the exit-rotation path, as working Tor. */ + @Test + fun `bootstrapping does not count as fully bootstrapped`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + val manager = buildManager(backend = backend) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + backend.setBootstrapping(17392) + advanceUntilIdle() + assertFalse(manager.status.value.isFullyBootstrapped) + + // onTorCircuitsDead is about dead exits behind a working Tor; a download in progress is + // not that, and resetting here would restart the very download we are waiting on. + manager.onTorCircuitsDead() + advanceUntilIdle() + assertEquals(0, backend.resetCount) + + backend.setActive(17392) + advanceUntilIdle() + assertTrue(manager.status.value.isFullyBootstrapped) + sub.cancel() + } + + // ------------------------------------------------------------------ + // audit regressions + // ------------------------------------------------------------------ + + /** + * The worst bug the audit found, now guarded by progress rather than a timer. + * + * A cold directory download legitimately takes 12.6-34.4s (measured), and a reset discards the + * partial consensus — so a watchdog firing on the 45s no-client cadence could stop the download + * ever completing. A download that is still advancing must be left alone no matter how long it + * takes. + */ + @Test + fun `a download that keeps making progress is never reset`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + backend.bootstrappedBefore = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + assertTrue(manager.status.value is TorServiceStatus.Bootstrapping) + + // Ten minutes of slow-but-real progress — far past any fixed patience window. + repeat(20) { step -> + advanceTimeBy(30_000L) + backend.advanceBootstrapProgress(step * 50) + runCurrent() + } + + assertEquals("a progressing download must never be reset", 0, backend.resetCount) + assertEquals("and never wiped", 0, backend.resetWithCleanStateCount) + sub.cancel() + } + + /** A download that stops moving is reset promptly — and still never has its cache wiped. */ + @Test + fun `a download that stops progressing is reset but never wiped`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + backend.bootstrappedBefore = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + backend.advanceBootstrapProgress(120) + runCurrent() + + // Nothing moves from here on. + advanceTimeBy(TorManager.BOOTSTRAP_STALL_MS + TorManager.SELF_HEAL_AFTER_MS) + runCurrent() + + assertTrue("a stalled download must be escaped", backend.resetCount >= 1) + assertEquals( + "but the consensus cache is what it is trying to fetch — never wipe it", + 0, + backend.resetWithCleanStateCount, + ) + sub.cancel() + } + + /** + * A fresh install with corrupt on-disk state can never produce a client, and `guards.json` + * (the only thing [ArtiGuardState] inspects) may look fine. Without an escalation the gentle + * branch drop-and-retries forever and never clears what is actually blocking it. + */ + @Test + fun `a fresh install that never gets a client eventually wipes state`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + // Never bootstrapped here, and start() cannot even bind a proxy. + backend.startFailsToConnect = true + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + advanceUntilIdle() + + advanceTimeBy(TorManager.SELF_HEAL_AFTER_MS * 12) + runCurrent() + + assertTrue( + "gentle resets alone never clear corrupt state; expected an escalation", + backend.resetWithCleanStateCount >= 1, + ) + sub.cancel() + } + + /** + * Each self-heal drives Bootstrapping -> Off -> Bootstrapping. If the prompt drops on the + * transient Off and re-raises immediately after, the user gets a modal blinking at them on + * every watchdog tick instead of a stable choice. + */ + @Test + fun `the bypass prompt stays up across a self-heal reset`() = + runTest(UnconfinedTestDispatcher()) { + val backend = FakeTorBackend() + val epoch = 1_700_000_000_000L + val manager = buildManager(backend = backend, clock = { epoch + testScheduler.currentTime }) + val sub = launch { manager.status.collect { } } + val seen = mutableListOf() + val subFail = launch { manager.connectionFailure.collect { seen.add(it) } } + advanceUntilIdle() + + advanceTimeBy(TorManager.BOOTSTRAP_TIMEOUT_MS + 1_000L) + runCurrent() + assertTrue("prompt should be up", manager.connectionFailure.value) + + // Drive several watchdog cycles; the prompt must not drop back to false. + val raisedAt = seen.size + advanceTimeBy(TorManager.BOOTSTRAP_STALL_MS * 3) + runCurrent() + + assertFalse( + "prompt blinked off during a self-heal cycle", + seen.drop(raisedAt).contains(false), + ) + sub.cancel() + subFail.cancel() + } + // ------------------------------------------------------------------ // helpers // ------------------------------------------------------------------ @@ -496,6 +897,28 @@ private class FakeTorBackend : TorBackend { private val _status = MutableStateFlow(TorServiceStatus.Off) override val status: StateFlow = _status.asStateFlow() + private val _bootstrapInFlight = MutableStateFlow(false) + override val bootstrapInFlight: StateFlow = _bootstrapInFlight.asStateFlow() + + private val _bootstrapProgress = MutableStateFlow(-1) + override val bootstrapProgress: StateFlow = _bootstrapProgress.asStateFlow() + + /** Models the directory download advancing. Only distinct values count as progress. */ + fun advanceBootstrapProgress(permille: Int) { + _bootstrapProgress.value = permille + } + + /** + * When true, [start] models a native bootstrap that is still running: status goes Connecting + * and [bootstrapInFlight] stays true until [finishBootstrapAttempt] is called. + */ + var holdBootstrapInFlight = false + + /** Models the native bootstrap attempt returning without reaching Active (Arti's own timeout). */ + fun finishBootstrapAttempt() { + _bootstrapInFlight.value = false + } + var startCount = 0 private set var stopCount = 0 @@ -515,9 +938,30 @@ private class FakeTorBackend : TorBackend { override suspend fun hasBootstrappedBefore(): Boolean = bootstrappedBefore + /** + * Set to model the real backend, whose `start()` does not return until the blocking native + * bootstrap attempt has finished. The status is published as soon as the attempt begins, the + * same as [TorService] does. + */ + var startNeverReturns = false + + /** When true, start() models a client that cannot be created at all: status stays Connecting. */ + var startFailsToConnect = false + + /** + * Mirrors [TorService]: the native client is created in ~130ms and the proxy binds, so a real + * start lands in [TorServiceStatus.Bootstrapping] — routable, directory still downloading — not + * in Connecting. Tests that want the pre-proxy state call [setConnecting]. + */ override suspend fun start() { startCount++ - _status.value = TorServiceStatus.Connecting + if (startFailsToConnect) { + _status.value = TorServiceStatus.Connecting + return + } + _status.value = TorServiceStatus.Bootstrapping(17392) + if (holdBootstrapInFlight) _bootstrapInFlight.value = true + if (startNeverReturns) awaitCancellation() } override suspend fun stop() { @@ -527,21 +971,28 @@ private class FakeTorBackend : TorBackend { override suspend fun reset() { resetCount++ + _bootstrapInFlight.value = false _status.value = TorServiceStatus.Off } override suspend fun resetWithCleanState() { resetWithCleanStateCount++ + _bootstrapInFlight.value = false _status.value = TorServiceStatus.Off } fun setActive(port: Int) { + _bootstrapInFlight.value = false _status.value = TorServiceStatus.Active(port) } fun setConnecting() { _status.value = TorServiceStatus.Connecting } + + fun setBootstrapping(port: Int = 17392) { + _status.value = TorServiceStatus.Bootstrapping(port) + } } /** In-memory [TorPreferencesPort] driven by tests. */ diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorServiceStatus.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorServiceStatus.kt index dbe8668f70..76ea3c2d47 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorServiceStatus.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/tor/TorServiceStatus.kt @@ -32,4 +32,18 @@ sealed class TorServiceStatus { data class Error( val message: String, ) : TorServiceStatus() + + /** + * Where to send bytes, or null. Mirrors the Android status class so callers express intent + * rather than matching variants. No `Bootstrapping` here on purpose: the desktop backend drives + * an external Tor, so it never observes the routable-but-not-yet-bootstrapped window that the + * in-process Arti client has, and a variant nothing emits is dead weight (see [Error], which is + * only ever constructed by `DesktopTorManager`). + */ + val socksPort: Int? + get() = (this as? Active)?.port + + /** Tor can build circuits right now. */ + val isFullyBootstrapped: Boolean + get() = this is Active } diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/tor/DesktopTorManager.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/tor/DesktopTorManager.kt index 2742782a25..5a4ad9e7ef 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/tor/DesktopTorManager.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/tor/DesktopTorManager.kt @@ -64,7 +64,7 @@ class DesktopTorManager( override val activePortOrNull: StateFlow = _status - .map { (it as? TorServiceStatus.Active)?.port } + .map { it.socksPort } .stateIn(scope, SharingStarted.Eagerly, null) private val runtime: TorRuntime by lazy { diff --git a/tools/arti-build/build-arti.sh b/tools/arti-build/build-arti.sh index 33ad3b2583..be7d1cacdd 100755 --- a/tools/arti-build/build-arti.sh +++ b/tools/arti-build/build-arti.sh @@ -224,6 +224,8 @@ verify_jni_symbols() { "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_initialize" "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_startSocksProxy" "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_stopSocksProxy" + "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_isBootstrapped" + "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_bootstrapProgressPermille" "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_destroy" ) diff --git a/tools/arti-build/src/lib.rs b/tools/arti-build/src/lib.rs index 237a07867b..0e4a2bc09c 100644 --- a/tools/arti-build/src/lib.rs +++ b/tools/arti-build/src/lib.rs @@ -3,7 +3,7 @@ use jni::objects::{JClass, JString, JObject, GlobalRef}; use jni::sys::{jint, jstring}; use jni::JavaVM; -use arti_client::TorClient; +use arti_client::{BootstrapBehavior, TorClient}; use arti_client::config::TorClientConfigBuilder; // `kind()` is a trait method (tor_error::HasKind), not inherent, so the trait has to be in // scope wherever we classify a connect failure. Both are re-exported by arti-client. @@ -23,6 +23,10 @@ static TOKIO_RUNTIME: Mutex> = Mutex::new(None); static JAVA_VM: Mutex> = Mutex::new(None); static LOG_CALLBACK: Mutex> = Mutex::new(None); static SOCKS_TASK: Mutex>> = Mutex::new(None); +// The background directory download started by initialize(). It holds an Arc, so +// destroy() must abort it too — otherwise the client cannot drop, the state file lock is never +// released, and the next initialize() fails. +static BOOTSTRAP_TASK: Mutex>> = Mutex::new(None); // Per-connection handler tasks. Tracked so destroy() can abort in-flight handlers // — otherwise their Arc clones keep the client alive and the // state file lock would not be released for the next initialize(). @@ -170,14 +174,6 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_initialize( std::fs::create_dir_all(&cache_dir).ok(); std::fs::create_dir_all(&state_dir).ok(); - // Bound the bootstrap so a hostile network (unreachable guards, wiped - // consensus) can't block this JNI call — and the Kotlin-side lifecycle lock - // it holds — indefinitely. On timeout the `create_bootstrapped` future is - // dropped, tearing down the partially built client, and -4 is returned so - // TorService can leave status Connecting and let the self-heal watchdog - // retry instead of wedging. The ABI is unchanged (still one String arg). - const BOOTSTRAP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); - let outcome: jint = runtime.block_on(async { log_info!("Creating Arti client..."); @@ -203,24 +199,62 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_initialize( } }; - match tokio::time::timeout(BOOTSTRAP_TIMEOUT, TorClient::create_bootstrapped(config)).await { - Ok(Ok(client)) => { - log_info!("Arti client created and bootstrapped"); - *ARTI_CLIENT.lock().unwrap() = Some(Arc::new(client)); - 0 + // Create the client WITHOUT waiting for the directory. + // + // `create_bootstrapped` used to block this JNI call — and the Kotlin lifecycle lock it + // holds — for the entire directory download: 12.6-33.8s measured on a cold install. The + // app treated Tor as absent for all of it, because `activePortOrNull` stays null until + // status flips to Active, so every relay dial fell back to 127.0.0.1:9050 (the Orbot + // default) where nothing listens, and failed instantly into backoff. + // + // With `BootstrapBehavior::OnDemand` the client is usable the moment it exists and each + // stream waits for the directory itself. The SOCKS proxy binds right away, so a dial + // issued mid-download queues on its own circuit instead of failing. Readiness stops being + // a global gate the app has to poll and becomes a property of individual connections. + // + // The `_async` variant is deliberate: it allows a short grace period for the state file + // lock, which a destroy()/initialize() cycle needs, where the sync one waits not at all. + let client = match TorClient::builder() + .config(config) + .bootstrap_behavior(BootstrapBehavior::OnDemand) + .create_unbootstrapped_async() + .await + { + Ok(c) => Arc::new(c), + Err(e) => { + log_error!("Failed to create Tor client: {:?}", e); + return -3; } - Ok(Err(e)) => { - log_error!("Failed to bootstrap Tor client: {:?}", e); - -3 - } - Err(_elapsed) => { - log_error!( - "Tor bootstrap timed out after {}s — aborting so the client can be retried", - BOOTSTRAP_TIMEOUT.as_secs() - ); - -4 + }; + + *ARTI_CLIENT.lock().unwrap() = Some(Arc::clone(&client)); + + // Start the download now rather than leaving it for the first stream to trigger, so it + // overlaps the login screen exactly as it used to, and publish the outcome so Kotlin can + // move the status from Bootstrapping to Active at the right moment. + let handle = tokio::spawn(async move { + let started = std::time::Instant::now(); + match client.bootstrap().await { + Ok(()) => log_info!( + "Arti directory bootstrap complete after {}ms", + started.elapsed().as_millis() + ), + // Not fatal and deliberately not latched anywhere: with OnDemand the next stream + // retries the bootstrap on its own, and isBootstrapped() reports live readiness, so + // a recovery after this point is picked up without us having to model it. + Err(e) => log_error!( + "Arti directory bootstrap failed after {}ms (streams will retry): {:?}", + started.elapsed().as_millis(), + e + ), } + }); + if let Some(previous) = BOOTSTRAP_TASK.lock().unwrap().replace(handle) { + previous.abort(); } + + log_info!("Arti client created (unbootstrapped; directory downloading in background)"); + 0 }); if outcome == 0 { @@ -486,6 +520,51 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_stopSocksPro 0 } +/// Directory-download progress in permille (0..1000), or -1 when there is no client. +/// +/// Lets Kotlin tell a slow download from a stalled one. A timeout cannot: measured cold downloads +/// ran 12.6-34.4s on the same hardware and network, so any fixed patience is either short enough to +/// kill healthy ones or long enough to sit on a dead one. Forward progress separates them exactly. +/// +/// Deliberately does NOT surface `BootstrapStatus::blocked()`. Arti documents it as best-effort and +/// warns it "may declare that Arti is stuck for reasons that are incorrect; or it may declare that +/// the client is not stuck when in fact no progress is being made" — acting on that would trade a +/// measurable signal for a guess. +#[no_mangle] +pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_bootstrapProgressPermille( + _env: JNIEnv, + _class: JClass, +) -> jint { + match ARTI_CLIENT.lock().unwrap().as_ref() { + Some(client) => (client.bootstrap_status().as_frac() * 1000.0).clamp(0.0, 1000.0) as jint, + None => -1, + } +} + +/// Live readiness: 1 = ready for traffic, 0 = not yet, -1 = no client at all. +/// +/// Asks Arti itself (`bootstrap_status().ready_for_traffic()`, a cheap borrow-and-clone of a small +/// struct) rather than latching the outcome of the one background `bootstrap()` call. That call can +/// fail while the client stays perfectly usable — with OnDemand the next stream just retries — so a +/// latched failure would report "not bootstrapped" forever against a Tor that actually works, +/// leaving the UI wrong and the exit-rotation self-heal disabled. +#[no_mangle] +pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_isBootstrapped( + _env: JNIEnv, + _class: JClass, +) -> jint { + match ARTI_CLIENT.lock().unwrap().as_ref() { + Some(client) => { + if client.bootstrap_status().ready_for_traffic() { + 1 + } else { + 0 + } + } + None => -1, + } +} + /// Destroy the TorClient — used by self-heal paths in Kotlin when Tor is /// stuck and the in-memory state (guards, circuits) needs to be rebuilt /// from scratch. Aborts the SOCKS listener and all in-flight per-connection @@ -521,6 +600,13 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_destroy( }); } + // The background directory download holds an Arc too, for as long as it runs — + // which on a dead network is indefinitely. Abort it with the handlers or the state file lock + // outlives this destroy() and the next initialize() cannot take it. + if let Some(h) = BOOTSTRAP_TASK.lock().unwrap().take() { + h.abort(); + } + // Abort all in-flight handlers — each holds an Arc clone, and // the client cannot drop (state file lock cannot release) while any clone // is alive. @@ -538,10 +624,10 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_destroy( }); } - // Drop the static Arc. If any handler is still holding a clone, the - // TorClient stays alive until that handler finishes — in which case the - // next initialize() will fail and Kotlin's clearAllArtiData retry path - // will handle it. + // Drop the static Arc. If any handler is still holding a clone, the TorClient stays alive + // until that handler finishes — in which case the next initialize() fails and Kotlin leaves + // status Connecting for TorManager's self-heal watchdog to retry. (It no longer wipes all Arti + // data inline: that turned a transient failure into a lost guard sample and a lost consensus.) let _ = ARTI_CLIENT.lock().unwrap().take(); log_info!("Arti client destroyed");