From 07c72dc86c6edc2df3d25552366f0ff224842a7c Mon Sep 17 00:00:00 2001 From: darken Date: Sun, 3 May 2026 15:31:42 +0200 Subject: [PATCH] fix(aap): Cancel hung BluetoothSocket connect on timeout BluetoothSocket.connect() is a blocking JNI call that ignores coroutine cancellation, so the previous withTimeout in AapAutoConnect only cancelled the suspending wrapper while the native thread stayed pinned. Hung threads accumulated and could trigger ANRs. Move the timeout inside AapConnection and run the blocking connect on a daemon thread; on timeout, close the socket from the caller thread to unblock the native call (the documented Android pattern for cancelling in-flight L2CAP connects). Also cancel appScope before delegating uncaught exceptions so coroutines have a best-effort window to release resources before the system handler terminates the process. --- app/src/main/java/eu/darken/capod/App.kt | 15 +++++- .../capod/CapodUncaughtExceptionHandler.kt | 2 + .../capod/monitor/core/aap/AapAutoConnect.kt | 14 ++---- .../core/apple/aap/engine/AapConnection.kt | 46 +++++++++++++++++-- .../CapodUncaughtExceptionHandlerTest.kt | 24 ++++++++++ .../apple/aap/AapConnectionManagerTest.kt | 46 +++++++++++++++++++ 6 files changed, 131 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/eu/darken/capod/App.kt b/app/src/main/java/eu/darken/capod/App.kt index 06124c5e..ac652e88 100644 --- a/app/src/main/java/eu/darken/capod/App.kt +++ b/app/src/main/java/eu/darken/capod/App.kt @@ -18,7 +18,9 @@ import eu.darken.capod.monitor.core.DeviceMonitor import eu.darken.capod.monitor.core.devicesWithProfiles +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -41,7 +43,18 @@ open class App : Application() { if (BuildConfig.DEBUG) Logging.install(LogCatLogger()) val oldHandler = Thread.getDefaultUncaughtExceptionHandler() - Thread.setDefaultUncaughtExceptionHandler(CapodUncaughtExceptionHandler(oldHandler)) + Thread.setDefaultUncaughtExceptionHandler( + CapodUncaughtExceptionHandler( + previousHandler = oldHandler, + cancelBeforeDelegate = { throwable -> + // Best-effort shutdown: the system handler may terminate the process immediately, + // but cancellation can still close sockets if it gets a scheduling window. + if (::appScope.isInitialized) { + appScope.cancel(CancellationException("Uncaught exception", throwable)) + } + }, + ) + ) autoReporting.setup(this) diff --git a/app/src/main/java/eu/darken/capod/CapodUncaughtExceptionHandler.kt b/app/src/main/java/eu/darken/capod/CapodUncaughtExceptionHandler.kt index 4bef99ec..f7c102d0 100644 --- a/app/src/main/java/eu/darken/capod/CapodUncaughtExceptionHandler.kt +++ b/app/src/main/java/eu/darken/capod/CapodUncaughtExceptionHandler.kt @@ -20,6 +20,7 @@ internal class CapodUncaughtExceptionHandler( exception = throwable, ) }, + private val cancelBeforeDelegate: (Throwable) -> Unit = {}, private val exit: (Int) -> Unit = { exitProcess(it) }, ) : Thread.UncaughtExceptionHandler { @@ -57,6 +58,7 @@ internal class CapodUncaughtExceptionHandler( } private fun delegate(thread: Thread, throwable: Throwable) { + runCatching { cancelBeforeDelegate(throwable) } previousHandler?.uncaughtException(thread, throwable) ?: exit(1) } } diff --git a/app/src/main/java/eu/darken/capod/monitor/core/aap/AapAutoConnect.kt b/app/src/main/java/eu/darken/capod/monitor/core/aap/AapAutoConnect.kt index a841544f..ee5fa6ec 100644 --- a/app/src/main/java/eu/darken/capod/monitor/core/aap/AapAutoConnect.kt +++ b/app/src/main/java/eu/darken/capod/monitor/core/aap/AapAutoConnect.kt @@ -23,7 +23,6 @@ import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch -import kotlinx.coroutines.withTimeout import javax.inject.Inject import javax.inject.Singleton import kotlin.time.Duration.Companion.seconds @@ -77,9 +76,7 @@ class AapAutoConnect @Inject constructor( log(TAG) { "AAP connecting to $address (${profile.label})" } try { - withTimeout(CONNECT_TIMEOUT) { - aapManager.connect(address, bonded.internal, profile.model) - } + aapManager.connect(address, bonded.internal, profile.model) log(TAG) { "AAP connected to $address" } } catch (e: Exception) { log(TAG, WARN) { "AAP initial connect failed for $address: ${e.message}" } @@ -102,9 +99,7 @@ class AapAutoConnect @Inject constructor( try { log(TAG) { "AAP initial retry ${attempt + 1} for $address after ${delayMs}ms" } - withTimeout(CONNECT_TIMEOUT) { - aapManager.connect(address, bonded.internal, profile.model) - } + aapManager.connect(address, bonded.internal, profile.model) log(TAG) { "AAP connected to $address on retry ${attempt + 1}" } break } catch (retryException: Exception) { @@ -164,9 +159,7 @@ class AapAutoConnect @Inject constructor( try { log(TAG) { "AAP reconnect attempt ${attempt + 1} for $address in ${delayMs}ms" } - withTimeout(CONNECT_TIMEOUT) { - aapManager.connect(address, bonded.internal, profile.model) - } + aapManager.connect(address, bonded.internal, profile.model) log(TAG) { "AAP reconnected to $address" } break } catch (e: Exception) { @@ -231,6 +224,5 @@ class AapAutoConnect @Inject constructor( companion object { private val TAG = logTag("Monitor", "AapAutoConnect") internal val RETRY_DELAYS = longArrayOf(3_000, 3_000, 3_000, 5_000, 5_000, 10_000, 10_000) - private val CONNECT_TIMEOUT = 5.seconds } } diff --git a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/engine/AapConnection.kt b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/engine/AapConnection.kt index d6c1b653..8f641763 100644 --- a/app/src/main/java/eu/darken/capod/pods/core/apple/aap/engine/AapConnection.kt +++ b/app/src/main/java/eu/darken/capod/pods/core/apple/aap/engine/AapConnection.kt @@ -16,6 +16,7 @@ import eu.darken.capod.pods.core.apple.aap.protocol.AapPacket import eu.darken.capod.pods.core.apple.aap.protocol.AapSleepEvent import eu.darken.capod.pods.core.apple.aap.protocol.KeyExchangeResult import eu.darken.capod.pods.core.apple.aap.protocol.StemPressEvent +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -26,7 +27,11 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import java.io.IOException +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds /** * Manages a single AAP L2CAP connection to a device. @@ -39,6 +44,7 @@ internal class AapConnection( private val profile: AapDeviceProfile, private val socketFactory: L2capSocketFactory, timeSource: TimeSource, + private val connectTimeout: Duration = DEFAULT_CONNECT_TIMEOUT, ) { private val engine = AapSessionEngine(profile, timeSource) @@ -68,7 +74,7 @@ internal class AapConnection( try { val sock = socketFactory.createSocket(device, PSM) - sock.connect() + sock.connectCancellable() socket = sock log(TAG, Logging.Priority.INFO) { "Connected to ${device.address}" } @@ -212,15 +218,47 @@ internal class AapConnection( } private fun cleanupSocket() { + socket?.closeQuietly() + socket = null + } + + private suspend fun BluetoothSocket.connectCancellable() { + val result = CompletableDeferred>() + val cancelled = AtomicBoolean(false) + val connectThread = Thread( + { + val outcome = runCatching { connect() } + result.complete(outcome) + if (cancelled.get() && outcome.isSuccess) closeQuietly() + }, + "AAP-L2CAP-connect-${device.address}", + ).apply { + isDaemon = true + start() + } + try { - socket?.close() + withTimeout(connectTimeout) { + result.await().getOrThrow() + } + } catch (e: Exception) { + cancelled.set(true) + closeQuietly() + connectThread.interrupt() + throw e + } + } + + private fun BluetoothSocket.closeQuietly() { + try { + close() } catch (_: Exception) { } - socket = null } companion object { private const val PSM = 0x1001 + internal val DEFAULT_CONNECT_TIMEOUT = 5.seconds private val TAG = logTag("AAP", "Connection") } -} \ No newline at end of file +} diff --git a/app/src/test/java/eu/darken/capod/CapodUncaughtExceptionHandlerTest.kt b/app/src/test/java/eu/darken/capod/CapodUncaughtExceptionHandlerTest.kt index 99c65fc3..23331fbb 100644 --- a/app/src/test/java/eu/darken/capod/CapodUncaughtExceptionHandlerTest.kt +++ b/app/src/test/java/eu/darken/capod/CapodUncaughtExceptionHandlerTest.kt @@ -89,6 +89,30 @@ class CapodUncaughtExceptionHandlerTest : BaseTest() { previousHandler.throwables shouldBe listOf(throwable) } + @Test + fun `cancels before delegating fatal exception`() { + val mainThread = Thread.currentThread() + val events = mutableListOf() + val previousHandler = object : Thread.UncaughtExceptionHandler { + override fun uncaughtException(thread: Thread, throwable: Throwable) { + events += "delegate" + } + } + val throwable = IllegalStateException("boom") + val handler = CapodUncaughtExceptionHandler( + previousHandler = previousHandler, + mainThreadProvider = { mainThread }, + loopMainThread = { throw AssertionError("loopMainThread should not run") }, + reportForegroundServiceTimingException = { throw AssertionError("report should not run") }, + cancelBeforeDelegate = { events += "cancel" }, + exit = { throw AssertionError("exitProcess($it)") }, + ) + + handler.uncaughtException(mainThread, throwable) + + events shouldBe listOf("cancel", "delegate") + } + @Test fun `delegates loop failure after suppression`() { val mainThread = Thread.currentThread() diff --git a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/AapConnectionManagerTest.kt b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/AapConnectionManagerTest.kt index 6789a4a0..5d071acb 100644 --- a/app/src/test/java/eu/darken/capod/pods/core/apple/aap/AapConnectionManagerTest.kt +++ b/app/src/test/java/eu/darken/capod/pods/core/apple/aap/AapConnectionManagerTest.kt @@ -5,7 +5,9 @@ import android.bluetooth.BluetoothSocket import eu.darken.capod.common.TimeSource import eu.darken.capod.common.bluetooth.l2cap.L2capSocketFactory import eu.darken.capod.pods.core.apple.PodModel +import eu.darken.capod.pods.core.apple.aap.engine.AapConnection import eu.darken.capod.pods.core.apple.aap.protocol.AapCommand +import eu.darken.capod.pods.core.apple.aap.protocol.AapDeviceProfile import eu.darken.capod.pods.core.apple.aap.protocol.AapSetting import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.maps.shouldBeEmpty @@ -13,6 +15,7 @@ import io.kotest.matchers.shouldBe import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest @@ -24,6 +27,10 @@ import testhelpers.TestTimeSource import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.IOException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import kotlin.time.Duration.Companion.milliseconds class AapConnectionManagerTest : BaseTest() { @@ -89,6 +96,45 @@ class AapConnectionManagerTest : BaseTest() { manager.allStates.value.shouldBeEmpty() } + @Test + fun `connect timeout closes in-flight socket and allows reconnect`() = testScope.runTest { + val closeCalled = CountDownLatch(1) + val closeCalls = AtomicInteger(0) + val blockingSocket = mockk(relaxed = true) { + every { connect() } answers { + closeCalled.await(1, TimeUnit.SECONDS) + throw IOException("closed") + } + every { close() } answers { + closeCalls.incrementAndGet() + closeCalled.countDown() + } + } + val reconnectSocket = mockk(relaxed = true) { + every { outputStream } returns ByteArrayOutputStream() + every { inputStream } returns ByteArrayInputStream(byteArrayOf()) + } + every { socketFactory.createSocket(any(), any()) } returnsMany listOf(blockingSocket, reconnectSocket) + val connection = AapConnection( + device = testDevice, + profile = AapDeviceProfile.forModel(PodModel.AIRPODS_PRO3), + socketFactory = socketFactory, + timeSource = timeSource, + connectTimeout = 50.milliseconds, + ) + + shouldThrow { + connection.connect(testScope) + } + + closeCalled.await(1, TimeUnit.SECONDS) shouldBe true + closeCalls.get() shouldBe 1 + connection.state.value.connectionState shouldBe AapPodState.ConnectionState.DISCONNECTED + + connection.connect(testScope) + advanceUntilIdle() + } + @Test fun `remote disconnect cleans up allStates`() = testScope.runTest { // Empty inputStream → readLoop gets -1 immediately → DISCONNECTED