Fix several race conditions

The fixes are specific to unit test environment.

PiperOrigin-RevId: 546342096
This commit is contained in:
Janusz Sobczak
2023-07-07 11:53:50 -07:00
committed by Copybara-Service
parent 61820e49fb
commit ca9bbd3a9c
6 changed files with 65 additions and 28 deletions
+5
View File
@@ -102,8 +102,13 @@ class Borrowed {
template <typename T>
class Borrowable {
public:
Borrowable() = default;
explicit Borrowable(std::weak_ptr<BorrowableSharedData<T>> 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
+17
View File
@@ -125,5 +125,22 @@ TEST(Borrowable, BorrowIsExclusive) {
EXPECT_NE(lender.GetBorrowable().Borrow()->GetValue(), kDefaultValue);
}
TEST(Borrowable, DefaultBorrowableFails) {
Borrowable<int> borrowable;
EXPECT_FALSE(borrowable.Borrow());
}
TEST(Borrowable, CopyBorrowable) {
constexpr int kValue = 1;
Lender<int> lender(kValue);
Borrowable<int> borrowable = lender.GetBorrowable();
Borrowable<int> copy = borrowable;
Borrowed<int> borrowed = copy.Borrow();
ASSERT_TRUE(borrowed);
EXPECT_EQ(*borrowed, kValue);
}
} // namespace
} // namespace nearby
+24 -14
View File
@@ -361,8 +361,8 @@ std::unique_ptr<api::ble_v2::GattServer> BleV2Medium::StartGattServer(
return std::make_unique<GattServer>(*this, std::move(callback));
}
bool BleV2Medium::IsStopped(Borrowable<api::ble_v2::GattServer*>* server) {
auto borrowed = server->Borrow();
bool BleV2Medium::IsStopped(Borrowable<api::ble_v2::GattServer*> server) {
auto borrowed = server.Borrow();
if (!borrowed) {
return true;
}
@@ -373,14 +373,14 @@ bool BleV2Medium::IsStopped(Borrowable<api::ble_v2::GattServer*>* server) {
std::unique_ptr<api::ble_v2::GattClient> BleV2Medium::ConnectToGattServer(
api::ble_v2::BlePeripheral& peripheral, TxPowerLevel tx_power_level,
api::ble_v2::ClientGattConnectionCallback callback) {
Borrowable<api::ble_v2::GattServer*>* server =
Borrowable<api::ble_v2::GattServer*> 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<GattClient>(peripheral, *server, std::move(callback));
return std::make_unique<GattClient>(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<api::ble_v2::GattServer*> 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();
}
+5 -8
View File
@@ -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<const BleV2Peripheral*, api::ble_v2::GattCharacteristic>;
using SubscriberCallback =
absl::AnyInvocable<void(absl::string_view value)>;
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<api::ble_v2::GattServer*> gatt_server_;
api::ble_v2::ClientGattConnectionCallback callback_;
};
bool IsStopped(Borrowable<api::ble_v2::GattServer*>* server);
bool IsStopped(Borrowable<api::ble_v2::GattServer*> server);
absl::Mutex mutex_;
BluetoothAdapter* adapter_; // Our device adapter; read-only.
BleV2Peripheral peripheral_{adapter_};
+13 -5
View File
@@ -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<api::ble_v2::GattServer*>* MediumEnvironment::GetGattServer(
Borrowable<api::ble_v2::GattServer*> MediumEnvironment::GetGattServer(
api::ble_v2::BlePeripheral& peripheral) {
Borrowable<api::ble_v2::GattServer*>* result = nullptr;
Borrowable<api::ble_v2::GattServer*> result;
bool found_server = false;
CountDownLatch latch(1);
RunOnMediumEnvironmentThread([&]() {
for (const auto& medium_info : ble_v2_mediums_) {
@@ -1171,13 +1178,14 @@ Borrowable<api::ble_v2::GattServer*>* 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();
}
+1 -1
View File
@@ -353,7 +353,7 @@ class MediumEnvironment {
Borrowable<api::ble_v2::GattServer*> gatt_server);
void UnregisterGattServer(api::ble_v2::BleMedium& medium);
Borrowable<api::ble_v2::GattServer*>* GetGattServer(
Borrowable<api::ble_v2::GattServer*> GetGattServer(
api::ble_v2::BlePeripheral& peripheral);
// Configures the BluetoothPairingContext for remote BluetoothDevice.