diff --git a/internal/platform/borrowable.h b/internal/platform/borrowable.h index 5b024ec8..93fa73c7 100644 --- a/internal/platform/borrowable.h +++ b/internal/platform/borrowable.h @@ -102,8 +102,13 @@ class Borrowed { template class Borrowable { public: + Borrowable() = default; explicit Borrowable(std::weak_ptr> resource) : resource_(resource) {} + Borrowable(const Borrowable&) = default; + Borrowable(Borrowable&&) = default; + Borrowable& operator=(const Borrowable&) = default; + Borrowable& operator=(Borrowable&&) = default; // Gives the caller exclusive access to the stored object. Borrowing fails if // the object has already been destroyed. The call will block if another diff --git a/internal/platform/borrowable_test.cc b/internal/platform/borrowable_test.cc index b7e913be..896e33a4 100644 --- a/internal/platform/borrowable_test.cc +++ b/internal/platform/borrowable_test.cc @@ -125,5 +125,22 @@ TEST(Borrowable, BorrowIsExclusive) { EXPECT_NE(lender.GetBorrowable().Borrow()->GetValue(), kDefaultValue); } +TEST(Borrowable, DefaultBorrowableFails) { + Borrowable borrowable; + EXPECT_FALSE(borrowable.Borrow()); +} + +TEST(Borrowable, CopyBorrowable) { + constexpr int kValue = 1; + + Lender lender(kValue); + Borrowable borrowable = lender.GetBorrowable(); + Borrowable copy = borrowable; + Borrowed borrowed = copy.Borrow(); + + ASSERT_TRUE(borrowed); + EXPECT_EQ(*borrowed, kValue); +} + } // namespace } // namespace nearby diff --git a/internal/platform/implementation/g3/ble_v2.cc b/internal/platform/implementation/g3/ble_v2.cc index 5354b937..469f72d9 100644 --- a/internal/platform/implementation/g3/ble_v2.cc +++ b/internal/platform/implementation/g3/ble_v2.cc @@ -361,8 +361,8 @@ std::unique_ptr BleV2Medium::StartGattServer( return std::make_unique(*this, std::move(callback)); } -bool BleV2Medium::IsStopped(Borrowable* server) { - auto borrowed = server->Borrow(); +bool BleV2Medium::IsStopped(Borrowable server) { + auto borrowed = server.Borrow(); if (!borrowed) { return true; } @@ -373,14 +373,14 @@ bool BleV2Medium::IsStopped(Borrowable* server) { std::unique_ptr BleV2Medium::ConnectToGattServer( api::ble_v2::BlePeripheral& peripheral, TxPowerLevel tx_power_level, api::ble_v2::ClientGattConnectionCallback callback) { - Borrowable* server = + Borrowable server = MediumEnvironment::Instance().GetGattServer(peripheral); - if (server == nullptr || IsStopped(server)) { + if (IsStopped(server)) { NEARBY_LOGS(WARNING) << "No GATT server found for " << peripheral.GetAddress(); return nullptr; } - return std::make_unique(peripheral, *server, std::move(callback)); + return std::make_unique(peripheral, server, std::move(callback)); } bool BleV2Medium::IsExtendedAdvertisementsAvailable() { @@ -580,14 +580,26 @@ bool BleV2Medium::GattServer::HasCharacteristic( return characteristics_.find(characteristic) != characteristics_.end(); } +void BleV2Medium::GattServer::Connect(GattClient* client) { + absl::MutexLock lock(&mutex_); + connected_clients_.push_back(client); +} + +void BleV2Medium::GattServer::Disconnect(GattClient* client) { + absl::MutexLock lock(&mutex_); + connected_clients_.erase( + std::remove(connected_clients_.begin(), connected_clients_.end(), client), + connected_clients_.end()); +} + void BleV2Medium::GattServer::Stop() { if (stopped_) return; - NEARBY_LOGS(INFO) << "G3 Ble GattServer Stop"; + absl::MutexLock lock(&mutex_); stopped_ = true; - characteristics_.clear(); for (auto& client : connected_clients_) { client->OnServerDisconnected(); } + characteristics_.clear(); } BleV2Medium::GattClient::GattClient( @@ -754,9 +766,9 @@ bool BleV2Medium::GattClient::SetCharacteristicSubscription( } void BleV2Medium::GattClient::Disconnect() { - absl::MutexLock lock(&mutex_); + bool was_alive = is_connection_alive_.exchange(false); + if (!was_alive) return; NEARBY_LOGS(INFO) << "G3 Ble GattClient Disconnect"; - is_connection_alive_ = false; Borrowed borrowed = gatt_server_.Borrow(); if (borrowed) { BleV2Medium::GattServer* gatt_server = @@ -766,11 +778,9 @@ void BleV2Medium::GattClient::Disconnect() { } void BleV2Medium::GattClient::OnServerDisconnected() { - { - absl::MutexLock lock(&mutex_); - NEARBY_LOGS(INFO) << "G3 Ble GattServer disconnected"; - is_connection_alive_ = false; - } + bool was_alive = is_connection_alive_.exchange(false); + if (!was_alive) return; + NEARBY_LOGS(INFO) << "G3 Ble GattServer disconnected"; if (callback_.disconnected_cb != nullptr) { callback_.disconnected_cb(); } diff --git a/internal/platform/implementation/g3/ble_v2.h b/internal/platform/implementation/g3/ble_v2.h index 9ba03b75..d2e6825c 100644 --- a/internal/platform/implementation/g3/ble_v2.h +++ b/internal/platform/implementation/g3/ble_v2.h @@ -279,18 +279,15 @@ class BleV2Medium : public api::ble_v2::BleMedium { bool HasCharacteristic( const api::ble_v2::GattCharacteristic& characteristic); - void Connect(GattClient* client) { connected_clients_.push_back(client); } - void Disconnect(GattClient* client) { - connected_clients_.erase(std::remove(connected_clients_.begin(), - connected_clients_.end(), client), - connected_clients_.end()); - } + void Connect(GattClient* client); + void Disconnect(GattClient* client); private: using SubscriberKey = std::pair; using SubscriberCallback = absl::AnyInvocable; + absl::Mutex mutex_; BleV2Medium& medium_; api::ble_v2::ServerGattConnectionCallback callback_; BleV2Peripheral ble_peripheral_; @@ -340,13 +337,13 @@ class BleV2Medium : public api::ble_v2::BleMedium { // A flag to indicate the gatt connection alive or not. If it is // disconnected/*false*/, the instance needs to be created again to bring // it alive. - bool is_connection_alive_ ABSL_GUARDED_BY(mutex_) = true; + std::atomic_bool is_connection_alive_ = true; BleV2Peripheral& peripheral_; Borrowable gatt_server_; api::ble_v2::ClientGattConnectionCallback callback_; }; - bool IsStopped(Borrowable* server); + bool IsStopped(Borrowable server); absl::Mutex mutex_; BluetoothAdapter* adapter_; // Our device adapter; read-only. BleV2Peripheral peripheral_{adapter_}; diff --git a/internal/platform/medium_environment.cc b/internal/platform/medium_environment.cc index 1a688443..06c6cb1a 100644 --- a/internal/platform/medium_environment.cc +++ b/internal/platform/medium_environment.cc @@ -1144,22 +1144,29 @@ void MediumEnvironment::RegisterGattServer( void MediumEnvironment::UnregisterGattServer(api::ble_v2::BleMedium& medium) { if (!enabled_) return; - RunOnMediumEnvironmentThread([this, &medium]() { + CountDownLatch latch(1); + RunOnMediumEnvironmentThread([&]() { auto it = ble_v2_mediums_.find(&medium); if (it == ble_v2_mediums_.end()) { NEARBY_LOGS(INFO) << "G3 UnregisterGattServer failed. There is no " "medium registered."; + latch.CountDown(); return; } auto& context = it->second; + NEARBY_LOGS(INFO) << "UnregisterGattServer for " + << context.ble_peripheral->GetAddress(); context.gatt_server = nullptr; context.ble_peripheral = nullptr; + latch.CountDown(); }); + latch.Await(); } -Borrowable* MediumEnvironment::GetGattServer( +Borrowable MediumEnvironment::GetGattServer( api::ble_v2::BlePeripheral& peripheral) { - Borrowable* result = nullptr; + Borrowable result; + bool found_server = false; CountDownLatch latch(1); RunOnMediumEnvironmentThread([&]() { for (const auto& medium_info : ble_v2_mediums_) { @@ -1171,13 +1178,14 @@ Borrowable* MediumEnvironment::GetGattServer( if (remote_context.gatt_server == nullptr) { break; } - result = remote_context.gatt_server.get(); + found_server = true; + result = *(remote_context.gatt_server); } } latch.CountDown(); }); latch.Await(); - if (result == nullptr) { + if (!found_server) { NEARBY_LOGS(INFO) << "G3 GetGattServer failed. No GATT server for " << peripheral.GetAddress(); } diff --git a/internal/platform/medium_environment.h b/internal/platform/medium_environment.h index 60595ac8..2e6d099d 100644 --- a/internal/platform/medium_environment.h +++ b/internal/platform/medium_environment.h @@ -353,7 +353,7 @@ class MediumEnvironment { Borrowable gatt_server); void UnregisterGattServer(api::ble_v2::BleMedium& medium); - Borrowable* GetGattServer( + Borrowable GetGattServer( api::ble_v2::BlePeripheral& peripheral); // Configures the BluetoothPairingContext for remote BluetoothDevice.