Fixed race conditions with simultaneous connections

internal/platform/implementation/linux/bluetooth_bluez_profile.cc

@@ -174,7 +174,7 @@ void ProfileManager::Unregister(absl::string_view service_uuid) {
 }
 
 // Get a service record FD for a connected profile (identified by service_uuid)
-// to the given device.
+// to the given device. Only fires when we're requesting a new connection. i.e: we're the client
 std::optional<sdbus::UnixFd> ProfileManager::GetServiceRecordFD(
     api::BluetoothDevice &remote_device, absl::string_view service_uuid,
     CancellationFlag *cancellation_flag) {
@@ -210,7 +210,7 @@ std::optional<sdbus::UnixFd> ProfileManager::GetServiceRecordFD(
             << " key=" << mac_addr;
   auto cond = [mac_addr, profile, cancellation_flag]() {
     profile->connections_lock_.AssertHeld();
-    LOG(INFO) << "connections_lock_ is held by: " << mac_addr;
+    LOG(INFO) << "connections_lock_ is held by: " << mac_addr << " with ptr: " << &profile -> connections_lock_;
     return profile->connections_.count(mac_addr) != 0 ||
            (cancellation_flag != nullptr && cancellation_flag->Cancelled());
   };
@@ -222,6 +222,10 @@ std::optional<sdbus::UnixFd> ProfileManager::GetServiceRecordFD(
                                    absl::Condition(&cond));
   LOG(INFO) << "WAIT_ACQUIRED "
             << " map_size=" << profile->connections_.size();
+
+  // Clean up pending tracking
+  profile->pending_outgoing_.erase(mac_addr);
+
   if (cancellation_flag != nullptr && cancellation_flag->Cancelled()) {
     LOG(INFO)
         << __func__ << ": " << profile->getObjectPath() << ": "
@@ -242,7 +246,7 @@ std::optional<sdbus::UnixFd> ProfileManager::GetServiceRecordFD(
 }
 
 // Listen for a connected profile on any device, returning the connected device
-// with its FD.
+// with its FD. Only fires when another device requests connection from us. i.e. we're the server
 std::optional<std::pair<std::shared_ptr<BluetoothDevice>, sdbus::UnixFd>>
 ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
                                    CancellationFlag *cancellation_flag) {
@@ -272,8 +276,15 @@ ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
   profile->connections_lock_.Lock();
   auto cond = [profile, &cancellation_flag]() {
     profile->connections_lock_.AssertReaderHeld();
-    return !profile->connections_.empty() ||
-           (cancellation_flag != nullptr && cancellation_flag->Cancelled());
+
+    // Only accept connections that DON'T have pending outgoing attempts
+    for (const auto& [mac, fds] : profile->connections_) {
+      if (profile->pending_outgoing_.count(mac) == 0) {
+        return true; // Found a connection without pending outgoing
+      }
+    }
+
+    return cancellation_flag != nullptr && cancellation_flag->Cancelled();
   };
   profile->connections_lock_.Await(absl::Condition(&cond));
 
@@ -285,13 +296,35 @@ ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
     return std::nullopt;
   }
 
-  auto it = profile->connections_.begin();
-  auto mac_addr = it->first;
-  auto [fd, properties] = it->second.back();
-  it->second.pop_back();
-  if (it->second.empty()) profile->connections_.erase(it);
+  // Find first connection without pending outgoing
+  std::string mac_addr;
+  sdbus::UnixFd fd;
+  bool found = false;
+
+  for (auto it = profile->connections_.begin(); it != profile->connections_.end(); ++it) {
+    if (profile->pending_outgoing_.count(it->first) == 0) {
+      mac_addr = it->first;
+      auto& fds = it->second;
+      // Use auto to avoid accessing private FDProperties type
+      auto [fd_tmp, properties] = fds.back();
+      fd = std::move(fd_tmp);
+      fds.pop_back();
+      if (fds.empty()) {
+        profile->connections_.erase(it);
+      }
+      found = true;
+      break;
+    }
+  }
+
+  LOG(INFO) << __func__ << " Cleared connections";
   profile->connections_lock_.Unlock();
 
+  if (!found) {
+    LOG(ERROR) << __func__ << ": No eligible connection found";
+    return std::nullopt;
+  }
+
   auto device = devices_.get_device_by_address(mac_addr);
   if (device == nullptr) {
     LOG(ERROR) << __func__ << ": Device " << mac_addr
@@ -301,6 +334,30 @@ ProfileManager::GetServiceRecordFD(absl::string_view service_uuid,
 
   return std::pair(device, std::move(fd));
 }
+  void ProfileManager::MarkPendingOutgoing(absl::string_view service_uuid,
+                                                const std::string& mac_address) {
+  absl::ReaderMutexLock lock(&registered_service_uuids_mutex_);
+  if (registered_services_.count(std::string(service_uuid)) == 0) {
+    return;
+  }
+  auto profile = registered_services_[std::string(service_uuid)];
+  absl::MutexLock l(&profile->connections_lock_);
+  profile->pending_outgoing_.insert(mac_address);
+  LOG(INFO) << __func__ << ": Marked " << mac_address
+            << " as pending outgoing for " << service_uuid;
+}
 
+  void ProfileManager::ClearPendingOutgoing(absl::string_view service_uuid,
+                                            const std::string& mac_address) {
+  absl::ReaderMutexLock lock(&registered_service_uuids_mutex_);
+  if (registered_services_.count(std::string(service_uuid)) == 0) {
+    return;
+  }
+  auto profile = registered_services_[std::string(service_uuid)];
+  absl::MutexLock l(&profile->connections_lock_);
+  profile->pending_outgoing_.erase(mac_address);
+  LOG(INFO) << __func__ << ": Cleared " << mac_address
+            << " as pending outgoing for " << service_uuid;
+}
 }  // namespace linux
 }  // namespace nearby

internal/platform/implementation/linux/bluetooth_bluez_profile.h

@@ -96,6 +96,9 @@ class Profile final
   std::map<std::string, std::vector<std::pair<sdbus::UnixFd, FDProperties>>>
       connections_ ABSL_GUARDED_BY(connections_lock_);
 
+  // Track pending outgoing connection attempts to avoid race with incoming
+  std::set<std::string> pending_outgoing_ ABSL_GUARDED_BY(connections_lock_);
+
   BluetoothDevices &devices_;
 };
 
@@ -133,7 +136,12 @@ class ProfileManager final
   GetServiceRecordFD(absl::string_view service_uuid,
                      CancellationFlag *cancellation_flag)
       ABSL_LOCKS_EXCLUDED(registered_service_uuids_mutex_);
-
+  void MarkPendingOutgoing(absl::string_view service_uuid,
+                                  const std::string& mac_address)
+             ABSL_LOCKS_EXCLUDED(registered_service_uuids_mutex_);
+  void ClearPendingOutgoing(absl::string_view service_uuid,
+                            const std::string& mac_address)
+      ABSL_LOCKS_EXCLUDED(registered_service_uuids_mutex_);
  private:
   BluetoothDevices &devices_;
   // Maps service UUIDs to RegisteredService

internal/platform/implementation/linux/bluetooth_classic_medium.cc

@@ -25,6 +25,8 @@
 #include "internal/platform/implementation/linux/bluetooth_bluez_profile.h"
 #include "internal/platform/implementation/linux/bluetooth_classic_device.h"
 #include "internal/platform/implementation/linux/bluetooth_classic_medium.h"
+
+#include "bluez_agent.h"
 #include "internal/platform/implementation/linux/bluetooth_classic_server_socket.h"
 #include "internal/platform/implementation/linux/bluetooth_classic_socket.h"
 #include "internal/platform/implementation/linux/bluetooth_pairing.h"
@@ -39,6 +41,7 @@ BluetoothClassicMedium::BluetoothClassicMedium(BluetoothAdapter &adapter)
       devices_(std::make_shared<BluetoothDevices>(
           system_bus_, adapter.GetObjectPath(), *observers_)),
       device_watcher_(nullptr),
+      agent_manager_(std::make_unique<AgentManager>(*system_bus_)),
       profile_manager_(
           std::make_unique<ProfileManager>(*system_bus_, *devices_)) {}
 
@@ -103,16 +106,24 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
     }
   }
 
-  // who is passing this here?
-  auto address = remote_device.GetMacAddress(); //BUG: this returns the last known name instead of mac address
-  auto device = devices_->get_device_by_address(address); //BUG: this returns nullptr. WHy? who knows
+  auto address = remote_device.GetMacAddress();
+  auto device = devices_->get_device_by_address(address);
   if (device == nullptr) {
     LOG(ERROR) << __func__ << ": Device " << address
                        << " is no longer known";
     return nullptr;
   }
+  if (!device -> Bonded())
+  {
+
+    LOG(ERROR) << __func__ << ": Device " << address
+                       << " is not Bonded";
+  }
+  // Mark as pending BEFORE calling ConnectToProfile to win the race
+  profile_manager_->MarkPendingOutgoing(service_uuid, address);
 
   if (!device->ConnectToProfile(service_uuid)) {
+    profile_manager_->ClearPendingOutgoing(service_uuid, address);
     return nullptr;
   }
 
@@ -132,6 +143,17 @@ std::unique_ptr<api::BluetoothSocket> BluetoothClassicMedium::ConnectToService(
 std::unique_ptr<api::BluetoothServerSocket>
 BluetoothClassicMedium::ListenForService(const std::string &service_name,
                                          const std::string &service_uuid) {
+  LOG(INFO) << __func__ << ": Creating bluez agent on path: " << "/com/example/bluez_agent" ;
+
+  if (!agent_manager_ -> AgentRegistered("/com/example/bluez_agent"))
+  {
+    if (!agent_manager_ -> Register(std::nullopt, "/com/example/bluez_agent"))
+    {
+      LOG(ERROR) << __func__ << ": Could not register agent " << service_name << " "
+      << service_uuid;
+      return nullptr;
+    }
+  }
   if (!profile_manager_->ProfileRegistered(service_uuid)) {
     if (!profile_manager_->Register(service_name, service_uuid)) {
       LOG(ERROR) << __func__ << ": Could not register profile "

internal/platform/implementation/linux/bluetooth_classic_medium.h

@@ -26,6 +26,7 @@
 #include <sdbus-c++/StandardInterfaces.h>
 #include <sdbus-c++/Types.h>
 
+#include "bluez_agent.h"
 #include "internal/base/observer_list.h"
 #include "internal/platform/implementation/bluetooth_classic.h"
 #include "internal/platform/implementation/linux/bluetooth_adapter.h"
@@ -34,7 +35,7 @@
 
 namespace nearby {
 namespace linux {
-// Container of operations that can be performed over the Bluetooth Classic
+  // Container of operations that can be performed over the Bluetooth Classic
 // medium.
 class BluetoothClassicMedium : public api::BluetoothClassicMedium {
  public:
@@ -106,6 +107,7 @@ class BluetoothClassicMedium : public api::BluetoothClassicMedium {
   std::shared_ptr<BluetoothDevices> devices_;
   std::unique_ptr<DeviceWatcher> device_watcher_;
 
+  std::unique_ptr<AgentManager> agent_manager_;
   std::unique_ptr<ProfileManager> profile_manager_;
 };
 

internal/platform/implementation/linux/bluetooth_classic_server_socket.cc

@@ -41,6 +41,7 @@ std::unique_ptr<api::BluetoothSocket> BluetoothServerSocket::Accept() {
   }
 
   auto [device, fd] = *pair;
+  LOG(INFO) << __func__ << ": accepted incoming connection for service uuid " << service_uuid_;
   return std::make_unique<BluetoothSocket>(device, std::move(fd));
 }
 

internal/platform/implementation/linux/bluetooth_devices.cc

@@ -173,6 +173,7 @@ void DeviceWatcher::notifyExistingDevices() {
       std::find_if(objects.begin(), objects.end(), [&](auto entry) {
         auto &[device_path, interfaces] = entry;
 
+
         return device_path.find(
                    absl::Substitute("$0/dev_", adapter_object_path_)) == 0 &&
                interfaces.count(org::bluez::Device1_proxy::INTERFACE_NAME) == 1;