Fix crash in peripheral <init> when parent job is already complete (#…

…665)
JuulLabs · Apr 4, 2024 · b500995 · b500995
1 parent 8213be8
commit b500995
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 18 deletions.
diff --git a/core/src/androidMain/kotlin/BluetoothDeviceAndroidPeripheral.kt b/core/src/androidMain/kotlin/BluetoothDeviceAndroidPeripheral.kt
@@ -68,12 +68,6 @@ internal class BluetoothDeviceAndroidPeripheral(
     private val logging: Logging,
 ) : AndroidPeripheral {
 
-    private val scope = CoroutineScope(
-        parentCoroutineContext +
-            SupervisorJob(parentCoroutineContext.job).apply { invokeOnCompletion(::dispose) } +
-            CoroutineName("Kable/Peripheral/${bluetoothDevice.address}"),
-    )
-
     private val logger = Logger(logging, tag = "Kable/Peripheral", identifier = bluetoothDevice.address)
 
     private val _state = MutableStateFlow<State>(Disconnected())
@@ -105,6 +99,17 @@ internal class BluetoothDeviceAndroidPeripheral(
 
     override val name: String? get() = bluetoothDevice.name
 
+    /**
+     * It's important that we instantiate this scope as late as possible, since [dispose] will be
+     * called immediately if the parent job is already complete. Doing so late in <init> is fine,
+     * but early in <init> it could reference non-nullable variables that are not yet set and crash.
+     */
+    private val scope = CoroutineScope(
+        parentCoroutineContext +
+            SupervisorJob(parentCoroutineContext.job).apply { invokeOnCompletion(::dispose) } +
+            CoroutineName("Kable/Peripheral/${bluetoothDevice.address}"),
+    )
+
     private val connectAction = scope.sharedRepeatableAction(::establishConnection)
 
     private suspend fun establishConnection(scope: CoroutineScope) {

diff --git a/core/src/appleMain/kotlin/CBPeripheralCoreBluetoothPeripheral.kt b/core/src/appleMain/kotlin/CBPeripheralCoreBluetoothPeripheral.kt
@@ -81,12 +81,6 @@ internal class CBPeripheralCoreBluetoothPeripheral(
     private val logging: Logging,
 ) : CoreBluetoothPeripheral {
 
-    private val scope = CoroutineScope(
-        parentCoroutineContext +
-            SupervisorJob(parentCoroutineContext.job).apply { invokeOnCompletion(::dispose) } +
-            CoroutineName("Kable/Peripheral/${cbPeripheral.identifier.UUIDString}"),
-    )
-
     private val logger = Logger(logging, identifier = cbPeripheral.identifier.UUIDString)
 
     private val centralManager: CentralManager = CentralManager.Default
@@ -98,6 +92,17 @@ internal class CBPeripheralCoreBluetoothPeripheral(
 
     private val observers = Observers<NSData>(this, logging, exceptionHandler = observationExceptionHandler)
 
+    /**
+     * It's important that we instantiate this scope as late as possible, since [dispose] will be
+     * called immediately if the parent job is already complete. Doing so late in <init> is fine,
+     * but early in <init> it could reference non-nullable variables that are not yet set and crash.
+     */
+    private val scope = CoroutineScope(
+        parentCoroutineContext +
+            SupervisorJob(parentCoroutineContext.job).apply { invokeOnCompletion(::dispose) } +
+            CoroutineName("Kable/Peripheral/${cbPeripheral.identifier.UUIDString}"),
+    )
+
     init {
         centralManager.delegate
             .connectionState

diff --git a/core/src/jsMain/kotlin/BluetoothDeviceWebBluetoothPeripheral.kt b/core/src/jsMain/kotlin/BluetoothDeviceWebBluetoothPeripheral.kt
@@ -46,12 +46,6 @@ internal class BluetoothDeviceWebBluetoothPeripheral(
     logging: Logging,
 ) : WebBluetoothPeripheral {
 
-    private val job = SupervisorJob(parentCoroutineContext.job).apply {
-        invokeOnCompletion { finalCleanup() }
-    }
-
-    private val scope = CoroutineScope(parentCoroutineContext + job)
-
     private val logger = Logger(logging, identifier = bluetoothDevice.id)
 
     private val ioLock = Mutex()
@@ -75,6 +69,17 @@ internal class BluetoothDeviceWebBluetoothPeripheral(
 
     override val name: String? get() = bluetoothDevice.name
 
+    /**
+     * It's important that we instantiate this job as late as possible, since [finalCleanup] will be
+     * called immediately if the parent job is already complete. Doing so late in <init> is fine,
+     * but early in <init> it could reference non-nullable variables that are not yet set and crash.
+     */
+    private val job = SupervisorJob(parentCoroutineContext.job).apply {
+        invokeOnCompletion { finalCleanup() }
+    }
+
+    private val scope = CoroutineScope(parentCoroutineContext + job)
+
     override suspend fun rssi(): Int = suspendCancellableCoroutine { continuation ->
         check(supportsAdvertisements) { "watchAdvertisements unavailable" }