From d94d8776eb7a0b01b6ed602aa3fcf889bcb39fb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Apr 2026 13:01:31 +0000 Subject: [PATCH] fix: keep ArtiProxy alive across mode switches to avoid file lock conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ArtiProxy holds an exclusive filesystem lock. Destroying it on OFF and recreating on INTERNAL caused lock conflicts because the native layer needs time to release the lock. Instead, create ArtiProxy once and never destroy it. When Tor is OFF or EXTERNAL, the proxy sits idle with no SOCKS connections — negligible resource usage. This eliminates all file lock race conditions. Also wrap start/stop in NonCancellable to prevent transformLatest cancellation from leaking half-initialized proxy instances. https://claude.ai/code/session_01BApgDd5udqBzMqysSRMpZu --- .../amethyst/ui/tor/TorManager.kt | 2 - .../amethyst/ui/tor/TorService.kt | 124 ++++++++++-------- 2 files changed, 67 insertions(+), 59 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/tor/TorManager.kt index 1db8739ba..aa9538424 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 @@ -63,12 +63,10 @@ class TorManager( } TorType.OFF -> { - service.stop() emit(TorServiceStatus.Off) } TorType.EXTERNAL -> { - service.stop() if (externalSocksPort > 0) { emit(TorServiceStatus.Active(externalSocksPort)) } else { 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 c968b98f6..d8df41018 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 @@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.utils.Log import info.guardianproject.arti.ArtiLogListener import info.guardianproject.arti.ArtiProxy import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -95,75 +96,84 @@ class TorService( } suspend fun start() { - mutex.withLock { - if (artiProxy != null) { - // Already running — just ensure status is up to date - if (bootstrapped.get()) { - _status.value = TorServiceStatus.Active(currentPort) - } else { - _status.value = TorServiceStatus.Connecting - } - return - } - - _status.value = TorServiceStatus.Connecting - bootstrapped.set(false) - lastLogTime.set(System.currentTimeMillis()) - - withContext(Dispatchers.IO) { - var socksPort = DEFAULT_SOCKS_PORT - var lastError: Exception? = null - - for (attempt in 0 until MAX_PORT_RETRIES) { - try { - val proxy = - ArtiProxy - .Builder(context.applicationContext) - .setSocksPort(socksPort) - .setDnsPort(socksPort + 1) - .setLogListener(logListener) - .build() - - proxy.start() - artiProxy = proxy - currentPort = socksPort - lastError = null - Log.d("TorService") { "Arti started on port $socksPort" } - break - } catch (e: Exception) { - lastError = e - Log.e("TorService") { - "Failed to start Arti on port $socksPort (attempt ${attempt + 1}): ${e.message}" - } - socksPort++ + // NonCancellable ensures that if the calling coroutine is cancelled + // (e.g., by transformLatest switching modes), we don't leak a + // half-started ArtiProxy with no reference to stop it. + withContext(NonCancellable) { + mutex.withLock { + if (artiProxy != null) { + // Already running — just ensure status is up to date + if (bootstrapped.get()) { + _status.value = TorServiceStatus.Active(currentPort) + } else { + _status.value = TorServiceStatus.Connecting } + return@withContext } - if (lastError != null) { - Log.e("TorService") { "Failed to start Arti after $MAX_PORT_RETRIES attempts" } - _status.value = TorServiceStatus.Off + _status.value = TorServiceStatus.Connecting + bootstrapped.set(false) + lastLogTime.set(System.currentTimeMillis()) + + withContext(Dispatchers.IO) { + var socksPort = DEFAULT_SOCKS_PORT + var lastError: Exception? = null + + for (attempt in 0 until MAX_PORT_RETRIES) { + try { + val proxy = + ArtiProxy + .Builder(context.applicationContext) + .setSocksPort(socksPort) + .setDnsPort(socksPort + 1) + .setLogListener(logListener) + .build() + + proxy.start() + artiProxy = proxy + currentPort = socksPort + lastError = null + Log.d("TorService") { "Arti started on port $socksPort" } + break + } catch (e: Exception) { + lastError = e + Log.e("TorService") { + "Failed to start Arti on port $socksPort (attempt ${attempt + 1}): ${e.message}" + } + socksPort++ + } + } + + if (lastError != null) { + Log.e("TorService") { "Failed to start Arti after $MAX_PORT_RETRIES attempts" } + _status.value = TorServiceStatus.Off + } } } } } suspend fun stop() { - mutex.withLock { - val proxy = artiProxy ?: return@withLock - artiProxy = null - bootstrapped.set(false) + // NonCancellable ensures stop() completes fully even if the caller + // is cancelled, preventing leaked ArtiProxy instances and file locks. + withContext(NonCancellable) { + mutex.withLock { + val proxy = artiProxy ?: return@withContext + artiProxy = null + bootstrapped.set(false) - Log.d("TorService", "Stopping Arti") - withContext(Dispatchers.IO) { - try { - proxy.stop() - } catch (e: Exception) { - Log.d("TorService") { "Failed to stop Arti: ${e.message}" } + Log.d("TorService", "Stopping Arti") + withContext(Dispatchers.IO) { + try { + proxy.stop() + } catch (e: Exception) { + Log.d("TorService") { "Failed to stop Arti: ${e.message}" } + } + // Give the native layer time to release file locks + delay(STOP_SETTLE_MS) } - // Give the native layer time to release file locks - delay(STOP_SETTLE_MS) + _status.value = TorServiceStatus.Off } - _status.value = TorServiceStatus.Off } } }