Fixed one GATT crash

PiperOrigin-RevId: 657684638
This commit is contained in:
Guogang Li
2024-07-30 12:13:51 -07:00
committed by Copybara-Service
parent 68d432c5a8
commit 4d457d134b
5 changed files with 162 additions and 96 deletions
@@ -26,6 +26,7 @@
#include <vector>
#include "absl/container/flat_hash_map.h"
#include "absl/functional/any_invocable.h"
#include "absl/log/check.h"
#include "absl/status/status.h"
#include "absl/strings/escaping.h"
@@ -116,7 +117,9 @@ BleGattServer::BleGattServer(api::BluetoothAdapter* adapter,
api::ble_v2::ServerGattConnectionCallback callback)
: adapter_(dynamic_cast<BluetoothAdapter*>(adapter)),
peripheral_(adapter_->GetMacAddress()),
gatt_connection_callback_(std::move(callback)) {}
gatt_connection_callback_(std::move(callback)) {
DCHECK(adapter_ != nullptr);
}
absl::optional<api::ble_v2::GattCharacteristic>
BleGattServer::CreateCharacteristic(
@@ -204,28 +207,35 @@ 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) {
NEARBY_LOGS(WARNING) << __func__ << ": GATT server already stopped.";
return;
}
absl::AnyInvocable<void()> close_notifier = nullptr;
{
absl::MutexLock lock(&mutex_);
NEARBY_LOGS(VERBOSE) << __func__ << ": Start to stop GATT server.";
if (gatt_service_provider_ != nullptr) {
try {
if (is_advertising_) {
gatt_service_provider_.StopAdvertising();
}
if (is_advertising_) {
gatt_service_provider_.StopAdvertising();
gatt_characteristic_datas_.clear();
service_uuid_ = Uuid();
gatt_service_provider_ = nullptr;
} catch (std::exception exception) {
NEARBY_LOGS(ERROR) << __func__ << ": Exception: " << exception.what();
} catch (const winrt::hresult_error& error) {
NEARBY_LOGS(ERROR) << __func__ << ": WinRT exception: " << error.code()
<< ": " << winrt::to_string(error.message());
} catch (...) {
NEARBY_LOGS(ERROR) << __func__ << ": Unknown exception.";
}
} else {
NEARBY_LOGS(WARNING) << __func__ << ": no GATT server is running.";
}
close_notifier = std::move(close_notifier_);
}
gatt_characteristic_datas_.clear();
service_uuid_ = Uuid();
gatt_service_provider_ = nullptr;
} catch (std::exception exception) {
NEARBY_LOGS(ERROR) << __func__ << ": Exception: " << exception.what();
} catch (const winrt::hresult_error& error) {
NEARBY_LOGS(ERROR) << __func__ << ": WinRT exception: " << error.code()
<< ": " << winrt::to_string(error.message());
} catch (...) {
NEARBY_LOGS(ERROR) << __func__ << ": Unknown exception.";
if (close_notifier != nullptr) {
close_notifier();
}
}
@@ -252,6 +262,14 @@ bool BleGattServer::InitializeGattServer() {
return false;
}
if (!adapter_->IsPeripheralRoleSupported()) {
NEARBY_LOGS(ERROR)
<< __func__
<< ": Bluetooth Hardware does not support Peripheral Role, which is "
"required to start GATT server.";
return false;
}
winrt::guid service_uuid = nearby_uuid_to_winrt_guid(service_uuid_);
GattServiceProviderResult service_provider_result =
GattServiceProvider::CreateAsync(service_uuid).get();
@@ -416,14 +434,6 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data,
return false;
}
if (!adapter_->IsPeripheralRoleSupported()) {
NEARBY_LOGS(ERROR)
<< __func__
<< ": Bluetooth Hardware does not support Peripheral Role, which is "
"required to start GATT server.";
return false;
}
// Start the GATT server advertising
GattServiceProviderAdvertisingParameters advertisement_parameters;
advertisement_parameters.IsConnectable(is_connectable);
@@ -514,6 +524,11 @@ bool BleGattServer::StopAdvertisement() {
return false;
}
void BleGattServer::SetCloseNotifier(absl::AnyInvocable<void()> notifier) {
absl::MutexLock lock(&mutex_);
close_notifier_ = std::move(notifier);
}
::winrt::fire_and_forget BleGattServer::Characteristic_ReadRequestedAsync(
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattLocalCharacteristic const& gatt_local_characteristic,
@@ -22,6 +22,7 @@
#include "absl/base/thread_annotations.h"
#include "absl/container/flat_hash_map.h"
#include "absl/functional/any_invocable.h"
#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "absl/synchronization/mutex.h"
@@ -43,6 +44,7 @@ namespace windows {
class BleGattServer : public api::ble_v2::GattServer {
public:
// Make sure the adapter parameter is not null.
BleGattServer(api::BluetoothAdapter* adapter,
api::ble_v2::ServerGattConnectionCallback callback);
~BleGattServer() override = default;
@@ -66,6 +68,9 @@ class BleGattServer : public api::ble_v2::GattServer {
ABSL_LOCKS_EXCLUDED(mutex_);
bool StopAdvertisement() ABSL_LOCKS_EXCLUDED(mutex_);
void SetCloseNotifier(absl::AnyInvocable<void()> notifier)
ABSL_LOCKS_EXCLUDED(mutex_);
api::ble_v2::BlePeripheral& GetBlePeripheral() override {
return peripheral_;
}
@@ -86,53 +91,65 @@ class BleGattServer : public api::ble_v2::GattServer {
::winrt::event_token subscribed_clients_changed_token{};
};
bool InitializeGattServer();
bool InitializeGattServer() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
void NotifyValueChanged(
const api::ble_v2::GattCharacteristic& gatt_characteristic);
const api::ble_v2::GattCharacteristic& gatt_characteristic)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
GattCharacteristicData* FindGattCharacteristicData(
const ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattLocalCharacteristic& gatt_local_characteristic);
GattLocalCharacteristic& gatt_local_characteristic)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
GattCharacteristicData* FindGattCharacteristicData(
const api::ble_v2::GattCharacteristic& gatt_characteristic);
const api::ble_v2::GattCharacteristic& gatt_characteristic)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
::winrt::fire_and_forget Characteristic_ReadRequestedAsync(
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattLocalCharacteristic const& gatt_local_characteristic,
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattReadRequestedEventArgs args);
GattReadRequestedEventArgs args)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
::winrt::fire_and_forget Characteristic_WriteRequestedAsync(
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattLocalCharacteristic const& gatt_local_characteristic,
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattWriteRequestedEventArgs args);
GattWriteRequestedEventArgs args)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
void Characteristic_SubscribedClientsChanged(
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattLocalCharacteristic const& gatt_local_characteristic,
::winrt::Windows::Foundation::IInspectable const& args);
::winrt::Windows::Foundation::IInspectable const& args)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
void ServiceProvider_AdvertisementStatusChanged(
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattServiceProvider const& sender,
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattServiceProviderAdvertisementStatusChangedEventArgs const& args);
GattServiceProviderAdvertisementStatusChangedEventArgs const& args)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
absl::Mutex mutex_;
BluetoothAdapter* adapter_ = nullptr;
BluetoothAdapter* const adapter_ = nullptr;
BleV2Peripheral peripheral_;
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattServiceProvider gatt_service_provider_ = nullptr;
Uuid service_uuid_;
std::vector<GattCharacteristicData> gatt_characteristic_datas_;
api::ble_v2::ServerGattConnectionCallback gatt_connection_callback_{};
::winrt::event_token service_provider_advertisement_changed_token_{};
bool is_advertising_ = false;
bool is_gatt_server_inited_ = false;
::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile::
GattServiceProvider gatt_service_provider_ ABSL_GUARDED_BY(mutex_) =
nullptr;
absl::AnyInvocable<void()> close_notifier_ ABSL_GUARDED_BY(mutex_) = nullptr;
Uuid service_uuid_ ABSL_GUARDED_BY(mutex_);
std::vector<GattCharacteristicData> gatt_characteristic_datas_
ABSL_GUARDED_BY(mutex_);
bool is_advertising_ ABSL_GUARDED_BY(mutex_) = false;
bool is_gatt_server_inited_ ABSL_GUARDED_BY(mutex_) = false;
::winrt::event_token service_provider_advertisement_changed_token_
ABSL_GUARDED_BY(mutex_) = {};
};
} // namespace windows
@@ -14,10 +14,13 @@
#include "internal/platform/implementation/windows/ble_gatt_server.h"
#include <functional>
#include <string>
#include <utility>
#include "gtest/gtest.h"
#include "absl/synchronization/notification.h"
#include "absl/time/time.h"
#include "internal/platform/implementation/ble_v2.h"
#include "internal/platform/implementation/windows/bluetooth_adapter.h"
@@ -44,6 +47,23 @@ TEST(BleV2GattServer, DISABLED_Stop) {
blev2_gatt_server.Stop();
}
TEST(BleV2GattServer, DISABLED_StopNotifierIsCalled) {
BluetoothAdapter bluetoothAdapter;
BleGattServer blev2_gatt_server(&bluetoothAdapter, {});
bool is_close_notifier_called = false;
absl::Notification notification;
std::function<void()> notifier = [&is_close_notifier_called,
&notification]() {
is_close_notifier_called = true;
notification.Notify();
};
blev2_gatt_server.SetCloseNotifier(std::move(notifier));
blev2_gatt_server.Stop();
notification.WaitForNotificationWithTimeout(absl::Seconds(1));
EXPECT_TRUE(is_close_notifier_called);
}
TEST(BleV2GattServer, DISABLED_CreateCharacteristic) {
BluetoothAdapter bluetoothAdapter;
BleGattServer blev2_gatt_server(&bluetoothAdapter, {});
@@ -16,6 +16,7 @@
#include <algorithm>
#include <array>
#include <cstddef>
#include <cstdint>
#include <exception>
#include <iostream>
@@ -29,6 +30,7 @@
#include "absl/strings/numbers.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"
#include "absl/synchronization/mutex.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
@@ -320,6 +322,7 @@ bool BleV2Medium::StartScanning(const Uuid& service_uuid,
std::unique_ptr<BleV2Medium::ScanningSession> BleV2Medium::StartScanning(
const Uuid& service_uuid, TxPowerLevel tx_power_level,
BleV2Medium::ScanningCallback callback) {
absl::MutexLock lock(&mutex_);
NEARBY_LOGS(INFO) << __func__
<< ": service UUID: " << std::string(service_uuid)
<< ", TxPowerLevel: " << TxPowerLevelToName(tx_power_level);
@@ -371,16 +374,12 @@ std::unique_ptr<BleV2Medium::ScanningSession> BleV2Medium::StartScanning(
}
uint64_t session_id = GenerateSessionId();
// Save session id, service id and callback for this scan session.
{
absl::MutexLock lock(&map_mutex_);
auto iter = service_uuid_to_session_map_.find(service_uuid);
if (iter == service_uuid_to_session_map_.end()) {
service_uuid_to_session_map_[service_uuid].insert(
{session_id, std::move(callback)});
} else {
iter->second.insert({session_id, std::move(callback)});
}
auto iter = service_uuid_to_session_map_.find(service_uuid);
if (iter == service_uuid_to_session_map_.end()) {
service_uuid_to_session_map_[service_uuid].insert(
{session_id, std::move(callback)});
} else {
iter->second.insert({session_id, std::move(callback)});
}
// Generate and return ScanningSession.
@@ -390,7 +389,7 @@ std::unique_ptr<BleV2Medium::ScanningSession> BleV2Medium::StartScanning(
[this, session_id, service_uuid]() {
size_t num_erased_from_service_and_session_map = 0u;
{
absl::MutexLock lock(&map_mutex_);
absl::MutexLock lock(&mutex_);
auto iter = service_uuid_to_session_map_.find(service_uuid);
if (iter != service_uuid_to_session_map_.end()) {
num_erased_from_service_and_session_map =
@@ -462,6 +461,14 @@ std::unique_ptr<api::ble_v2::GattServer> BleV2Medium::StartGattServer(
std::make_unique<BleGattServer>(adapter_, std::move(callback));
ble_gatt_server_ = gatt_server.get();
ble_gatt_server_->SetCloseNotifier([this]() {
// In avoid to create a new thread to close the gatt server, we don't
// acquire the mutex here. The calling flow may cause deadlock due to
// StartGattAdvertising may run into the codes. It is not ideal, but it is
// hard to run in thread issues.
NEARBY_LOGS(INFO) << __func__ << ": GATT server is closed.";
ble_gatt_server_ = nullptr;
});
return gatt_server;
}
@@ -937,13 +944,13 @@ void BleV2Medium::PublisherHandler(
case BluetoothLEAdvertisementPublisherStatus::Aborted:
switch (args.Error()) {
case BluetoothError::Success:
if (publisher_.Status() ==
if (publisher.Status() ==
BluetoothLEAdvertisementPublisherStatus::Started) {
NEARBY_LOGS(ERROR)
<< "Nearby BLE Medium start advertising operation was "
"successfully completed or serviced.";
}
if (publisher_.Status() ==
if (publisher.Status() ==
BluetoothLEAdvertisementPublisherStatus::Stopped) {
NEARBY_LOGS(ERROR)
<< "Nearby BLE Medium stop advertising operation was "
@@ -1088,12 +1095,15 @@ void BleV2Medium::AdvertisementReceivedHandler(
std::string bluetooth_address =
uint64_to_mac_address_string(args.BluetoothAddress());
BleV2Peripheral* peripheral_ptr =
GetOrCreatePeripheral(bluetooth_address);
if (peripheral_ptr == nullptr) {
NEARBY_LOGS(ERROR) << "No BLE peripheral with address: "
<< bluetooth_address;
return;
BleV2Peripheral* peripheral_ptr = nullptr;
{
absl::MutexLock lock(&mutex_);
peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
if (peripheral_ptr == nullptr) {
NEARBY_LOGS(ERROR)
<< "No BLE peripheral with address: " << bluetooth_address;
return;
}
}
NEARBY_LOGS(INFO) << "BLE peripheral with address: " << bluetooth_address;
@@ -1123,7 +1133,7 @@ void BleV2Medium::AdvertisementFoundHandler(
std::vector<Uuid> service_uuid_list;
bool found_matching_service_uuid = false;
{
absl::MutexLock lock(&map_mutex_);
absl::MutexLock lock(&mutex_);
for (auto windows_service_uuid : advertisement.ServiceUuids()) {
auto nearby_service_uuid =
winrt_guid_to_nearby_uuid(windows_service_uuid);
@@ -1176,18 +1186,22 @@ void BleV2Medium::AdvertisementFoundHandler(
// Save the BleV2Peripheral.
std::string bluetooth_address =
uint64_to_mac_address_string(args.BluetoothAddress());
BleV2Peripheral* peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
if (peripheral_ptr == nullptr) {
NEARBY_LOGS(ERROR) << "No BLE peripheral with address: "
<< bluetooth_address;
return;
BleV2Peripheral* peripheral_ptr = nullptr;
{
absl::MutexLock lock(&mutex_);
peripheral_ptr = GetOrCreatePeripheral(bluetooth_address);
if (peripheral_ptr == nullptr) {
NEARBY_LOGS(ERROR) << "No BLE peripheral with address: "
<< bluetooth_address;
return;
}
}
NEARBY_LOGS(INFO) << "BLE peripheral with address: " << bluetooth_address;
// Invokes callbacks that matches the UUID.
for (auto service_uuid : service_uuid_list) {
{
absl::MutexLock lock(&map_mutex_);
absl::MutexLock lock(&mutex_);
if (service_uuid_to_session_map_.find(service_uuid) !=
service_uuid_to_session_map_.end()) {
for (auto& id_session_pair :
@@ -1202,6 +1216,7 @@ void BleV2Medium::AdvertisementFoundHandler(
bool BleV2Medium::GetRemotePeripheral(const std::string& mac_address,
GetRemotePeripheralCallback callback) {
absl::MutexLock lock(&mutex_);
BleV2Peripheral* peripheral = GetOrCreatePeripheral(mac_address);
if (peripheral != nullptr && peripheral->Ok()) {
callback(*peripheral);
@@ -1212,6 +1227,7 @@ bool BleV2Medium::GetRemotePeripheral(const std::string& mac_address,
bool BleV2Medium::GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id,
GetRemotePeripheralCallback callback) {
absl::MutexLock lock(&mutex_);
BleV2Peripheral* peripheral = GetPeripheral(id);
if (peripheral == nullptr) {
NEARBY_LOGS(WARNING) << __func__ << ": No matched peripheral device.";
@@ -1222,7 +1238,6 @@ bool BleV2Medium::GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id,
}
uint64_t BleV2Medium::GenerateSessionId() {
absl::MutexLock lock(&map_mutex_);
for (int i = 0; i < kGenerateSessionIdRetryLimit; i++) {
uint64_t session_id = Prng().NextInt64();
if (session_id == kFailedGenerateSessionId) continue;
@@ -1235,7 +1250,6 @@ uint64_t BleV2Medium::GenerateSessionId() {
}
BleV2Peripheral* BleV2Medium::GetOrCreatePeripheral(absl::string_view address) {
absl::MutexLock lock(&peripheral_map_mutex_);
auto it = std::find_if(
peripheral_map_.begin(), peripheral_map_.end(), [&](const auto& item) {
return item.second.peripheral->GetAddress() == address;
@@ -1261,7 +1275,6 @@ BleV2Peripheral* BleV2Medium::GetOrCreatePeripheral(absl::string_view address) {
}
BleV2Peripheral* BleV2Medium::GetPeripheral(BleV2Peripheral::UniqueId id) {
absl::MutexLock lock(&peripheral_map_mutex_);
auto it = peripheral_map_.find(id);
if (it == peripheral_map_.end()) {
return nullptr;
@@ -55,7 +55,7 @@ class BleV2Medium : public api::ble_v2::BleMedium {
std::unique_ptr<AdvertisingSession> StartAdvertising(
const api::ble_v2::BleAdvertisementData& advertising_data,
api::ble_v2::AdvertiseParameters advertise_set_parameters,
AdvertisingCallback callback) override;
AdvertisingCallback callback) override ABSL_LOCKS_EXCLUDED(mutex_);
bool StartScanning(const Uuid& service_uuid,
api::ble_v2::TxPowerLevel tx_power_level,
@@ -125,52 +125,53 @@ class BleV2Medium : public api::ble_v2::BleMedium {
winrt::Windows::Devices::Bluetooth::Advertisement::
BluetoothLEAdvertisementWatcherStoppedEventArgs args);
uint64_t GenerateSessionId();
uint64_t GenerateSessionId() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
// Returns nullptr if `address` is invalid.
BleV2Peripheral* GetOrCreatePeripheral(absl::string_view address);
BleV2Peripheral* GetOrCreatePeripheral(absl::string_view address)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
// Returns nullptr if `id` does not match a known peripheral.
BleV2Peripheral* GetPeripheral(BleV2Peripheral::UniqueId id);
BleV2Peripheral* GetPeripheral(BleV2Peripheral::UniqueId id)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
void RemoveExpiredPeripherals()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(peripheral_map_mutex_);
void RemoveExpiredPeripherals() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
absl::Mutex mutex_;
BluetoothAdapter* adapter_;
BluetoothAdapter* const adapter_;
Uuid service_uuid_;
api::ble_v2::TxPowerLevel tx_power_level_;
ScanCallback scan_callback_;
absl::Mutex map_mutex_;
// std::map<Uuid, std::map<uint64_t, ScanningCallback>>
absl::flat_hash_map<Uuid, absl::flat_hash_map<uint64_t, ScanningCallback>>
service_uuid_to_session_map_ ABSL_GUARDED_BY(map_mutex_);
service_uuid_to_session_map_ ABSL_GUARDED_BY(mutex_);
// WinRT objects
::winrt::Windows::Devices::Bluetooth::Advertisement::
BluetoothLEAdvertisementPublisher publisher_ = nullptr;
BluetoothLEAdvertisementPublisher publisher_ ABSL_GUARDED_BY(mutex_) =
nullptr;
::winrt::Windows::Devices::Bluetooth::Advertisement::
BluetoothLEAdvertisementWatcher watcher_ = nullptr;
BluetoothLEAdvertisementWatcher watcher_ ABSL_GUARDED_BY(mutex_) =
nullptr;
bool is_ble_publisher_started_ = false;
bool is_gatt_publisher_started_ = false;
bool is_watcher_started_ = false;
bool is_ble_publisher_started_ ABSL_GUARDED_BY(mutex_) = false;
bool is_gatt_publisher_started_ ABSL_GUARDED_BY(mutex_) = false;
bool is_watcher_started_ ABSL_GUARDED_BY(mutex_) = false;
::winrt::event_token publisher_token_;
::winrt::event_token watcher_token_;
::winrt::event_token advertisement_received_token_;
::winrt::event_token publisher_token_ ABSL_GUARDED_BY(mutex_);
::winrt::event_token watcher_token_ ABSL_GUARDED_BY(mutex_);
::winrt::event_token advertisement_received_token_ ABSL_GUARDED_BY(mutex_);
BleGattServer* ble_gatt_server_ = nullptr;
// Map to protect the pointer for BlePeripheral because
// DiscoveredPeripheralCallback only keeps the pointer to the object
absl::Mutex peripheral_map_mutex_;
struct PeripheralInfo {
absl::Time last_access_time;
std::unique_ptr<BleV2Peripheral> peripheral;
};
absl::flat_hash_map<BleV2Peripheral::UniqueId, PeripheralInfo> peripheral_map_
ABSL_GUARDED_BY(peripheral_map_mutex_);
absl::Time cleanup_time_ ABSL_GUARDED_BY(peripheral_map_mutex_) = absl::Now();
ABSL_GUARDED_BY(mutex_);
absl::Time cleanup_time_ ABSL_GUARDED_BY(mutex_) = absl::Now();
};
} // namespace windows