From ddd4f922b94afa6c037a3c16a4e6bb9e2c2ccae8 Mon Sep 17 00:00:00 2001 From: Guogang Li Date: Wed, 6 Aug 2025 22:45:54 -0700 Subject: [PATCH] Fixed deadlock during stop scanning on iOS PiperOrigin-RevId: 791998760 --- internal/platform/implementation/apple/BUILD | 39 +++++++++++++++---- .../apple/Mediums/BLEv2/GNCBLEMedium.h | 19 +++++++-- .../apple/Mediums/BLEv2/GNCBLEMedium.m | 30 +++++--------- .../platform/implementation/apple/Tests/BUILD | 1 + .../implementation/apple/ble_medium.h | 4 ++ .../implementation/apple/ble_medium.mm | 30 ++++++++++++-- 6 files changed, 88 insertions(+), 35 deletions(-) diff --git a/internal/platform/implementation/apple/BUILD b/internal/platform/implementation/apple/BUILD index 2b369191..bd3316ad 100644 --- a/internal/platform/implementation/apple/BUILD +++ b/internal/platform/implementation/apple/BUILD @@ -1,7 +1,3 @@ -load("@rules_cc//cc:cc_library.bzl", "cc_library") -load("@rules_cc//cc:cc_test.bzl", "cc_test") -load("@rules_cc//cc:objc_library.bzl", "objc_library") - # Copyright 2020 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -15,6 +11,11 @@ load("@rules_cc//cc:objc_library.bzl", "objc_library") # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_cc//cc:cc_test.bzl", "cc_test") +load("@rules_cc//cc:objc_library.bzl", "objc_library") + licenses(["notice"]) package(default_visibility = [ @@ -25,13 +26,35 @@ package(default_visibility = [ "//sharing:__subpackages__", ]) +objc_library( + name = "comm", + srcs = [ + "multi_thread_executor.mm", + ], + hdrs = [ + "multi_thread_executor.h", + "scheduled_executor.h", + "single_thread_executor.h", + ], + deps = [ + "//internal/platform:base", + "//internal/platform:cancellation_flag", + "//internal/platform:uuid", + "//internal/platform/implementation:comm", + "//internal/platform/implementation:types", + "//internal/platform/implementation/apple/Flags", + "//internal/platform/implementation/apple/Log:GNCLogger", + "//third_party/apple_frameworks:CoreBluetooth", + "//third_party/apple_frameworks:Foundation", + ], +) + objc_library( name = "apple", srcs = [ "awdl.mm", "crypto.mm", "device_info.mm", - "multi_thread_executor.mm", "platform.mm", "preferences_manager.mm", "scheduled_executor.mm", @@ -42,10 +65,7 @@ objc_library( hdrs = [ "awdl.h", "device_info.h", - "multi_thread_executor.h", "preferences_manager.h", - "scheduled_executor.h", - "single_thread_executor.h", "timer.h", "wifi.h", "wifi_hotspot.h", @@ -55,6 +75,7 @@ objc_library( aspect_hints = ["//tools/build_defs/swift:no_module"], features = ["-layering_check"], deps = [ + ":comm", ":Platform_cc", ":Shared", ":ble_v2", @@ -119,10 +140,12 @@ objc_library( # Prevent Objective-C++ headers from being pulled into swift. aspect_hints = ["//tools/build_defs/swift:no_module"], deps = [ + ":comm", "//internal/platform:base", "//internal/platform:cancellation_flag", "//internal/platform:uuid", "//internal/platform/implementation:comm", + "//internal/platform/implementation:types", "//internal/platform/implementation/apple/Flags", "//internal/platform/implementation/apple/Log:GNCLogger", "//internal/platform/implementation/apple/Mediums", diff --git a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.h b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.h index 86460b02..b95790a9 100644 --- a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.h +++ b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.h @@ -17,7 +17,7 @@ #import "internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPClient.h" #import "internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPServer.h" -// #import "internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEL2CAPStream.h" +#import "internal/platform/implementation/apple/Mediums/BLEv2/GNCCentralManager.h" @class GNCBLEGATTServer; @class GNCBLEGATTClient; @@ -101,6 +101,20 @@ typedef void (^GNCGATTConnectionCompletionHandler)(GNCBLEGATTClient *_Nullable c */ @interface GNCBLEMedium : NSObject +/** + * Initializes the BLE medium. + */ +- (instancetype)init; + +/** + * Initializes the BLE medium with a custom central manager. + * + * @param centralManager The central manager to use for BLE operations. + * @param queue The queue to use for all internal operations. + */ +- (instancetype)initWithCentralManager:(id)centralManager + queue:(nullable dispatch_queue_t)queue; + /** The hardware supports BOTH advertising extensions and extended scans. */ @property(nonatomic, readonly) BOOL supportsExtendedAdvertisements; @@ -171,8 +185,7 @@ typedef void (^GNCGATTConnectionCompletionHandler)(GNCBLEGATTClient *_Nullable c * @param completionHandler Called on a private queue with @c nil if successfully resumed scanning * or an error if one has occurred. */ -- (void)resumeMediumScanning: - (nullable GNCStartScanningCompletionHandler)completionHandler; +- (void)resumeMediumScanning:(nullable GNCStartScanningCompletionHandler)completionHandler; /** * Starts a GATT server. diff --git a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.m b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.m index c3e6b9de..283620d2 100644 --- a/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.m +++ b/internal/platform/implementation/apple/Mediums/BLEv2/GNCBLEMedium.m @@ -96,11 +96,11 @@ static GNCBLEL2CAPServer *_Nonnull CreateL2CapServer( } - (instancetype)init { + dispatch_queue_t queue = dispatch_queue_create(kBLEMediumQueueLabel, DISPATCH_QUEUE_SERIAL); CBCentralManager *centralManager = [[CBCentralManager alloc] initWithDelegate:self - queue:_queue + queue:queue options:@{CBCentralManagerOptionShowPowerAlertKey : @NO}]; - dispatch_queue_t queue = dispatch_queue_create(kBLEMediumQueueLabel, DISPATCH_QUEUE_SERIAL); return [self initWithCentralManager:centralManager queue:queue]; } @@ -506,43 +506,33 @@ static GNCBLEL2CAPServer *_Nonnull CreateL2CapServer( #pragma mark - CBCentralManagerDelegate - (void)centralManagerDidUpdateState:(CBCentralManager *)central { - dispatch_async(_queue, ^{ - [self gnc_centralManagerDidUpdateState:central]; - }); + [self gnc_centralManagerDidUpdateState:central]; } - (void)centralManager:(CBCentralManager *)central didDiscoverPeripheral:(CBPeripheral *)peripheral advertisementData:(NSDictionary *)advertisementData RSSI:(NSNumber *)RSSI { - dispatch_async(_queue, ^{ - [self gnc_centralManager:central - didDiscoverPeripheral:peripheral - advertisementData:advertisementData - RSSI:RSSI]; - }); + [self gnc_centralManager:central + didDiscoverPeripheral:peripheral + advertisementData:advertisementData + RSSI:RSSI]; } - (void)centralManager:(CBCentralManager *)central didConnectPeripheral:(CBPeripheral *)peripheral { - dispatch_async(_queue, ^{ - [self gnc_centralManager:central didConnectPeripheral:peripheral]; - }); + [self gnc_centralManager:central didConnectPeripheral:peripheral]; } - (void)centralManager:(CBCentralManager *)central didFailToConnectPeripheral:(CBPeripheral *)peripheral error:(nullable NSError *)error { - dispatch_async(_queue, ^{ - [self gnc_centralManager:central didFailToConnectPeripheral:peripheral error:error]; - }); + [self gnc_centralManager:central didFailToConnectPeripheral:peripheral error:error]; } - (void)centralManager:(CBCentralManager *)central didDisconnectPeripheral:(CBPeripheral *)peripheral error:(nullable NSError *)error { - dispatch_async(_queue, ^{ - [self gnc_centralManager:central didDisconnectPeripheral:peripheral error:error]; - }); + [self gnc_centralManager:central didDisconnectPeripheral:peripheral error:error]; } @end diff --git a/internal/platform/implementation/apple/Tests/BUILD b/internal/platform/implementation/apple/Tests/BUILD index 95df676c..22c6e1b0 100644 --- a/internal/platform/implementation/apple/Tests/BUILD +++ b/internal/platform/implementation/apple/Tests/BUILD @@ -65,6 +65,7 @@ objc_library( "//internal/platform/implementation/apple", # buildcleaner: keep "//internal/platform/implementation/apple:Shared", "//internal/platform/implementation/apple:ble_v2", + "//internal/platform/implementation/apple:comm", "//internal/platform/implementation/apple/Mediums", "//third_party/apple_frameworks:CommonCrypto", "//third_party/apple_frameworks:CoreBluetooth", diff --git a/internal/platform/implementation/apple/ble_medium.h b/internal/platform/implementation/apple/ble_medium.h index ec12c5d7..6520221e 100644 --- a/internal/platform/implementation/apple/ble_medium.h +++ b/internal/platform/implementation/apple/ble_medium.h @@ -29,6 +29,7 @@ #import "internal/platform/implementation/apple/ble_l2cap_server_socket.h" #import "internal/platform/implementation/apple/ble_server_socket.h" #import "internal/platform/implementation/apple/bluetooth_adapter_v2.h" +#import "internal/platform/implementation/apple/single_thread_executor.h" #include "internal/platform/implementation/ble_v2.h" #include "internal/platform/implementation/bluetooth_adapter.h" #include "internal/platform/uuid.h" @@ -202,6 +203,9 @@ class BleMedium : public api::ble_v2::BleMedium { NSDictionary *service_data); NSDate *GetLastTimestampToCleanExpiredAdvertisementPackets(); + // The executor for handling callbacks. + apple::SingleThreadExecutor callback_executor_; + GNCBLEMedium *medium_; PeripheralsMap peripherals_; diff --git a/internal/platform/implementation/apple/ble_medium.mm b/internal/platform/implementation/apple/ble_medium.mm index d2895819..96071ed8 100644 --- a/internal/platform/implementation/apple/ble_medium.mm +++ b/internal/platform/implementation/apple/ble_medium.mm @@ -200,7 +200,8 @@ std::unique_ptr BleMedium::StartScannin [medium_ startScanningForService:serviceUUID advertisementFoundHandler:^(id peripheral, NSDictionary *serviceData) { - HandleAdvertisementFound(peripheral, serviceData); + callback_executor_.Execute( + [this, peripheral, serviceData] { HandleAdvertisementFound(peripheral, serviceData); }); } completionHandler:^(NSError *error) { if (scanning_cb_.start_scanning_result) { @@ -234,7 +235,8 @@ bool BleMedium::StartScanning(const Uuid &service_uuid, api::ble_v2::TxPowerLeve [medium_ startScanningForService:serviceUUID advertisementFoundHandler:^(id peripheral, NSDictionary *serviceData) { - HandleAdvertisementFound(peripheral, serviceData); + callback_executor_.Execute( + [this, peripheral, serviceData] { HandleAdvertisementFound(peripheral, serviceData); }); } completionHandler:^(NSError *error) { if (error != nil) { @@ -276,7 +278,8 @@ bool BleMedium::StartMultipleServicesScanning(const std::vector &service_u [medium_ startScanningForMultipleServices:serviceUUIDs advertisementFoundHandler:^(id peripheral, NSDictionary *serviceData) { - HandleAdvertisementFound(peripheral, serviceData); + callback_executor_.Execute( + [this, peripheral, serviceData] { HandleAdvertisementFound(peripheral, serviceData); }); } completionHandler:^(NSError *error) { if (error != nil) { @@ -349,7 +352,26 @@ std::unique_ptr BleMedium::ConnectToGattServer( return nullptr; } - __block api::ble_v2::ClientGattConnectionCallback blockCallback = std::move(callback); + api::ble_v2::ClientGattConnectionCallback thread_callback = { + .on_characteristic_changed_cb = + [this, + call_on_characteristic_changed_cb = std::move(callback.on_characteristic_changed_cb)]( + absl::string_view characteristic_uuid) mutable { + callback_executor_.Execute([characteristic_uuid = std::string(characteristic_uuid), + call_on_characteristic_changed_cb = std::move( + call_on_characteristic_changed_cb)]() mutable { + call_on_characteristic_changed_cb(characteristic_uuid); + }); + }, + .disconnected_cb = + [this, call_disconnected_cb = std::move(callback.disconnected_cb)]() mutable { + callback_executor_.Execute( + [call_disconnected_cb = std::move(call_disconnected_cb)]() mutable { + call_disconnected_cb(); + }); + }}; + + __block api::ble_v2::ClientGattConnectionCallback blockCallback = std::move(thread_callback); dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); __block GNCBLEGATTClient *blockClient = nil;