Internal crash fix

PiperOrigin-RevId: 651635619

internal/platform/implementation/windows/ble_gatt_client.cc

@@ -37,6 +37,7 @@
 #include "internal/platform/byte_array.h"
 #include "internal/platform/flags/nearby_platform_feature_flags.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/implementation/windows/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/utils.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/uuid.h"
@@ -101,28 +102,33 @@ std::string GattCommunicationStatusToString(GattCommunicationStatus status) {
 
 BleGattClient::BleGattClient(BluetoothLEDevice ble_device)
     : ble_device_(ble_device) {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": GATT client is created.";
+  if (ble_device_ == nullptr) {
+    NEARBY_LOGS(WARNING) << __func__ << ": ble_device is null.";
+  } else {
+    NEARBY_LOGS(INFO) << __func__ << ": GATT client is created, address: "
+                      << uint64_to_mac_address_string(
+                             ble_device_.BluetoothAddress());
+  }
 }
 
 BleGattClient::~BleGattClient() {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": GATT client is released.";
+  NEARBY_LOGS(INFO) << __func__ << ": GATT client is released.";
   Disconnect();
 }
 
 bool BleGattClient::DiscoverServiceAndCharacteristics(
     const Uuid& service_uuid, const std::vector<Uuid>& characteristic_uuids) {
+  absl::MutexLock lock(&mutex_);
   if (!NearbyFlags::GetInstance().GetBoolFlag(
           platform::config_package_nearby::nearby_platform_feature::
               kEnableBleV2Gatt)) {
-    auto windows_bluetooth_adapter_ = ::winrt::Windows::Devices::Bluetooth::
-                                          BluetoothAdapter::GetDefaultAsync()
-                                              .get();
-    if (windows_bluetooth_adapter_.IsExtendedAdvertisingSupported()) {
+    BluetoothAdapter bluetooth_adapter;
+    if (bluetooth_adapter.IsExtendedAdvertisingSupported()) {
       NEARBY_LOGS(WARNING) << __func__ << ": GATT is disabled.";
       return false;
     }
 
-    if (!windows_bluetooth_adapter_.IsCentralRoleSupported()) {
+    if (!bluetooth_adapter.IsCentralRoleSupported()) {
       NEARBY_LOGS(ERROR) << __func__
                          << ": Bluetooth Hardware does not support Central "
                             "Role, which is required to start GATT client.";
@@ -192,9 +198,8 @@ bool BleGattClient::DiscoverServiceAndCharacteristics(
                           winrt::to_string(winrt::to_hstring(service.Uuid())));
         });
 
-    NEARBY_LOGS(VERBOSE) << __func__
-                         << ": Found GATT services=" << flat_services
-                         << " from BLE device.";
+    NEARBY_LOGS(INFO) << __func__ << ": Found GATT services=" << flat_services
+                      << " from BLE device.";
 
     // Needs to check each service to make sure it includes all characteristic
     // uuids. Services may include duplicate service UUID, but each of them may
@@ -269,8 +274,8 @@ bool BleGattClient::DiscoverServiceAndCharacteristics(
       return true;
     }
 
-    NEARBY_LOGS(VERBOSE) << __func__
-                         << ": Failed to find service and all characteristics.";
+    NEARBY_LOGS(ERROR) << __func__
+                       << ": Failed to find service and all characteristics.";
   } catch (std::exception exception) {
     NEARBY_LOGS(ERROR) << __func__
                        << ": Failed to get GATT services. exception: "
@@ -288,10 +293,10 @@ bool BleGattClient::DiscoverServiceAndCharacteristics(
 absl::optional<api::ble_v2::GattCharacteristic>
 BleGattClient::GetCharacteristic(const Uuid& service_uuid,
                                  const Uuid& characteristic_uuid) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(VERBOSE) << __func__ << ": Stared to get characteristic UUID="
                        << std::string(characteristic_uuid)
                        << " in service UUID=" << std::string(service_uuid);
-  absl::MutexLock lock(&mutex_);
   try {
     std::optional<GattCharacteristic> gatt_characteristic =
         GetNativeCharacteristic(service_uuid, characteristic_uuid);
@@ -355,6 +360,7 @@ BleGattClient::GetCharacteristic(const Uuid& service_uuid,
 
 absl::optional<std::string> BleGattClient::ReadCharacteristic(
     const api::ble_v2::GattCharacteristic& characteristic) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(VERBOSE) << __func__ << ": Read characteristic="
                        << std::string(characteristic.uuid);
   try {
@@ -411,9 +417,9 @@ absl::optional<std::string> BleGattClient::ReadCharacteristic(
 bool BleGattClient::WriteCharacteristic(
     const api::ble_v2::GattCharacteristic& characteristic,
     absl::string_view value, api::ble_v2::GattClient::WriteType write_type) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(VERBOSE) << __func__ << ": write characteristic: "
                        << std::string(characteristic.uuid);
-  absl::MutexLock lock(&mutex_);
   try {
     std::optional<GattCharacteristic> gatt_characteristic =
         native_characteristic_map_[characteristic].native_characteristic;
@@ -466,6 +472,7 @@ bool BleGattClient::SetCharacteristicSubscription(
     const api::ble_v2::GattCharacteristic& characteristic, bool enable,
     absl::AnyInvocable<void(absl::string_view value)>
         on_characteristic_changed_cb) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(VERBOSE) << __func__
                        << ": Started to set Characteristic Subscription.";
   GattClientCharacteristicConfigurationDescriptorValue gcccd_value =
@@ -484,11 +491,9 @@ bool BleGattClient::SetCharacteristicSubscription(
   }
 
   std::optional<GattCharacteristic> gatt_characteristic;
-  {
-    absl::MutexLock lock(&mutex_);
-    gatt_characteristic =
-        native_characteristic_map_[characteristic].native_characteristic;
-  }
+
+  gatt_characteristic =
+      native_characteristic_map_[characteristic].native_characteristic;
 
   if (!gatt_characteristic.has_value()) {
     NEARBY_LOGS(ERROR) << __func__
@@ -505,7 +510,6 @@ bool BleGattClient::SetCharacteristicSubscription(
     return false;
   }
 
-  absl::MutexLock lock(&mutex_);
   // Set value changed handler
   try {
     if (enable) {
@@ -643,10 +647,10 @@ bool BleGattClient::WriteCharacteristicConfigurationDescriptor(
                               "configuration descriptor";
       return true;
     }
-    NEARBY_LOGS(VERBOSE) << __func__
-                         << ": Failed to write client characteristic "
-                            "configuration descriptor with error: "
-                         << GattCommunicationStatusToString(status);
+    NEARBY_LOGS(ERROR) << __func__
+                       << ": Failed to write client characteristic "
+                          "configuration descriptor with error: "
+                       << GattCommunicationStatusToString(status);
   } catch (std::exception exception) {
     // This usually happens when a device reports that it support notify, but
     // it actually doesn't.

internal/platform/implementation/windows/ble_gatt_client.h

@@ -23,10 +23,15 @@
 #include <string>
 #include <vector>
 
+#include "absl/base/thread_annotations.h"
 #include "absl/container/flat_hash_map.h"
+#include "absl/functional/any_invocable.h"
+#include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
+#include "absl/types/optional.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/uuid.h"
 #include "winrt/Windows.Devices.Bluetooth.GenericAttributeProfile.h"
 #include "winrt/Windows.Devices.Bluetooth.h"
 #include "winrt/base.h"
@@ -42,25 +47,29 @@ class BleGattClient : public api::ble_v2::GattClient {
 
   bool DiscoverServiceAndCharacteristics(
       const Uuid& service_uuid,
-      const std::vector<Uuid>& characteristic_uuids) override;
+      const std::vector<Uuid>& characteristic_uuids) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   absl::optional<api::ble_v2::GattCharacteristic> GetCharacteristic(
-      const Uuid& service_uuid, const Uuid& characteristic_uuid) override;
+      const Uuid& service_uuid, const Uuid& characteristic_uuid) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   absl::optional<std::string> ReadCharacteristic(
-      const api::ble_v2::GattCharacteristic& characteristic) override;
+      const api::ble_v2::GattCharacteristic& characteristic) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   bool WriteCharacteristic(
       const api::ble_v2::GattCharacteristic& characteristic,
       absl::string_view value,
-      api::ble_v2::GattClient::WriteType write_type) override;
+      api::ble_v2::GattClient::WriteType write_type) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   bool SetCharacteristicSubscription(
       const api::ble_v2::GattCharacteristic& characteristic, bool enable,
       absl::AnyInvocable<void(absl::string_view value)>
-          on_characteristic_changed_cb) override;
+          on_characteristic_changed_cb) override ABSL_LOCKS_EXCLUDED(mutex_);
 
-  void Disconnect() override;
+  void Disconnect() override ABSL_LOCKS_EXCLUDED(mutex_);
 
  private:
   // Used to save native data related to the GATT characteristic.
@@ -75,13 +84,15 @@ class BleGattClient : public api::ble_v2::GattClient {
   std::optional<::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
                     GattCharacteristic>
   GetNativeCharacteristic(const Uuid& service_uuid,
-                          const Uuid& characteristic_uuid);
+                          const Uuid& characteristic_uuid)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   bool WriteCharacteristicConfigurationDescriptor(
       ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
           GattCharacteristic& characteristic,
       ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
-          GattClientCharacteristicConfigurationDescriptorValue value);
+          GattClientCharacteristicConfigurationDescriptorValue value)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   void OnCharacteristicValueChanged(
       const api::ble_v2::GattCharacteristic& characteristic,
@@ -90,9 +101,11 @@ class BleGattClient : public api::ble_v2::GattClient {
 
   absl::Mutex mutex_;
 
-  ::winrt::Windows::Devices::Bluetooth::BluetoothLEDevice ble_device_;
+  ::winrt::Windows::Devices::Bluetooth::BluetoothLEDevice ble_device_
+      ABSL_GUARDED_BY(mutex_);
   ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
-      GattDeviceServicesResult gatt_devices_services_result_ = nullptr;
+      GattDeviceServicesResult gatt_devices_services_result_
+          ABSL_GUARDED_BY(mutex_) = nullptr;
 
   absl::flat_hash_map<api::ble_v2::GattCharacteristic, GattCharacteristicData>
       native_characteristic_map_ ABSL_GUARDED_BY(mutex_);

internal/platform/implementation/windows/ble_gatt_server.cc

@@ -15,6 +15,7 @@
 #include "internal/platform/implementation/windows/ble_gatt_server.h"
 
 #include <cstddef>
+#include <cstdint>
 #include <cstring>
 #include <exception>
 #include <memory>
@@ -29,11 +30,17 @@
 #include "absl/status/status.h"
 #include "absl/strings/escaping.h"
 #include "absl/strings/str_format.h"
+#include "absl/synchronization/mutex.h"
+#include "absl/time/clock.h"
+#include "absl/time/time.h"
 #include "absl/types/optional.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/implementation/bluetooth_adapter.h"
+#include "internal/platform/implementation/windows/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/utils.h"
 #include "internal/platform/logging.h"
+#include "internal/platform/uuid.h"
 #include "winrt/Windows.Foundation.Collections.h"
 #include "winrt/Windows.Storage.Streams.h"
 #include "winrt/base.h"
@@ -81,6 +88,9 @@ using ::winrt::Windows::Storage::Streams::DataWriter;
 using Permission = api::ble_v2::GattCharacteristic::Permission;
 using Property = api::ble_v2::GattCharacteristic::Property;
 
+constexpr absl::Duration kGattServerTimeout = absl::Milliseconds(500);
+constexpr int kGattServerCheckIntervalInMills = 50;
+
 std::string ConvertGattStatusToString(
     GattServiceProviderAdvertisementStatus status) {
   switch (status) {
@@ -113,9 +123,10 @@ BleGattServer::CreateCharacteristic(
     const Uuid& service_uuid, const Uuid& characteristic_uuid,
     api::ble_v2::GattCharacteristic::Permission permission,
     api::ble_v2::GattCharacteristic::Property property) {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": create characteristic, service_uuid: "
-                       << std::string(service_uuid) << ", characteristic_uuid: "
-                       << std::string(characteristic_uuid);
+  absl::MutexLock lock(&mutex_);
+  NEARBY_LOGS(INFO) << __func__ << ": create characteristic, service_uuid: "
+                    << std::string(service_uuid) << ", characteristic_uuid: "
+                    << std::string(characteristic_uuid);
 
   if (!service_uuid_.IsEmpty() && service_uuid_ != service_uuid) {
     NEARBY_LOGS(ERROR) << __func__
@@ -142,8 +153,9 @@ BleGattServer::CreateCharacteristic(
 bool BleGattServer::UpdateCharacteristic(
     const api::ble_v2::GattCharacteristic& characteristic,
     const nearby::ByteArray& value) {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": update characteristic: "
-                       << std::string(characteristic.uuid);
+  absl::MutexLock lock(&mutex_);
+  NEARBY_LOGS(INFO) << __func__ << ": update characteristic: "
+                    << std::string(characteristic.uuid);
 
   if (characteristic.service_uuid != service_uuid_) {
     NEARBY_LOGS(ERROR) << __func__ << ": Cannot found the GATT service.";
@@ -184,6 +196,7 @@ bool BleGattServer::UpdateCharacteristic(
 absl::Status BleGattServer::NotifyCharacteristicChanged(
     const api::ble_v2::GattCharacteristic& characteristic, bool confirm,
     const ByteArray& new_value) {
+  absl::MutexLock lock(&mutex_);
   // Currently, the method is not hooked up at platform layer.
   NEARBY_LOGS(VERBOSE) << __func__ << ": Notify characteristic="
                        << std::string(characteristic.uuid) << " changed.";
@@ -191,6 +204,7 @@ absl::Status BleGattServer::NotifyCharacteristicChanged(
 }
 
 void BleGattServer::Stop() {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(VERBOSE) << __func__ << ": Start to stop GATT server.";
   try {
     if (gatt_service_provider_ == nullptr) {
@@ -373,6 +387,8 @@ bool BleGattServer::InitializeGattServer() {
 
 bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
                                        bool is_connectable) {
+  absl::MutexLock lock(&mutex_);
+
   try {
     NEARBY_LOGS(VERBOSE) << __func__ << ": service_data="
                          << absl::BytesToHexString(service_data.AsStringView())
@@ -385,13 +401,11 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
 
     if (!is_gatt_server_inited_ && !InitializeGattServer()) {
       NEARBY_LOGS(ERROR) << ":Failed to initalize GATT service.";
-      is_advertising_ = false;
       return false;
     }
 
     if (gatt_service_provider_ == nullptr) {
       NEARBY_LOGS(WARNING) << __func__ << ": no GATT server is running.";
-      is_advertising_ = false;
       return false;
     }
 
@@ -399,7 +413,6 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
         GattServiceProviderAdvertisementStatus::Started) {
       NEARBY_LOGS(WARNING) << __func__
                            << ": GATT server is already in advertising.";
-      is_advertising_ = true;
       return false;
     }
 
@@ -408,12 +421,9 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
           << __func__
           << ": Bluetooth Hardware does not support Peripheral Role, which is "
              "required to start GATT server.";
-      is_advertising_ = false;
       return false;
     }
 
-    is_advertising_ = true;
-
     // Start the GATT server advertising
     GattServiceProviderAdvertisingParameters advertisement_parameters;
     advertisement_parameters.IsConnectable(is_connectable);
@@ -427,6 +437,21 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
     advertisement_parameters.ServiceData(data_writer.DetachBuffer());
 
     gatt_service_provider_.StartAdvertising(advertisement_parameters);
+
+    // Wait for the advertising to start.
+    int wait_milliseconds = 0;
+    while (gatt_service_provider_.AdvertisementStatus() !=
+           GattServiceProviderAdvertisementStatus::Started) {
+      absl::SleepFor(absl::Milliseconds(kGattServerCheckIntervalInMills));
+      wait_milliseconds += kGattServerCheckIntervalInMills;
+      if (absl::Milliseconds(wait_milliseconds) > kGattServerTimeout) {
+        NEARBY_LOGS(ERROR)
+            << __func__ << ": Failed to start GATT advertising due to timeout.";
+        return false;
+      }
+    }
+
+    is_advertising_ = true;
     NEARBY_LOGS(INFO) << __func__ << ": GATT server started.";
 
     return true;
@@ -445,6 +470,8 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
 }
 
 bool BleGattServer::StopAdvertisement() {
+  absl::MutexLock lock(&mutex_);
+
   try {
     NEARBY_LOGS(INFO) << __func__ << ": stop advertisement.";
 
@@ -467,6 +494,11 @@ bool BleGattServer::StopAdvertisement() {
     }
 
     gatt_service_provider_.StopAdvertising();
+
+    // Don't wait for the advertising to stop, because the advertisement status
+    // cannot back to stopped. Based on the observation, the advertisement
+    // status is stopped after the stop advertising is called.
+
     is_advertising_ = false;
     NEARBY_LOGS(INFO) << __func__ << ": GATT server stopped.";
     return true;
@@ -487,9 +519,9 @@ bool BleGattServer::StopAdvertisement() {
         GattLocalCharacteristic const& gatt_local_characteristic,
     ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
         GattReadRequestedEventArgs args) {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": Read characteristic. uuid: "
-                       << winrt::to_string(winrt::to_hstring(
-                              gatt_local_characteristic.Uuid()));
+  NEARBY_LOGS(INFO) << __func__ << ": Read characteristic. uuid: "
+                    << winrt::to_string(
+                           winrt::to_hstring(gatt_local_characteristic.Uuid()));
 
   auto deferral = args.GetDeferral();
 
@@ -542,7 +574,7 @@ bool BleGattServer::StopAdvertisement() {
         GattLocalCharacteristic const& gatt_local_characteristic,
     ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
         GattWriteRequestedEventArgs args) {
-  // In Nearby Connctions, don't support write charaterisctics right now.
+  // In Nearby Connections, don't support write characteristics right now.
   throw std::logic_error("Not implemented.");
 }
 
@@ -550,10 +582,10 @@ void BleGattServer::Characteristic_SubscribedClientsChanged(
     ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
         GattLocalCharacteristic const& gatt_local_characteristic,
     ::winrt::Windows::Foundation::IInspectable const& args) {
-  NEARBY_LOGS(VERBOSE) << __func__
-                       << ": Subscribed clients changed. characteristic="
-                       << ::winrt::to_string(::winrt::to_hstring(
-                              gatt_local_characteristic.Uuid()));
+  NEARBY_LOGS(INFO) << __func__
+                    << ": Subscribed clients changed. characteristic="
+                    << ::winrt::to_string(::winrt::to_hstring(
+                           gatt_local_characteristic.Uuid()));
 
   try {
     std::vector<api::ble_v2::GattCharacteristic>
@@ -637,8 +669,9 @@ void BleGattServer::ServiceProvider_AdvertisementStatusChanged(
         GattServiceProvider const& sender,
     ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
         GattServiceProviderAdvertisementStatusChangedEventArgs const& args) {
-  NEARBY_LOGS(VERBOSE) << __func__ << ": Advertisement status changed. status="
-                       << ConvertGattStatusToString(args.Status());
+  NEARBY_LOGS(INFO) << __func__ << ": Advertisement status changed. status="
+                    << ConvertGattStatusToString(args.Status())
+                    << ", error=" << static_cast<int>(args.Error());
 }
 
 void BleGattServer::NotifyValueChanged(

internal/platform/implementation/windows/ble_gatt_server.h

@@ -17,13 +17,19 @@
 
 #include <windows.h>
 
-#include <optional>
+#include <memory>
 #include <vector>
 
+#include "absl/base/thread_annotations.h"
 #include "absl/container/flat_hash_map.h"
+#include "absl/status/status.h"
 #include "absl/strings/string_view.h"
+#include "absl/synchronization/mutex.h"
+#include "absl/synchronization/notification.h"
+#include "absl/types/optional.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/implementation/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/ble_v2_peripheral.h"
 #include "internal/platform/implementation/windows/bluetooth_adapter.h"
 #include "internal/platform/uuid.h"
@@ -43,20 +49,22 @@ class BleGattServer : public api::ble_v2::GattServer {
   absl::optional<api::ble_v2::GattCharacteristic> CreateCharacteristic(
       const Uuid& service_uuid, const Uuid& characteristic_uuid,
       api::ble_v2::GattCharacteristic::Permission permission,
-      api::ble_v2::GattCharacteristic::Property property) override;
+      api::ble_v2::GattCharacteristic::Property property) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   bool UpdateCharacteristic(
       const api::ble_v2::GattCharacteristic& characteristic,
-      const nearby::ByteArray& value) override;
+      const nearby::ByteArray& value) override ABSL_LOCKS_EXCLUDED(mutex_);
 
   absl::Status NotifyCharacteristicChanged(
       const api::ble_v2::GattCharacteristic& characteristic, bool confirm,
-      const ByteArray& new_value) override;
+      const ByteArray& new_value) override ABSL_LOCKS_EXCLUDED(mutex_);
 
-  void Stop() override;
+  void Stop() override ABSL_LOCKS_EXCLUDED(mutex_);
 
-  bool StartAdvertisement(const ByteArray& service_data, bool is_connectable);
-  bool StopAdvertisement();
+  bool StartAdvertisement(const ByteArray& service_data, bool is_connectable)
+      ABSL_LOCKS_EXCLUDED(mutex_);
+  bool StopAdvertisement() ABSL_LOCKS_EXCLUDED(mutex_);
 
   api::ble_v2::BlePeripheral& GetBlePeripheral() override {
     return peripheral_;
@@ -109,6 +117,8 @@ class BleGattServer : public api::ble_v2::GattServer {
       ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
           GattServiceProviderAdvertisementStatusChangedEventArgs const& args);
 
+  absl::Mutex mutex_;
+
   BluetoothAdapter* adapter_ = nullptr;
   BleV2Peripheral peripheral_;
 

internal/platform/implementation/windows/ble_v2.cc

@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <array>
+#include <cstdint>
 #include <exception>
 #include <iostream>
 #include <memory>
@@ -29,18 +30,21 @@
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
 #include "absl/synchronization/mutex.h"
+#include "absl/time/clock.h"
 #include "absl/time/time.h"
 #include "internal/flags/nearby_flags.h"
-#include "internal/platform/bluetooth_adapter.h"
 #include "internal/platform/byte_array.h"
 #include "internal/platform/cancellation_flag.h"
 #include "internal/platform/cancellation_flag_listener.h"
 #include "internal/platform/flags/nearby_platform_feature_flags.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/implementation/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/ble_gatt_client.h"
 #include "internal/platform/implementation/windows/ble_gatt_server.h"
+#include "internal/platform/implementation/windows/ble_v2_peripheral.h"
 #include "internal/platform/implementation/windows/ble_v2_server_socket.h"
 #include "internal/platform/implementation/windows/ble_v2_socket.h"
+#include "internal/platform/implementation/windows/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/utils.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/prng.h"
@@ -116,6 +120,9 @@ static constexpr uint64_t kGenerateSessionIdRetryLimit = 3;
 // Indicating failed to generate unused session id.
 static constexpr uint64_t kFailedGenerateSessionId = 0;
 
+constexpr absl::Duration kMediumTimeout = absl::Milliseconds(500);
+constexpr int kMediumCheckIntervalInMills = 50;
+
 // Remove lost/unused peripherals after a timeout.
 constexpr absl::Duration kPeripheralExpiryTime = absl::Minutes(15);
 // Prevent too frequent cleanup tasks.
@@ -128,6 +135,7 @@ BleV2Medium::BleV2Medium(api::BluetoothAdapter& adapter)
 // Advertisement packet and populate accordingly.
 bool BleV2Medium::StartAdvertising(const BleAdvertisementData& advertising_data,
                                    AdvertiseParameters advertising_parameters) {
+  absl::MutexLock lock(&mutex_);
   std::string service_data_info;
   for (const auto& it : advertising_data.service_data) {
     service_data_info += "{uuid:" + std::string(it.first) +
@@ -160,6 +168,7 @@ bool BleV2Medium::StartAdvertising(const BleAdvertisementData& advertising_data,
 }
 
 bool BleV2Medium::StopAdvertising() {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(INFO) << __func__ << ": Stop advertising.";
   bool result;
   if (is_gatt_publisher_started_) {
@@ -222,6 +231,7 @@ std::unique_ptr<BleV2Medium::AdvertisingSession> BleV2Medium::StartAdvertising(
 bool BleV2Medium::StartScanning(const Uuid& service_uuid,
                                 TxPowerLevel tx_power_level,
                                 ScanCallback callback) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(INFO) << __func__
                     << ": service UUID: " << std::string(service_uuid)
                     << ", TxPowerLevel: " << TxPowerLevelToName(tx_power_level);
@@ -270,6 +280,22 @@ bool BleV2Medium::StartScanning(const Uuid& service_uuid,
     watcher_.AdvertisementFilter(advertisement_filter);
     watcher_.Start();
 
+    // Wait for the watcher to start.
+    int wait_milliseconds = 0;
+    while (watcher_.Status() !=
+           BluetoothLEAdvertisementWatcherStatus::Started) {
+      absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills));
+      wait_milliseconds += kMediumCheckIntervalInMills;
+      if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) {
+        NEARBY_LOGS(ERROR) << __func__
+                           << ": Failed to start BLE scan due to timeout..";
+        watcher_.Stopped(watcher_token_);
+        watcher_.Received(advertisement_received_token_);
+        watcher_ = nullptr;
+        return false;
+      }
+    }
+
     is_watcher_started_ = true;
 
     NEARBY_LOGS(INFO) << __func__ << ": BLE scanning started.";
@@ -414,6 +440,7 @@ std::unique_ptr<BleV2Medium::ScanningSession> BleV2Medium::StartScanning(
 
 std::unique_ptr<api::ble_v2::GattServer> BleV2Medium::StartGattServer(
     api::ble_v2::ServerGattConnectionCallback callback) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(INFO) << __func__ << ": Start GATT server.";
 
   if (!NearbyFlags::GetInstance().GetBoolFlag(
@@ -442,6 +469,7 @@ std::unique_ptr<api::ble_v2::GattServer> BleV2Medium::StartGattServer(
 std::unique_ptr<api::ble_v2::GattClient> BleV2Medium::ConnectToGattServer(
     api::ble_v2::BlePeripheral& peripheral, TxPowerLevel tx_power_level,
     api::ble_v2::ClientGattConnectionCallback callback) {
+  absl::MutexLock lock(&mutex_);
   NEARBY_LOGS(INFO) << "ConnectToGattServer is called, address: "
                     << peripheral.GetAddress()
                     << ", power:" << TxPowerLevelToName(tx_power_level);
@@ -482,6 +510,8 @@ std::unique_ptr<api::ble_v2::GattClient> BleV2Medium::ConnectToGattServer(
 }
 
 bool BleV2Medium::StopScanning() {
+  absl::MutexLock lock(&mutex_);
+
   NEARBY_LOGS(INFO) << __func__ << ": BLE StopScanning: service_uuid: "
                     << std::string(service_uuid_);
   try {
@@ -498,9 +528,26 @@ bool BleV2Medium::StopScanning() {
 
     watcher_.Stop();
 
-    // Don't need to wait for the status becomes to `Stopped`. If application
-    // starts to scanning immediately, the scanning still needs to wait the
-    // stopping to finish.
+    // Wait for the watcher to stop.
+    int wait_milliseconds = 0;
+    while (watcher_.Status() !=
+           BluetoothLEAdvertisementWatcherStatus::Stopped) {
+      absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills));
+      wait_milliseconds += kMediumCheckIntervalInMills;
+      if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) {
+        NEARBY_LOGS(ERROR) << __func__
+                           << ": Failed to stop BLE scan due to timeout.";
+        watcher_.Stopped(watcher_token_);
+        watcher_.Received(advertisement_received_token_);
+        watcher_ = nullptr;
+        is_watcher_started_ = false;
+        return false;
+      }
+    }
+
+    watcher_.Stopped(watcher_token_);
+    watcher_.Received(advertisement_received_token_);
+    watcher_ = nullptr;
     is_watcher_started_ = false;
 
     NEARBY_LOGS(ERROR)
@@ -662,6 +709,22 @@ bool BleV2Medium::StartBleAdvertising(
 
     publisher_.Start();
 
+    // Wait for the publisher to start.
+    int wait_milliseconds = 0;
+    while (publisher_.Status() !=
+           BluetoothLEAdvertisementPublisherStatus::Started) {
+      absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills));
+      wait_milliseconds += kMediumCheckIntervalInMills;
+      if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) {
+        NEARBY_LOGS(ERROR)
+            << __func__ << ": BLE advertising failed to start due to timeout.";
+        publisher_.StatusChanged(publisher_token_);
+        publisher_ = nullptr;
+        is_ble_publisher_started_ = false;
+        return false;
+      }
+    }
+
     is_ble_publisher_started_ = true;
     NEARBY_LOGS(INFO) << "BLE advertising started.";
     return true;
@@ -697,15 +760,34 @@ bool BleV2Medium::StopBleAdvertising() {
     }
 
     // publisher_ may be null when status changed during advertising.
-    if (publisher_ != nullptr &&
-        publisher_.Status() ==
+    if (publisher_ == nullptr ||
+        publisher_.Status() !=
             BluetoothLEAdvertisementPublisherStatus::Started) {
-      publisher_.Stop();
+      NEARBY_LOGS(WARNING) << "No started publisher is running.";
+      return false;
     }
 
-    // Don't need to wait for the status becomes to `Stopped`. If application
-    // starts to scanning immediately, the scanning still needs to wait the
-    // stopping to finish.
+    publisher_.Stop();
+
+    // Wait for the publisher to stop.
+    int wait_milliseconds = 0;
+    while (publisher_.Status() !=
+           BluetoothLEAdvertisementPublisherStatus::Stopped) {
+      absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills));
+      wait_milliseconds += kMediumCheckIntervalInMills;
+      if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) {
+        NEARBY_LOGS(ERROR)
+            << __func__ << ": BLE advertising failed to stop due to timeout.";
+        publisher_.StatusChanged(publisher_token_);
+        publisher_ = nullptr;
+        is_ble_publisher_started_ = false;
+        return false;
+      }
+    }
+
+    // Reset publisher.
+    publisher_.StatusChanged(publisher_token_);
+    publisher_ = nullptr;
     is_ble_publisher_started_ = false;
 
     return true;
@@ -912,14 +994,6 @@ void BleV2Medium::PublisherHandler(
     default:
       break;
   }
-
-  // The publisher is stopped. Clean up the running publisher
-  if (publisher_ != nullptr) {
-    NEARBY_LOGS(ERROR) << "Nearby BLE Medium cleaned the publisher.";
-    publisher_.StatusChanged(publisher_token_);
-    publisher_ = nullptr;
-    is_ble_publisher_started_ = false;
-  }
 }
 
 void BleV2Medium::WatcherHandler(
@@ -972,16 +1046,6 @@ void BleV2Medium::WatcherHandler(
           << "Nearby BLE Medium stoped to scan due to unknown errors.";
       break;
   }
-
-  // No matter the reason, should clean up the watcher if it is not empty.
-  // The BLE V1 interface doesn't have API to return the error to upper layer.
-  if (watcher_ != nullptr) {
-    NEARBY_LOGS(ERROR) << "Nearby BLE Medium cleaned the watcher.";
-    watcher_.Stopped(watcher_token_);
-    watcher_.Received(advertisement_received_token_);
-    watcher_ = nullptr;
-    is_watcher_started_ = false;
-  }
 }
 
 void BleV2Medium::AdvertisementReceivedHandler(

internal/platform/implementation/windows/ble_v2.h

@@ -15,18 +15,24 @@
 #ifndef THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_IMPLEMENTATION_WINDOWS_BLE_V2_H_
 #define THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_IMPLEMENTATION_WINDOWS_BLE_V2_H_
 
+#include <algorithm>
+#include <cstdint>
 #include <memory>
 #include <string>
 
+#include "absl/base/thread_annotations.h"
+#include "absl/container/flat_hash_map.h"
+#include "absl/strings/string_view.h"
 #include "absl/synchronization/mutex.h"
-#include "internal/platform/byte_array.h"
+#include "absl/synchronization/notification.h"
+#include "absl/time/clock.h"
+#include "absl/time/time.h"
+#include "internal/platform/cancellation_flag.h"
 #include "internal/platform/implementation/ble_v2.h"
+#include "internal/platform/implementation/bluetooth_adapter.h"
 #include "internal/platform/implementation/windows/ble_gatt_server.h"
 #include "internal/platform/implementation/windows/ble_v2_peripheral.h"
 #include "internal/platform/implementation/windows/bluetooth_adapter.h"
-#include "internal/platform/implementation/windows/bluetooth_classic.h"
-#include "internal/platform/input_stream.h"
-#include "internal/platform/output_stream.h"
 #include "internal/platform/uuid.h"
 #include "winrt/Windows.Devices.Bluetooth.Advertisement.h"
 
@@ -42,8 +48,9 @@ class BleV2Medium : public api::ble_v2::BleMedium {
   // Returns true once the Ble advertising has been initiated.
   bool StartAdvertising(
       const api::ble_v2::BleAdvertisementData& advertising_data,
-      api::ble_v2::AdvertiseParameters advertising_parameters) override;
-  bool StopAdvertising() override;
+      api::ble_v2::AdvertiseParameters advertising_parameters) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
+  bool StopAdvertising() override ABSL_LOCKS_EXCLUDED(mutex_);
 
   std::unique_ptr<AdvertisingSession> StartAdvertising(
       const api::ble_v2::BleAdvertisementData& advertising_data,
@@ -52,41 +59,48 @@ class BleV2Medium : public api::ble_v2::BleMedium {
 
   bool StartScanning(const Uuid& service_uuid,
                      api::ble_v2::TxPowerLevel tx_power_level,
-                     ScanCallback callback) override;
-  bool StopScanning() override;
+                     ScanCallback callback) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
+  bool StopScanning() override ABSL_LOCKS_EXCLUDED(mutex_);
   std::unique_ptr<ScanningSession> StartScanning(
       const Uuid& service_uuid, api::ble_v2::TxPowerLevel tx_power_level,
       ScanningCallback callback) override;
   std::unique_ptr<api::ble_v2::GattServer> StartGattServer(
-      api::ble_v2::ServerGattConnectionCallback callback) override;
+      api::ble_v2::ServerGattConnectionCallback callback) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
   std::unique_ptr<api::ble_v2::GattClient> ConnectToGattServer(
       api::ble_v2::BlePeripheral& peripheral,
       api::ble_v2::TxPowerLevel tx_power_level,
-      api::ble_v2::ClientGattConnectionCallback callback) override;
+      api::ble_v2::ClientGattConnectionCallback callback) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
   std::unique_ptr<api::ble_v2::BleServerSocket> OpenServerSocket(
-      const std::string& service_id) override;
+      const std::string& service_id) override ABSL_LOCKS_EXCLUDED(mutex_);
   std::unique_ptr<api::ble_v2::BleSocket> Connect(
       const std::string& service_id, api::ble_v2::TxPowerLevel tx_power_level,
       api::ble_v2::BlePeripheral& remote_peripheral,
-      CancellationFlag* cancellation_flag) override;
+      CancellationFlag* cancellation_flag) override ABSL_LOCKS_EXCLUDED(mutex_);
   bool IsExtendedAdvertisementsAvailable() override;
 
   bool GetRemotePeripheral(const std::string& mac_address,
-                           GetRemotePeripheralCallback callback) override;
+                           GetRemotePeripheralCallback callback) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
   bool GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id,
-                           GetRemotePeripheralCallback callback) override;
+                           GetRemotePeripheralCallback callback) override
+      ABSL_LOCKS_EXCLUDED(mutex_);
 
  private:
   bool StartBleAdvertising(
       const api::ble_v2::BleAdvertisementData& advertising_data,
-      api::ble_v2::AdvertiseParameters advertising_parameters);
-  bool StopBleAdvertising();
+      api::ble_v2::AdvertiseParameters advertising_parameters)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
+  bool StopBleAdvertising() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   bool StartGattAdvertising(
       const api::ble_v2::BleAdvertisementData& advertising_data,
-      api::ble_v2::AdvertiseParameters advertising_parameters);
-  bool StopGattAdvertising();
+      api::ble_v2::AdvertiseParameters advertising_parameters)
+      ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
+  bool StopGattAdvertising() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
 
   void PublisherHandler(
       winrt::Windows::Devices::Bluetooth::Advertisement::
@@ -120,6 +134,8 @@ class BleV2Medium : public api::ble_v2::BleMedium {
   void RemoveExpiredPeripherals()
       ABSL_EXCLUSIVE_LOCKS_REQUIRED(peripheral_map_mutex_);
 
+  absl::Mutex mutex_;
+
   BluetoothAdapter* adapter_;
   Uuid service_uuid_;
   api::ble_v2::TxPowerLevel tx_power_level_;