mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
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.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Result<Unit>>()
|
||||
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")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String>()
|
||||
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()
|
||||
|
||||
@@ -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<BluetoothSocket>(relaxed = true) {
|
||||
every { connect() } answers {
|
||||
closeCalled.await(1, TimeUnit.SECONDS)
|
||||
throw IOException("closed")
|
||||
}
|
||||
every { close() } answers {
|
||||
closeCalls.incrementAndGet()
|
||||
closeCalled.countDown()
|
||||
}
|
||||
}
|
||||
val reconnectSocket = mockk<BluetoothSocket>(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<TimeoutCancellationException> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user