Fix race conditions with bluetooth devices being erased while being used.

This commit is contained in:
Vibhav Pant
2023-09-26 18:34:56 +05:30
parent b4bd7ddbb8
commit 97af86a414
10 changed files with 135 additions and 157 deletions
@@ -25,6 +25,7 @@
#include "internal/platform/implementation/linux/ble_v2_medium.h"
#include "internal/platform/implementation/linux/bluetooth_classic_device.h"
#include "internal/platform/implementation/linux/bluetooth_devices.h"
#include "internal/platform/implementation/linux/bluez.h"
#include "internal/platform/implementation/linux/bluez_advertisement_monitor.h"
#include "internal/platform/implementation/linux/bluez_advertisement_monitor_manager.h"
#include "internal/platform/implementation/linux/bluez_le_advertisement.h"
@@ -38,7 +39,7 @@ BleV2Medium::BleV2Medium(BluetoothAdapter &adapter)
: system_bus_(adapter.GetConnection()),
adapter_(adapter),
devices_(std::make_unique<BluetoothDevices>(
*system_bus_, adapter_.GetObjectPath(), observers_)),
system_bus_, adapter_.GetObjectPath(), observers_)),
gatt_discovery_(std::make_shared<BluezGattDiscovery>(system_bus_)),
root_object_manager_(std::make_unique<RootObjectManager>(*system_bus_)),
adv_monitor_manager_(
@@ -215,10 +216,10 @@ std::unique_ptr<api::ble_v2::GattClient> BleV2Medium::ConnectToGattServer(
api::ble_v2::BlePeripheral &peripheral,
api::ble_v2::TxPowerLevel tx_power_level,
api::ble_v2::ClientGattConnectionCallback callback) {
auto &device = dynamic_cast<BluetoothDevice &>(peripheral);
auto path = bluez::device_object_path(adapter_.GetObjectPath(),
peripheral.GetAddress());
return std::make_unique<GattClient>(system_bus_, device.getObjectPath(),
gatt_discovery_,
return std::make_unique<GattClient>(system_bus_, path, gatt_discovery_,
std::move(callback.disconnected_cb));
}
@@ -67,10 +67,10 @@ void Profile::NewConnection(
device = devices_.add_new_device(device_object_path);
}
auto alias = device->Alias();
auto mac_addr = device->Address();
auto alias = device->GetName();
auto mac_addr = device->GetAddress();
NEARBY_LOGS(VERBOSE) << __func__ << ": " << getObjectPath()
<< ": Connected to " << device->getObjectPath();
<< ": Connected to " << mac_addr;
FDProperties props(fd_props);
@@ -89,7 +89,7 @@ void Profile::RequestDisconnection(
throw sdbus::Error("org.bluez.Error.Rejected", "Unknown object");
}
auto mac_addr = device->Address();
auto mac_addr = device->GetMacAddress();
NEARBY_LOGS(VERBOSE) << __func__ << ": Disconnection requested for device "
<< device_object_path;
@@ -12,118 +12,96 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#include <memory>
#include <sdbus-c++/IObject.h>
#include <sdbus-c++/ProxyInterfaces.h>
#include <systemd/sd-bus.h>
#include "absl/strings/string_view.h"
#include "internal/platform/bluetooth_utils.h"
#include "internal/platform/implementation/linux/bluetooth_classic_device.h"
#include "internal/platform/implementation/linux/bluez.h"
#include "internal/platform/implementation/linux/bluez_device.h"
#include "internal/platform/implementation/linux/dbus.h"
#include "internal/platform/logging.h"
namespace nearby {
namespace linux {
BluetoothDevice::BluetoothDevice(sdbus::IConnection &system_bus,
sdbus::ObjectPath device_object_path)
: ProxyInterfaces(system_bus, bluez::SERVICE_DEST,
std::move(device_object_path)),
lost_(false) {
registerProxy();
BluetoothDevice::BluetoothDevice(std::shared_ptr<bluez::Device> device)
: lost_(false), device_(device) {
try {
last_known_name_ = Alias();
last_known_name_ = device->Alias();
} catch (const sdbus::Error &e) {
DBUS_LOG_PROPERTY_GET_ERROR(this, "Alias", e);
DBUS_LOG_PROPERTY_GET_ERROR(device, "Alias", e);
}
try {
last_known_address_ = Address();
last_known_address_ = device->Address();
unique_id_ = BluetoothUtils::ToNumber(last_known_address_);
} catch (const sdbus::Error &e) {
DBUS_LOG_PROPERTY_GET_ERROR(this, "Address", e);
DBUS_LOG_PROPERTY_GET_ERROR(device, "Address", e);
}
}
std::string BluetoothDevice::GetName() const {
auto bluez_device =
sdbus::createProxy(getProxy().getConnection(), bluez::SERVICE_DEST,
getProxy().getObjectPath());
auto device = device_.lock();
if (device == nullptr) {
absl::ReaderMutexLock l(&properties_mutex_);
return last_known_name_;
}
try {
std::string alias =
bluez_device->getProperty("Alias").onInterface(bluez::DEVICE_INTERFACE);
std::string alias = device->Alias();
{
absl::MutexLock l(&properties_mutex_);
last_known_name_ = alias;
}
return alias;
} catch (const sdbus::Error &e) {
if (e.getName() == "org.freedesktop.DBus.Error.UnknownObject") {
NEARBY_LOGS(VERBOSE)
<< __func__ << ": " << getObjectPath()
<< ": device is no longer known, returning last known name";
absl::ReaderMutexLock l(&properties_mutex_);
return last_known_name_;
}
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to get Alias for device "
<< bluez_device->getObjectPath();
return std::string();
DBUS_LOG_PROPERTY_GET_ERROR(device, "Alias", e);
return {};
}
}
std::string BluetoothDevice::GetMacAddress() const {
auto bluez_device =
sdbus::createProxy(getProxy().getConnection(), bluez::SERVICE_DEST,
getProxy().getObjectPath());
auto device = device_.lock();
if (device == nullptr) {
absl::ReaderMutexLock l(&properties_mutex_);
return last_known_name_;
}
try {
std::string addr = bluez_device->getProperty("Address").onInterface(
bluez::DEVICE_INTERFACE);
std::string addr = device->Address();
{
absl::MutexLock l(&properties_mutex_);
last_known_address_ = addr;
}
return addr;
} catch (const sdbus::Error &e) {
if (e.getName() == "org.freedesktop.DBus.Error.UnknownObject") {
NEARBY_LOGS(VERBOSE)
<< __func__ << ": " << getObjectPath()
<< ": device is no longer known, returning last known address";
absl::ReaderMutexLock l(&properties_mutex_);
return last_known_address_;
}
NEARBY_LOGS(ERROR) << __func__ << "Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to get Address for device "
<< bluez_device->getObjectPath();
DBUS_LOG_PROPERTY_GET_ERROR(device, "Address", e);
return std::string();
}
}
bool BluetoothDevice::ConnectToProfile(absl::string_view service_uuid) {
NEARBY_LOGS(VERBOSE) << __func__ << ": " << getObjectPath()
<< ": Attempting to connect to profile " << service_uuid;
auto device = device_.lock();
if (device == nullptr) return false;
try {
ConnectProfile(std::string(service_uuid));
device->ConnectProfile(std::string(service_uuid));
return true;
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to connect to profile "
<< service_uuid << " on device " << getObjectPath();
DBUS_LOG_METHOD_CALL_ERROR(device, "ConnectProfile", e);
return false;
}
}
MonitoredBluetoothDevice::MonitoredBluetoothDevice(
sdbus::IConnection &system_bus, const sdbus::ObjectPath &device_object_path,
std::shared_ptr<sdbus::IConnection> system_bus,
std::shared_ptr<bluez::Device> device,
ObserverList<api::BluetoothClassicMedium::Observer> &observers)
: BluetoothDevice(system_bus, device_object_path),
ProxyInterfaces<sdbus::Properties_proxy>(system_bus, bluez::SERVICE_DEST,
std::string(device_object_path)),
: BluetoothDevice(std::move(device)),
ProxyInterfaces<sdbus::Properties_proxy>(*system_bus, bluez::SERVICE_DEST,
device->getObjectPath()),
system_bus_(std::move(system_bus)),
observers_(observers) {
registerProxy();
}
@@ -142,20 +120,20 @@ void MonitoredBluetoothDevice::onPropertiesChanged(
NEARBY_LOGS(VERBOSE) << __func__ << ": " << getObjectPath()
<< ": Notifying observers about address change";
std::string address = it->second;
for (auto &observer : observers_.GetObservers()) {
for (const auto &observer : observers_.GetObservers()) {
observer->DeviceAddressChanged(*this, address);
}
} else if (it->first == bluez::DEVICE_PROP_PAIRED) {
NEARBY_LOGS(VERBOSE) << __func__ << ": " << getObjectPath()
<< "Notifying observers about paired status change.";
for (auto &observer : observers_.GetObservers()) {
for (const auto &observer : observers_.GetObservers()) {
observer->DevicePairedChanged(*this, it->second);
}
} else if (it->first == bluez::DEVICE_PROP_CONNECTED) {
NEARBY_LOGS(VERBOSE)
<< __func__ << ": " << getObjectPath()
<< "Notifying observers about connected status change";
for (auto &observer : observers_.GetObservers()) {
for (const auto &observer : observers_.GetObservers()) {
observer->DeviceConnectedStateChanged(*this, it->second);
}
} else if (it->first == bluez::DEVICE_NAME) {
@@ -16,7 +16,9 @@
#define PLATFORM_IMPL_LINUX_BLUETOOTH_CLASSIC_DEVICE_H_
#include <atomic>
#include <memory>
#include <sdbus-c++/Error.h>
#include <sdbus-c++/IConnection.h>
#include <sdbus-c++/IProxy.h>
#include <sdbus-c++/ProxyInterfaces.h>
@@ -29,15 +31,15 @@
#include "internal/base/observer_list.h"
#include "internal/platform/implementation/ble_v2.h"
#include "internal/platform/implementation/bluetooth_classic.h"
#include "internal/platform/implementation/linux/bluez_device.h"
#include "internal/platform/implementation/linux/dbus.h"
#include "internal/platform/implementation/linux/generated/dbus/bluez/device_client.h"
namespace nearby {
namespace linux {
// https://developer.android.com/reference/android/bluetooth/BluetoothDevice.html.
class BluetoothDevice
: public api::BluetoothDevice,
public sdbus::ProxyInterfaces<org::bluez::Device1_proxy>,
public api::ble_v2::BlePeripheral {
class BluetoothDevice : public api::BluetoothDevice,
public api::ble_v2::BlePeripheral {
public:
using UniqueId = std::uint64_t;
@@ -45,9 +47,7 @@ class BluetoothDevice
BluetoothDevice(BluetoothDevice &&) = delete;
BluetoothDevice &operator=(const BluetoothDevice &) = delete;
BluetoothDevice &operator=(BluetoothDevice &&) = delete;
BluetoothDevice(sdbus::IConnection &system_bus,
sdbus::ObjectPath device_object_path);
~BluetoothDevice() override { unregisterProxy(); }
explicit BluetoothDevice(std::shared_ptr<bluez::Device> device);
// BluetoothDevice methods
// https://developer.android.com/reference/android/bluetooth/BluetoothDevice.html#getName()
@@ -59,16 +59,57 @@ class BluetoothDevice
std::string GetAddress() const override { return GetMacAddress(); }
UniqueId GetUniqueId() const override { return unique_id_; };
void set_pair_reply_callback(
absl::AnyInvocable<void(const sdbus::Error *)> cb)
ABSL_LOCKS_EXCLUDED(pair_callback_lock_) {
absl::MutexLock l(&pair_callback_lock_);
on_pair_reply_cb_ = std::move(cb);
std::optional<std::map<std::string, sdbus::Variant>> ServiceData() {
auto device = device_.lock();
if (device == nullptr) return std::nullopt;
try {
return device->ServiceData();
} catch (const sdbus::Error &e) {
DBUS_LOG_PROPERTY_GET_ERROR(device, "ServiceData", e);
return std::nullopt;
}
}
bool Bonded() {
auto device = device_.lock();
if (device == nullptr) return false;
try {
return device->Bonded();
} catch (const sdbus::Error &e) {
DBUS_LOG_METHOD_CALL_ERROR(device, "Bonded", e);
return false;
}
}
void reset_pair_reply_callback() ABSL_LOCKS_EXCLUDED(pair_callback_lock_) {
absl::MutexLock l(&pair_callback_lock_);
on_pair_reply_cb_ = nullptr;
std::optional<sdbus::PendingAsyncCall> Pair() {
auto device = device_.lock();
if (device == nullptr) return std::nullopt;
try {
return device->Pair();
} catch (const sdbus::Error &e) {
DBUS_LOG_METHOD_CALL_ERROR(device, "Pair", e);
return std::nullopt;
}
}
bool CancelPairing() {
auto device = device_.lock();
if (device == nullptr) return false;
try {
device->CancelPairing();
return true;
} catch (const sdbus::Error &e) {
DBUS_LOG_METHOD_CALL_ERROR(device, "CancelPairing", e);
return false;
}
}
void SetPairReplyCallback(absl::AnyInvocable<void(const sdbus::Error *)> cb) {
auto device = device_.lock();
if (device != nullptr) device->SetPairReplyCallback(std::move(cb));
}
bool ConnectToProfile(absl::string_view service_uuid);
@@ -76,23 +117,14 @@ class BluetoothDevice
void UnmarkLost() { lost_ = false; }
bool Lost() const { return lost_; }
protected:
void onPairReply(const sdbus::Error *error) override
ABSL_LOCKS_EXCLUDED(pair_callback_lock_) {
absl::ReaderMutexLock l(&pair_callback_lock_);
if (on_pair_reply_cb_ != nullptr) on_pair_reply_cb_(error);
};
private:
absl::Mutex pair_callback_lock_;
absl::AnyInvocable<void(const sdbus::Error *)> on_pair_reply_cb_
ABSL_GUARDED_BY(pair_callback_lock_) = nullptr;
UniqueId unique_id_;
std::atomic_bool lost_;
mutable absl::Mutex properties_mutex_;
mutable std::string last_known_name_ ABSL_GUARDED_BY(properties_mutex_);
mutable std::string last_known_address_ ABSL_GUARDED_BY(properties_mutex_);
mutable std::weak_ptr<bluez::Device> device_;
};
class MonitoredBluetoothDevice final
@@ -109,7 +141,8 @@ class MonitoredBluetoothDevice final
delete;
MonitoredBluetoothDevice &operator=(MonitoredBluetoothDevice &&) = delete;
MonitoredBluetoothDevice(
sdbus::IConnection &system_bus, const sdbus::ObjectPath &,
std::shared_ptr<sdbus::IConnection> system_bus,
std::shared_ptr<bluez::Device> device,
ObserverList<api::BluetoothClassicMedium::Observer> &observers);
~MonitoredBluetoothDevice() override { unregisterProxy(); }
@@ -127,6 +160,7 @@ class MonitoredBluetoothDevice final
const std::vector<std::string> &invalidatedProperties) override;
private:
std::shared_ptr<sdbus::IConnection> system_bus_;
std::shared_ptr<api::BluetoothClassicMedium::DiscoveryCallback>
GetDiscoveryCallback() ABSL_LOCKS_EXCLUDED(discovery_cb_mutex_) {
discovery_cb_mutex_.ReaderLock();
@@ -37,7 +37,7 @@ BluetoothClassicMedium::BluetoothClassicMedium(BluetoothAdapter &adapter)
adapter_(adapter),
observers_(std::make_shared<ObserverList<Observer>>()),
devices_(std::make_shared<BluetoothDevices>(
*system_bus_, adapter.GetObjectPath(), *observers_)),
system_bus_, adapter.GetObjectPath(), *observers_)),
device_watcher_(nullptr),
profile_manager_(
std::make_unique<ProfileManager>(*system_bus_, *devices_)) {}
@@ -90,7 +90,9 @@ std::shared_ptr<MonitoredBluetoothDevice> BluetoothDevices::add_new_device(
auto [device_it, inserted] = devices_by_path_.emplace(
std::string(device_object_path),
std::make_shared<MonitoredBluetoothDevice>(
system_bus_, std::move(device_object_path), observers_));
system_bus_,
std::make_shared<bluez::Device>(system_bus_, device_object_path),
observers_));
if (!inserted) device_it->second->UnmarkLost();
return device_it->second;
}
@@ -37,9 +37,10 @@ namespace linux {
class BluetoothDevices final {
public:
BluetoothDevices(
sdbus::IConnection &system_bus, sdbus::ObjectPath adapter_object_path,
std::shared_ptr<sdbus::IConnection> system_bus,
sdbus::ObjectPath adapter_object_path,
ObserverList<api::BluetoothClassicMedium::Observer> &observers)
: system_bus_(system_bus),
: system_bus_(std::move(system_bus)),
observers_(observers),
adapter_object_path_(std::move(adapter_object_path)) {}
@@ -62,7 +63,7 @@ class BluetoothDevices final {
void cleanup_lost_peripherals() ABSL_LOCKS_EXCLUDED(devices_by_path_lock_);
private:
sdbus::IConnection &system_bus_;
std::shared_ptr<sdbus::IConnection> system_bus_;
ObserverList<api::BluetoothClassicMedium::Observer> &observers_;
sdbus::ObjectPath adapter_object_path_;
@@ -36,7 +36,7 @@ void BluetoothPairing::pairing_reply_handler(const sdbus::Error *error) {
<< "Got error '" << error->getName()
<< "' with message '" << error->getMessage()
<< "' while pairing with device "
<< device_->getObjectPath();
<< device_->GetMacAddress();
if (name == "org.bluez.Error.AuthenticationCanceled") {
err = api::BluetoothPairingCallback::PairingError::kAuthCanceled;
@@ -61,7 +61,10 @@ void BluetoothPairing::pairing_reply_handler(const sdbus::Error *error) {
BluetoothPairing::BluetoothPairing(
BluetoothAdapter &adapter, std::shared_ptr<BluetoothDevice> remote_device)
: device_(std::move(remote_device)), adapter_(adapter) {}
: device_(std::move(remote_device)),
device_object_path_(bluez::device_object_path(adapter.GetObjectPath(),
device_->GetAddress())),
adapter_(adapter) {}
bool BluetoothPairing::InitiatePairing(
api::BluetoothPairingCallback pairing_cb) {
@@ -75,66 +78,28 @@ bool BluetoothPairing::InitiatePairing(
bool BluetoothPairing::FinishPairing(
std::optional<absl::string_view> pin_code) {
device_->set_pair_reply_callback([this](const sdbus::Error *error) {
device_->SetPairReplyCallback([this](const sdbus::Error *error) {
this->pairing_reply_handler(error);
});
try {
pair_async_call_ = device_->Pair();
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to initiate pairing for device "
<< device_->getObjectPath();
return false;
}
auto call = device_->Pair();
if (!call.has_value()) return false;
pair_async_call_ = *call;
return true;
}
bool BluetoothPairing::CancelPairing() {
try {
if (pair_async_call_.isPending()) {
pair_async_call_.cancel();
}
device_->CancelPairing();
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to cancel pairing for device "
<< device_->getObjectPath();
return false;
if (pair_async_call_.isPending()) {
pair_async_call_.cancel();
}
return true;
return device_->CancelPairing();
}
bool BluetoothPairing::Unpair() {
try {
adapter_.RemoveDeviceByObjectPath(device_->getObjectPath());
return true;
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to unpair device "
<< device_->getObjectPath() << " on adapter "
<< adapter_.GetObjectPath();
return false;
}
return adapter_.RemoveDeviceByObjectPath(device_object_path_);
}
bool BluetoothPairing::IsPaired() {
try {
bool bonded = device_->Bonded();
return bonded;
} catch (const sdbus::Error &e) {
NEARBY_LOGS(ERROR) << __func__ << ": Got error '" << e.getName()
<< "' with message '" << e.getMessage()
<< "' while trying to get Bonded state for device "
<< device_->getObjectPath();
return false;
}
}
bool BluetoothPairing::IsPaired() { return device_->Bonded(); }
} // namespace linux
} // namespace nearby
@@ -31,7 +31,8 @@ namespace nearby {
namespace linux {
class BluetoothPairing final : public api::BluetoothPairing {
public:
BluetoothPairing(BluetoothAdapter &adapter, std::shared_ptr<BluetoothDevice> remote_device);
BluetoothPairing(BluetoothAdapter &adapter,
std::shared_ptr<BluetoothDevice> remote_device);
bool InitiatePairing(api::BluetoothPairingCallback pairing_cb) override;
bool FinishPairing(std::optional<absl::string_view> pin_code) override;
@@ -45,7 +46,8 @@ class BluetoothPairing final : public api::BluetoothPairing {
sdbus::PendingAsyncCall pair_async_call_;
std::shared_ptr<BluetoothDevice> device_;
linux::BluetoothAdapter &adapter_;
sdbus::ObjectPath device_object_path_;
linux::BluetoothAdapter adapter_;
api::BluetoothPairingCallback pairing_cb_;
};
@@ -39,16 +39,11 @@ AdvertisementMonitor::AdvertisementMonitor(
void AdvertisementMonitor::DeviceFound(const sdbus::ObjectPath &device) {
devices_->cleanup_lost_peripherals();
auto peripheral = devices_->add_new_device(device);
std::map<std::string, sdbus::Variant> service_data;
try {
service_data = peripheral->ServiceData();
} catch (const sdbus::Error &e) {
DBUS_LOG_PROPERTY_GET_ERROR(peripheral, "ServiceData", e);
return;
}
auto service_data = peripheral->ServiceData();
if (!service_data.has_value()) return;
struct api::ble_v2::BleAdvertisementData adv_data;
for (const auto &[uuid_str, data] : service_data) {
for (const auto &[uuid_str, data] : *service_data) {
auto uuid = UuidFromString(uuid_str);
if (!uuid.has_value()) {
NEARBY_LOGS(ERROR)