From 799996fa0b2237794413c81c8188eb81a29706de Mon Sep 17 00:00:00 2001 From: hawkff <109485367+hawkff@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:37:10 -0400 Subject: [PATCH] fix(service): suspend blocking teardown off main --- .../io/nekohasekai/sagernet/bg/BaseService.kt | 19 ++- .../sagernet/bg/ServiceTeardownExecution.kt | 36 ++++ .../io/nekohasekai/sagernet/bg/VpnService.kt | 9 +- .../sagernet/bg/proto/ProxyInstance.kt | 24 ++- .../bg/ServiceTeardownExecutionTest.kt | 156 ++++++++++++++++++ 5 files changed, 224 insertions(+), 20 deletions(-) create mode 100644 app/src/main/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecution.kt create mode 100644 app/src/test/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecutionTest.kt diff --git a/app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt b/app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt index a3c627d4b..ea97c8938 100644 --- a/app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt +++ b/app/src/main/java/io/nekohasekai/sagernet/bg/BaseService.kt @@ -312,14 +312,17 @@ class BaseService { } } - fun killProcesses() { - data.proxy?.close() - wakeLock?.apply { - release() - wakeLock = null - } - runOnDefaultDispatcher { - DefaultNetworkListener.stop(this@Interface) + suspend fun killProcesses() { + runServiceTeardown( + after = { + wakeLock?.apply { + release() + wakeLock = null + } + DefaultNetworkListener.stop(this@Interface) + }, + ) { + data.proxy?.closeAndPersist() } } diff --git a/app/src/main/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecution.kt b/app/src/main/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecution.kt new file mode 100644 index 000000000..9968b39fb --- /dev/null +++ b/app/src/main/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecution.kt @@ -0,0 +1,36 @@ +package io.nekohasekai.sagernet.bg + +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext + +internal suspend fun runServiceTeardown( + dispatcher: CoroutineDispatcher = Dispatchers.IO, + after: suspend () -> Unit = {}, + block: suspend () -> Unit, +) = withContext(NonCancellable) { + runRequiredCompletion(after) { + withContext(dispatcher) { + block() + } + } +} + +internal suspend fun runRequiredCompletion(after: suspend () -> Unit, block: suspend () -> Unit) { + var failure: Throwable? = null + try { + block() + } catch (throwable: Throwable) { + failure = throwable + } + try { + after() + } catch (throwable: Throwable) { + val primaryFailure = failure + if (primaryFailure == null) throw throwable + if (primaryFailure !== throwable) primaryFailure.addSuppressed(throwable) + } + val finalFailure = failure + if (finalFailure != null) throw finalFailure +} diff --git a/app/src/main/java/io/nekohasekai/sagernet/bg/VpnService.kt b/app/src/main/java/io/nekohasekai/sagernet/bg/VpnService.kt index a7a6ca676..58376f3f2 100644 --- a/app/src/main/java/io/nekohasekai/sagernet/bg/VpnService.kt +++ b/app/src/main/java/io/nekohasekai/sagernet/bg/VpnService.kt @@ -51,10 +51,11 @@ class VpnService : } @Suppress("EXPERIMENTAL_API_USAGE") - override fun killProcesses() { - conn?.close() - conn = null - super.killProcesses() + override suspend fun killProcesses() { + runServiceTeardown(after = { super.killProcesses() }) { + conn?.close() + conn = null + } } override fun onBind(intent: Intent) = when (intent.action) { diff --git a/app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt b/app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt index fa1610a19..2bf3ed7e8 100644 --- a/app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt +++ b/app/src/main/java/io/nekohasekai/sagernet/bg/proto/ProxyInstance.kt @@ -2,6 +2,7 @@ package io.nekohasekai.sagernet.bg.proto import io.nekohasekai.sagernet.bg.BaseService import io.nekohasekai.sagernet.bg.ServiceNotification +import io.nekohasekai.sagernet.bg.runRequiredCompletion import io.nekohasekai.sagernet.database.ProxyEntity import io.nekohasekai.sagernet.ktx.Logs import kotlinx.coroutines.Dispatchers @@ -51,14 +52,21 @@ class ProxyInstance(profile: ProxyEntity, var service: BaseService.Interface? = looper?.start() } - override fun close() { + suspend fun closeAndPersist() = runRequiredCompletion( + after = { + try { + looper?.stop() + } finally { + looper = null + } + }, + ) { super.close() - // Teardown is called on the main thread; the final traffic flush in looper.stop() does - // synchronous DAO writes, so run the blocking body on a background dispatcher to keep it - // off the UI thread (Plan 027 — main-thread-DB allowance removed). - runBlocking(Dispatchers.Default) { - looper?.stop() - looper = null - } + } + + // Synchronous compatibility path for Closeable callers. Service teardown uses + // closeAndPersist() through runServiceTeardown instead of blocking its caller. + override fun close() = runBlocking(Dispatchers.Default) { + closeAndPersist() } } diff --git a/app/src/test/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecutionTest.kt b/app/src/test/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecutionTest.kt new file mode 100644 index 000000000..b893ff6cb --- /dev/null +++ b/app/src/test/java/io/nekohasekai/sagernet/bg/ServiceTeardownExecutionTest.kt @@ -0,0 +1,156 @@ +package io.nekohasekai.sagernet.bg + +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import kotlin.coroutines.ContinuationInterceptor + +@OptIn(ExperimentalCoroutinesApi::class) +class ServiceTeardownExecutionTest { + + @Test + fun callerWaitsForTeardownToComplete() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val releaseTeardown = CompletableDeferred() + val caller = launch { + runServiceTeardown(dispatcher) { + releaseTeardown.await() + } + } + + runCurrent() + assertFalse(caller.isCompleted) + + releaseTeardown.complete(Unit) + advanceUntilIdle() + assertTrue(caller.isCompleted) + } + + @Test + fun parentCancellationAfterEntryDoesNotSkipTeardown() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val teardownEntered = CompletableDeferred() + val releaseTeardown = CompletableDeferred() + var teardownCompleted = false + val caller = launch { + runServiceTeardown(dispatcher) { + teardownEntered.complete(Unit) + releaseTeardown.await() + teardownCompleted = true + } + } + + runCurrent() + teardownEntered.await() + caller.cancel() + releaseTeardown.complete(Unit) + advanceUntilIdle() + + assertTrue(teardownCompleted) + assertTrue(caller.isCancelled) + } + + @Test + fun parentCancellationDoesNotSkipRequiredCompletion() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + val teardownEntered = CompletableDeferred() + val releaseTeardown = CompletableDeferred() + var completionRan = false + val caller = launch { + runServiceTeardown( + dispatcher = dispatcher, + after = { completionRan = true }, + ) { + teardownEntered.complete(Unit) + releaseTeardown.await() + } + } + + runCurrent() + teardownEntered.await() + caller.cancel() + releaseTeardown.complete(Unit) + advanceUntilIdle() + + assertTrue(completionRan) + assertTrue(caller.isCancelled) + } + + @Test + fun teardownExceptionPropagatesAfterRequiredCompletion() = runBlocking { + val failure = IllegalStateException("teardown failure") + var completionRan = false + + val result = runCatching { + runServiceTeardown( + dispatcher = Dispatchers.Unconfined, + after = { completionRan = true }, + ) { + throw failure + } + } + + assertTrue(completionRan) + var propagated = result.exceptionOrNull() + while (propagated != null && propagated !== failure) { + propagated = propagated.cause + } + assertSame(failure, propagated) + } + + @Test + fun completionExceptionIsSuppressedByTeardownFailure() = runBlocking { + val teardownFailure = IllegalStateException("teardown failure") + val completionFailure = IllegalArgumentException("completion failure") + val result = runCatching { + runRequiredCompletion( + after = { throw completionFailure }, + ) { + throw teardownFailure + } + } + + val failure = result.exceptionOrNull() + assertSame(teardownFailure, failure) + assertSame(completionFailure, failure?.suppressed?.single()) + } + + @Test + fun repeatedFailureInstancePreservesOriginalFailure() = runBlocking { + val failure = IllegalStateException("shared failure") + val result = runCatching { + runRequiredCompletion( + after = { throw failure }, + ) { + throw failure + } + } + + val thrown = result.exceptionOrNull() + assertSame(failure, thrown) + assertTrue(thrown?.suppressed?.isEmpty() == true) + } + + @Test + fun teardownUsesSuppliedDispatcher() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var observedDispatcher: ContinuationInterceptor? = null + + runServiceTeardown(dispatcher) { + observedDispatcher = currentCoroutineContext()[ContinuationInterceptor] + } + + assertSame(dispatcher, observedDispatcher) + } +}