Skip to content

Commit 9bb35ad

Browse files
committed
fix(ios): Improve thread-safety on iOS (#779)
- Converted callback and device dictionaries to ThreadSafeDictionary across all iOS Bluetooth classes - Replaced check-then-act patterns with atomic operations to prevent Device instance duplication and state loss - Converted force-unwrap patterns to safe optional binding throughout Key threading behaviours are: - CoreBluetooth delegates execute serially on main queue, but async blocks and timeouts can interleave between callbacks Fixes #779
1 parent 3fa8a9e commit 9bb35ad

File tree

5 files changed

+134
-128
lines changed

5 files changed

+134
-128
lines changed

.nvmrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
v24.11.1

ios/Sources/BluetoothLe/Device.swift

Lines changed: 31 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ class Device: NSObject, CBPeripheralDelegate {
66
typealias Callback = (_ success: Bool, _ value: String) -> Void
77

88
private var peripheral: CBPeripheral!
9-
private var callbackMap = ThreadSafeDictionary<String, Callback>()
10-
private var timeoutMap = [String: DispatchWorkItem]()
9+
private let callbackMap = ThreadSafeDictionary<String, Callback>()
10+
private let timeoutMap = ThreadSafeDictionary<String, DispatchWorkItem>()
1111
private var servicesCount = 0
1212
private var servicesDiscovered = 0
1313
private var characteristicsCount = 0
@@ -51,8 +51,8 @@ class Device: NSObject, CBPeripheralDelegate {
5151
didDiscoverServices error: Error?
5252
) {
5353
log("didDiscoverServices")
54-
if error != nil {
55-
log("Error", error!.localizedDescription)
54+
if let error = error {
55+
log("Error", error.localizedDescription)
5656
return
5757
}
5858
self.servicesCount = peripheral.services?.count ?? 0
@@ -130,8 +130,8 @@ class Device: NSObject, CBPeripheralDelegate {
130130
error: Error?
131131
) {
132132
let key = "readRssi"
133-
if error != nil {
134-
self.reject(key, error!.localizedDescription)
133+
if let error = error {
134+
self.reject(key, error.localizedDescription)
135135
return
136136
}
137137
self.resolve(key, RSSI.stringValue)
@@ -193,8 +193,8 @@ class Device: NSObject, CBPeripheralDelegate {
193193
) {
194194
let key = self.getKey("read", characteristic)
195195
let notifyKey = self.getKey("notification", characteristic)
196-
if error != nil {
197-
self.reject(key, error!.localizedDescription)
196+
if let error = error {
197+
self.reject(key, error.localizedDescription)
198198
return
199199
}
200200
if characteristic.value == nil {
@@ -206,9 +206,8 @@ class Device: NSObject, CBPeripheralDelegate {
206206
self.resolve(key, valueString)
207207

208208
// notifications
209-
let callback = self.callbackMap[notifyKey]
210-
if callback != nil {
211-
callback!(true, valueString)
209+
if let callback = self.callbackMap[notifyKey] {
210+
callback(true, valueString)
212211
}
213212
}
214213

@@ -236,8 +235,8 @@ class Device: NSObject, CBPeripheralDelegate {
236235
error: Error?
237236
) {
238237
let key = self.getKey("readDescriptor", descriptor)
239-
if error != nil {
240-
self.reject(key, error!.localizedDescription)
238+
if let error = error {
239+
self.reject(key, error.localizedDescription)
241240
return
242241
}
243242
if descriptor.value == nil {
@@ -277,8 +276,8 @@ class Device: NSObject, CBPeripheralDelegate {
277276
error: Error?
278277
) {
279278
let key = self.getKey("write", characteristic)
280-
if error != nil {
281-
self.reject(key, error!.localizedDescription)
279+
if let error = error {
280+
self.reject(key, error.localizedDescription)
282281
return
283282
}
284283
self.resolve(key, "Successfully written value.")
@@ -309,8 +308,8 @@ class Device: NSObject, CBPeripheralDelegate {
309308
error: Error?
310309
) {
311310
let key = self.getKey("writeDescriptor", descriptor)
312-
if error != nil {
313-
self.reject(key, error!.localizedDescription)
311+
if let error = error {
312+
self.reject(key, error.localizedDescription)
314313
return
315314
}
316315
self.resolve(key, "Successfully written descriptor value.")
@@ -327,7 +326,7 @@ class Device: NSObject, CBPeripheralDelegate {
327326
let key = "setNotifications|\(serviceUUID.uuidString)|\(characteristicUUID.uuidString)"
328327
let notifyKey = "notification|\(serviceUUID.uuidString)|\(characteristicUUID.uuidString)"
329328
self.callbackMap[key] = callback
330-
if notifyCallback != nil {
329+
if let notifyCallback = notifyCallback {
331330
self.callbackMap[notifyKey] = notifyCallback
332331
}
333332
guard let characteristic = self.getCharacteristic(serviceUUID, characteristicUUID) else {
@@ -345,8 +344,8 @@ class Device: NSObject, CBPeripheralDelegate {
345344
error: Error?
346345
) {
347346
let key = self.getKey("setNotifications", characteristic)
348-
if error != nil {
349-
self.reject(key, error!.localizedDescription)
347+
if let error = error {
348+
self.reject(key, error.localizedDescription)
350349
return
351350
}
352351
self.resolve(key, "Successfully set notifications.")
@@ -357,15 +356,14 @@ class Device: NSObject, CBPeripheralDelegate {
357356
_ characteristic: CBCharacteristic?
358357
) -> String {
359358
let serviceUUIDString: String
360-
let service: CBService? = characteristic?.service
361-
if service != nil {
362-
serviceUUIDString = cbuuidToStringUppercase(service!.uuid)
359+
if let service = characteristic?.service {
360+
serviceUUIDString = cbuuidToStringUppercase(service.uuid)
363361
} else {
364362
serviceUUIDString = "UNKNOWN-SERVICE"
365363
}
366364
let characteristicUUIDString: String
367-
if characteristic != nil {
368-
characteristicUUIDString = cbuuidToStringUppercase(characteristic!.uuid)
365+
if let characteristic = characteristic {
366+
characteristicUUIDString = cbuuidToStringUppercase(characteristic.uuid)
369367
} else {
370368
characteristicUUIDString = "UNKNOWN-CHARACTERISTIC"
371369
}
@@ -385,28 +383,20 @@ class Device: NSObject, CBPeripheralDelegate {
385383
_ key: String,
386384
_ value: String
387385
) {
388-
let callback = self.callbackMap[key]
389-
if callback != nil {
390-
log("Resolve", key, value)
391-
callback!(true, value)
392-
self.callbackMap[key] = nil
393-
self.timeoutMap[key]?.cancel()
394-
self.timeoutMap[key] = nil
395-
}
386+
guard let callback = self.callbackMap.removeValue(forKey: key) else { return }
387+
self.timeoutMap.removeValue(forKey: key)?.cancel()
388+
log("Resolve", key, value)
389+
callback(true, value)
396390
}
397391

398392
private func reject(
399393
_ key: String,
400394
_ value: String
401395
) {
402-
let callback = self.callbackMap[key]
403-
if callback != nil {
404-
log("Reject", key, value)
405-
callback!(false, value)
406-
self.callbackMap[key] = nil
407-
self.timeoutMap[key]?.cancel()
408-
self.timeoutMap[key] = nil
409-
}
396+
guard let callback = self.callbackMap.removeValue(forKey: key) else { return }
397+
self.timeoutMap.removeValue(forKey: key)?.cancel()
398+
log("Reject", key, value)
399+
callback(false, value)
410400
}
411401

412402
private func setTimeout(

ios/Sources/BluetoothLe/DeviceManager.swift

Lines changed: 52 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,17 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
1414
typealias ScanResultCallback = (_ device: Device, _ advertisementData: [String: Any], _ rssi: NSNumber) -> Void
1515

1616
private var centralManager: CBCentralManager!
17-
private var viewController: UIViewController?
17+
private let viewController: UIViewController?
1818
private var displayStrings: [String: String]!
19-
private var callbackMap = [String: Callback]()
19+
private let callbackMap = ThreadSafeDictionary<String, Callback>()
2020
private var scanResultCallback: ScanResultCallback?
2121
private var stateReceiver: StateReceiver?
22-
private var timeoutMap = [String: DispatchWorkItem]()
22+
private let timeoutMap = ThreadSafeDictionary<String, DispatchWorkItem>()
2323
private var stopScanWorkItem: DispatchWorkItem?
2424
private var alertController: UIAlertController?
2525
private var deviceListView: DeviceListView?
2626
private var popoverController: UIPopoverPresentationController?
27-
private var discoveredDevices = [String: Device]()
27+
private let discoveredDevices = ThreadSafeDictionary<String, Device>()
2828
private var deviceNameFilter: String?
2929
private var deviceNamePrefixFilter: String?
3030
private var deviceListMode: DeviceListMode = .none
@@ -33,8 +33,8 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
3333
private var serviceDataFilters: [ServiceDataFilter]?
3434

3535
init(_ viewController: UIViewController?, _ displayStrings: [String: String], _ callback: @escaping Callback) {
36-
super.init()
3736
self.viewController = viewController
37+
super.init()
3838
self.displayStrings = displayStrings
3939
self.callbackMap["initialize"] = callback
4040
self.centralManager = CBCentralManager(delegate: self, queue: DispatchQueue.main)
@@ -102,7 +102,7 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
102102
self.scanResultCallback = scanResultCallback
103103

104104
if self.centralManager.isScanning == false {
105-
self.discoveredDevices = [String: Device]()
105+
self.discoveredDevices.removeAll()
106106
self.deviceListMode = deviceListMode
107107
self.allowDuplicates = allowDuplicates
108108
self.deviceNameFilter = name
@@ -114,11 +114,12 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
114114
self.showDeviceList()
115115
}
116116

117-
if scanDuration != nil {
118-
self.stopScanWorkItem = DispatchWorkItem {
117+
if let scanDuration = scanDuration {
118+
let workItem = DispatchWorkItem {
119119
self.stopScan()
120120
}
121-
DispatchQueue.main.asyncAfter(deadline: .now() + scanDuration!, execute: self.stopScanWorkItem!)
121+
self.stopScanWorkItem = workItem
122+
DispatchQueue.main.asyncAfter(deadline: .now() + scanDuration, execute: workItem)
122123
}
123124
self.centralManager.scanForPeripherals(
124125
withServices: serviceUUIDs,
@@ -174,43 +175,42 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
174175
return
175176
}
176177

177-
let isNew = self.discoveredDevices[peripheral.identifier.uuidString] == nil
178-
guard isNew || self.allowDuplicates else { return }
179-
180178
guard self.passesNameFilter(peripheralName: peripheral.name) else { return }
181179
guard self.passesNamePrefixFilter(peripheralName: peripheral.name) else { return }
182180
guard ScanFilterUtils.passesManufacturerDataFilter(advertisementData, filters: self.manufacturerDataFilters) else { return }
183181
guard ScanFilterUtils.passesServiceDataFilter(advertisementData, filters: self.serviceDataFilters) else { return }
184182

185-
let device: Device
186-
if self.allowDuplicates, let knownDevice = discoveredDevices.first(where: { $0.key == peripheral.identifier.uuidString })?.value {
187-
device = knownDevice
188-
} else {
189-
device = Device(peripheral)
190-
self.discoveredDevices[device.getId()] = device
183+
let deviceId = peripheral.identifier.uuidString
184+
let result = self.discoveredDevices.getOrInsert(key: deviceId) {
185+
Device(peripheral)
191186
}
192-
log("New device found: ", device.getName() ?? "Unknown")
187+
let device = result.value
188+
let isNew = result.wasInserted
193189

194-
switch deviceListMode {
195-
case .none:
196-
if self.scanResultCallback != nil {
197-
self.scanResultCallback!(device, advertisementData, RSSI)
198-
}
199-
case .alert:
200-
DispatchQueue.main.async { [weak self] in
201-
self?.alertController?.addAction(UIAlertAction(title: device.getName() ?? "Unknown", style: UIAlertAction.Style.default, handler: { (_) in
202-
log("Selected device")
203-
self?.stopScan()
204-
self?.resolve("startScanning", device.getId())
205-
}))
206-
}
207-
case .list:
208-
DispatchQueue.main.async { [weak self] in
209-
self?.deviceListView?.addItem(device.getName() ?? "Unknown", action: {
210-
log("Selected device")
211-
self?.stopScan()
212-
self?.resolve("startScanning", device.getId())
213-
})
190+
if isNew || self.allowDuplicates {
191+
log("New device found: ", device.getName() ?? "Unknown")
192+
193+
switch deviceListMode {
194+
case .none:
195+
if let callback = self.scanResultCallback {
196+
callback(device, advertisementData, RSSI)
197+
}
198+
case .alert:
199+
DispatchQueue.main.async { [weak self] in
200+
self?.alertController?.addAction(UIAlertAction(title: device.getName() ?? "Unknown", style: UIAlertAction.Style.default, handler: { (_) in
201+
log("Selected device")
202+
self?.stopScan()
203+
self?.resolve("startScanning", device.getId())
204+
}))
205+
}
206+
case .list:
207+
DispatchQueue.main.async { [weak self] in
208+
self?.deviceListView?.addItem(device.getName() ?? "Unknown", action: {
209+
log("Selected device")
210+
self?.stopScan()
211+
self?.resolve("startScanning", device.getId())
212+
})
213+
}
214214
}
215215
}
216216
}
@@ -297,8 +297,8 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
297297
error: Error?
298298
) {
299299
let key = "connect|\(peripheral.identifier.uuidString)"
300-
if error != nil {
301-
self.reject(key, error!.localizedDescription)
300+
if let error = error {
301+
self.reject(key, error.localizedDescription)
302302
return
303303
}
304304
self.reject(key, "Failed to connect.")
@@ -337,9 +337,9 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
337337
let key = "disconnect|\(peripheral.identifier.uuidString)"
338338
let keyOnDisconnected = "onDisconnected|\(peripheral.identifier.uuidString)"
339339
self.resolve(keyOnDisconnected, "Disconnected.")
340-
if error != nil {
341-
log(error!.localizedDescription)
342-
self.reject(key, error!.localizedDescription)
340+
if let error = error {
341+
log(error.localizedDescription)
342+
self.reject(key, error.localizedDescription)
343343
return
344344
}
345345
self.resolve(key, "Successfully disconnected.")
@@ -363,25 +363,17 @@ class DeviceManager: NSObject, CBCentralManagerDelegate {
363363

364364

365365
private func resolve(_ key: String, _ value: String) {
366-
let callback = self.callbackMap[key]
367-
if callback != nil {
368-
log("Resolve", key, value)
369-
callback!(true, value)
370-
self.callbackMap[key] = nil
371-
self.timeoutMap[key]?.cancel()
372-
self.timeoutMap[key] = nil
373-
}
366+
guard let callback = self.callbackMap.removeValue(forKey: key) else { return }
367+
self.timeoutMap.removeValue(forKey: key)?.cancel()
368+
log("Resolve", key, value)
369+
callback(true, value)
374370
}
375371

376372
private func reject(_ key: String, _ value: String) {
377-
let callback = self.callbackMap[key]
378-
if callback != nil {
379-
log("Reject", key, value)
380-
callback!(false, value)
381-
self.callbackMap[key] = nil
382-
self.timeoutMap[key]?.cancel()
383-
self.timeoutMap[key] = nil
384-
}
373+
guard let callback = self.callbackMap.removeValue(forKey: key) else { return }
374+
self.timeoutMap.removeValue(forKey: key)?.cancel()
375+
log("Reject", key, value)
376+
callback(false, value)
385377
}
386378

387379
private func setTimeout(

0 commit comments

Comments
 (0)