[Nearby Connections] Stop 3 AttemptsToConnect over BluetoothClassic for CancellationFlag

CancellationFlags will be used to prevent crashes during the shutdown of Nearby Connections from pending tasks taking too long during the shutdown period. By using CancellationFlags, we can prevent the retries occuring during the Shutdown by short-circuiting an in flight AttemptToConnect, and checking for Cancellation before retries. Because BluetoothClassic does not use futures, the short circuit only requires a check after ConnectToService, and returning an empty socket. PiperOrigin-RevId: 539755631
2026-09-15 07:06:11 -04:00 · 2023-06-12 14:05:36 -07:00
parent 240159e872
commit 6f69e97421
4 changed files with 302 additions and 46 deletions
@@ -18,6 +18,7 @@
 #include <string>
 #include <utility>

+#include "internal/platform/bluetooth_classic.h"
 #include "internal/platform/logging.h"
 #include "internal/platform/mutex_lock.h"
 #include "internal/platform/uuid.h"
@@ -40,7 +41,15 @@ std::string ScanModeToString(BluetoothAdapter::ScanMode mode) {
 }
 }  // namespace

-BluetoothClassic::BluetoothClassic(BluetoothRadio& radio) : radio_(radio) {}
+BluetoothClassic::BluetoothClassic(BluetoothRadio& radio)
+    : BluetoothClassic(radio, std::make_unique<BluetoothClassicMedium>(
+                                  radio.GetBluetoothAdapter())) {}
+
+BluetoothClassic::BluetoothClassic(
+    BluetoothRadio& radio, std::unique_ptr<BluetoothClassicMedium> medium)
+    : radio_(radio),
+      adapter_(radio_.GetBluetoothAdapter()),
+      medium_(std::move(medium)) {}

 BluetoothClassic::~BluetoothClassic() {
  // Destructor is not taking locks, but methods it is calling are.
@@ -63,7 +72,7 @@ bool BluetoothClassic::IsAvailable() const {
 }

 bool BluetoothClassic::IsAvailableLocked() const {
-  return medium_.IsValid() && adapter_.IsValid() && adapter_.IsEnabled();
+  return medium_->IsValid() && adapter_.IsValid() && adapter_.IsEnabled();
 }

 bool BluetoothClassic::TurnOnDiscoverability(const std::string& device_name) {
@@ -203,7 +212,7 @@ bool BluetoothClassic::StartDiscovery(DiscoveredDeviceCallback callback) {
    return false;
  }

-  if (!medium_.StartDiscovery(std::move(callback))) {
+  if (!medium_->StartDiscovery(std::move(callback))) {
    NEARBY_LOGS(INFO) << "Failed to start discovery of BT devices.";
    return false;
  }
@@ -223,7 +232,7 @@ bool BluetoothClassic::StopDiscovery() {
    return false;
  }

-  if (!medium_.StopDiscovery()) {
+  if (!medium_->StopDiscovery()) {
    NEARBY_LOGS(INFO) << "Failed to stop discovery of Bluetooth devices.";
    return false;
  }
@@ -264,7 +273,7 @@ bool BluetoothClassic::StartAcceptingConnections(
  }

  BluetoothServerSocket socket =
-      medium_.ListenForService(service_id, GenerateUuidFromString(service_id));
+      medium_->ListenForService(service_id, GenerateUuidFromString(service_id));
  if (!socket.IsValid()) {
    NEARBY_LOGS(INFO) << "Failed to start accepting Bluetooth connections for "
                      << service_id;
@@ -351,14 +360,31 @@ bool BluetoothClassic::StopAcceptingConnections(const std::string& service_id) {
 BluetoothSocket BluetoothClassic::Connect(BluetoothDevice& bluetooth_device,
                                          const std::string& service_id,
                                          CancellationFlag* cancellation_flag) {
-  for (int attempts_count = 0; attempts_count < kConnectAttemptsLimit;
-       attempts_count++) {
+  service_id_to_connect_attempts_count_map_[service_id] = 1;
+  while (service_id_to_connect_attempts_count_map_[service_id] <=
+         kConnectAttemptsLimit) {
+    if (cancellation_flag->Cancelled()) {
+      NEARBY_LOGS(WARNING)
+          << "Attempt #"
+          << service_id_to_connect_attempts_count_map_[service_id]
+          << ": Cannot start creating client BT socket due to cancel.";
+      return BluetoothSocket();
+    }
+
+    NEARBY_LOGS(INFO) << "Attempt #"
+                      << service_id_to_connect_attempts_count_map_[service_id]
+                      << " to connect.";
    auto wrapper_result =
        AttemptToConnect(bluetooth_device, service_id, cancellation_flag);
    if (wrapper_result.IsValid()) {
      return wrapper_result;
    }
+
+    service_id_to_connect_attempts_count_map_[service_id]++;
  }
+
+  NEARBY_LOGS(WARNING) << "Giving up after " << kConnectAttemptsLimit
+                       << " attempts";
  return BluetoothSocket();
 }

@@ -389,16 +415,16 @@ BluetoothSocket BluetoothClassic::AttemptToConnect(
    return socket;
  }

-  if (cancellation_flag->Cancelled()) {
-    NEARBY_LOGS(INFO) << "Can't create client BT socket due to cancel.";
-    return socket;
-  }
-
-  socket = medium_.ConnectToService(
+  socket = medium_->ConnectToService(
      bluetooth_device, GenerateUuidFromString(service_id), cancellation_flag);
-  if (!socket.IsValid()) {
+
+  // If the socket isn't valid or if the cancellation flag has fired during
+  // `ConnectToService`, return an empty socket. There is no need for a
+  // CancellationFlagListener because the attempt logic is not asynchronous.
+  if (!socket.IsValid() || cancellation_flag->Cancelled()) {
    NEARBY_LOGS(INFO) << "Failed to Connect via BT [service=" << service_id
                      << "]";
+    return BluetoothSocket();
  }

  return socket;
@@ -412,7 +438,7 @@ BluetoothDevice BluetoothClassic::GetRemoteDevice(
    return {};
  }

-  return medium_.GetRemoteDevice(mac_address);
+  return medium_->GetRemoteDevice(mac_address);
 }

 std::string BluetoothClassic::GetMacAddress() const {
@@ -422,7 +448,7 @@ std::string BluetoothClassic::GetMacAddress() const {
    return {};
  }

-  return medium_.GetMacAddress();
+  return medium_->GetMacAddress();
 }

 std::string BluetoothClassic::GenerateUuidFromString(const std::string& data) {
@@ -18,6 +18,8 @@
 #include <cstdint>
 #include <functional>
 #include <string>
+#include <memory>
+#include <map>

 #include "absl/container/flat_hash_map.h"
 #include "connections/implementation/mediums/bluetooth_radio.h"
@@ -43,7 +45,7 @@ class BluetoothClassic {
        accepted_cb = [](const std::string&, BluetoothSocket) {};
  };

-  explicit BluetoothClassic(BluetoothRadio& bluetooth_radio);
+  explicit BluetoothClassic(BluetoothRadio& radio);
  ~BluetoothClassic();

  // Returns true, if BT communications are supported by a platform.
@@ -97,7 +99,7 @@ class BluetoothClassic {
  // Returns true if this object owns a valid platform implementation.
  bool IsMediumValid() const ABSL_LOCKS_EXCLUDED(mutex_) {
    MutexLock lock(&mutex_);
-    return medium_.IsValid();
+    return medium_->IsValid();
  }

  // Returns true if this object has a valid BluetoothAdapter reference.
@@ -121,6 +123,15 @@ class BluetoothClassic {
  BluetoothDevice GetRemoteDevice(const std::string& mac_address)
      ABSL_LOCKS_EXCLUDED(mutex_);

+ protected:
+  // Use for unit tests only to inject a BluetoothClassicMedium.
+  BluetoothClassic(BluetoothRadio& radio,
+                   std::unique_ptr<BluetoothClassicMedium> medium);
+
+  // Used in unit tests to determine how many calls to `AttemptToConnect`
+  // occured during a call to `Connect`, per service id.
+  std::map<std::string, int> service_id_to_connect_attempts_count_map_;
+
 private:
  struct ScanInfo {
    bool valid = false;
@@ -175,9 +186,8 @@ class BluetoothClassic {

  mutable Mutex mutex_;
  BluetoothRadio& radio_ ABSL_GUARDED_BY(mutex_);
-  BluetoothAdapter& adapter_ ABSL_GUARDED_BY(mutex_){
-      radio_.GetBluetoothAdapter()};
-  BluetoothClassicMedium medium_ ABSL_GUARDED_BY(mutex_){adapter_};
+  BluetoothAdapter& adapter_ ABSL_GUARDED_BY(mutex_);
+  std::unique_ptr<BluetoothClassicMedium> medium_ ABSL_GUARDED_BY(mutex_);

  // A bundle of state required to do a Bluetooth Classic scan. When non-null,
  // we are currently performing a Bluetooth scan.
@@ -14,7 +14,9 @@

 #include "connections/implementation/mediums/bluetooth_classic.h"

+#include <memory>
 #include <string>
+#include <utility>

 #include "gmock/gmock.h"
 #include "protobuf-matchers/protocol-buffer-matchers.h"
@@ -44,6 +46,39 @@ constexpr FeatureFlags kTestCases[] = {

 constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000);

+class FakeBluetoothClassicMedium final : public BluetoothClassicMedium {
+ public:
+  explicit FakeBluetoothClassicMedium(BluetoothAdapter& adapter)
+      : BluetoothClassicMedium(adapter) {}
+
+  BluetoothSocket ConnectToService(
+      BluetoothDevice& remote_device, const std::string& service_uuid,
+      CancellationFlag* cancellation_flag) override {
+    if (cancel_) {
+      cancellation_flag->Cancel();
+    }
+
+    return BluetoothClassicMedium::ConnectToService(remote_device, service_uuid,
+                                                    cancellation_flag);
+  }
+
+  void CancelDuringConnectToService() { cancel_ = true; }
+
+ private:
+  bool cancel_ = false;
+};
+
+class TestBluetoothClassic : public BluetoothClassic {
+ public:
+  TestBluetoothClassic(BluetoothRadio& radio,
+                       std::unique_ptr<BluetoothClassicMedium> medium)
+      : BluetoothClassic(radio, std::move(medium)) {}
+
+  int connect_attempts_count(std::string service_id) {
+    return service_id_to_connect_attempts_count_map_[service_id];
+  }
+};
+
 class BluetoothClassicTest : public ::testing::TestWithParam<FeatureFlags> {
 protected:
  using DiscoveryCallback = BluetoothClassicMedium::DiscoveryCallback;
@@ -52,8 +87,16 @@ class BluetoothClassicTest : public ::testing::TestWithParam<FeatureFlags> {
    env_.Start();
    radio_a_ = std::make_unique<BluetoothRadio>();
    radio_b_ = std::make_unique<BluetoothRadio>();
-    bt_a_ = std::make_unique<BluetoothClassic>(*radio_a_);
-    bt_b_ = std::make_unique<BluetoothClassic>(*radio_b_);
+    auto medium_a = std::make_unique<FakeBluetoothClassicMedium>(
+        radio_a_->GetBluetoothAdapter());
+    auto medium_b = std::make_unique<FakeBluetoothClassicMedium>(
+        radio_b_->GetBluetoothAdapter());
+    medium_a_ = medium_a.get();
+    medium_b_ = medium_b.get();
+    bt_a_ =
+        std::make_unique<TestBluetoothClassic>(*radio_a_, std::move(medium_a));
+    bt_b_ =
+        std::make_unique<TestBluetoothClassic>(*radio_b_, std::move(medium_b));
    radio_a_->GetBluetoothAdapter().SetName("Device-A");
    radio_b_->GetBluetoothAdapter().SetName("Device-B");
    radio_a_->Enable();
@@ -77,8 +120,10 @@ class BluetoothClassicTest : public ::testing::TestWithParam<FeatureFlags> {

  std::unique_ptr<BluetoothRadio> radio_a_;
  std::unique_ptr<BluetoothRadio> radio_b_;
-  std::unique_ptr<BluetoothClassic> bt_a_;
-  std::unique_ptr<BluetoothClassic> bt_b_;
+  FakeBluetoothClassicMedium* medium_a_ = nullptr;
+  FakeBluetoothClassicMedium* medium_b_ = nullptr;
+  std::unique_ptr<TestBluetoothClassic> bt_a_;
+  std::unique_ptr<TestBluetoothClassic> bt_b_;
 };

 TEST_P(BluetoothClassicTest, CanConnect) {
@@ -86,7 +131,7 @@ TEST_P(BluetoothClassicTest, CanConnect) {
  env_.SetFeatureFlags(feature_flags);

  constexpr absl::string_view kDeviceName{"Simulated BT device #1"};
-  constexpr absl::string_view kServiceName{"service name"};
+  constexpr absl::string_view kServiceName1{"service name"};

  BluetoothRadio& radio_for_client = *radio_a_;
  BluetoothRadio& radio_for_server = *radio_b_;
@@ -116,7 +161,7 @@ TEST_P(BluetoothClassicTest, CanConnect) {
  BluetoothSocket socket_for_server;
  CountDownLatch accept_latch(1);
  EXPECT_TRUE(bt_server.StartAcceptingConnections(
-      std::string(kServiceName),
+      std::string(kServiceName1),
      {
          .accepted_cb =
              [&socket_for_server, &accept_latch](const std::string& service_id,
@@ -127,26 +172,26 @@ TEST_P(BluetoothClassicTest, CanConnect) {
      }));
  CancellationFlag flag;
  BluetoothSocket socket_for_client =
-      bt_client.Connect(discovered_device, std::string(kServiceName), &flag);
+      bt_client.Connect(discovered_device, std::string(kServiceName1), &flag);
  EXPECT_TRUE(accept_latch.Await(kWaitDuration).result());
-  EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName)));
+  EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
  EXPECT_TRUE(socket_for_server.IsValid());
  EXPECT_TRUE(socket_for_client.IsValid());
  EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid());
  EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid());
 }

-TEST_P(BluetoothClassicTest, CanCancelConnect) {
+TEST_P(BluetoothClassicTest, CanCancelBeforeConnect) {
  FeatureFlags feature_flags = GetParam();
  env_.SetFeatureFlags(feature_flags);

  constexpr absl::string_view kDeviceName{"Simulated BT device #1"};
-  constexpr absl::string_view kServiceName{"service name"};
+  constexpr absl::string_view kServiceName1{"service name"};

  BluetoothRadio& radio_for_client = *radio_a_;
  BluetoothRadio& radio_for_server = *radio_b_;
-  BluetoothClassic& bt_client = *bt_a_;
-  BluetoothClassic& bt_server = *bt_b_;
+  TestBluetoothClassic& bt_client = *bt_a_;
+  TestBluetoothClassic& bt_server = *bt_b_;

  EXPECT_TRUE(radio_for_client.IsEnabled());
  EXPECT_TRUE(radio_for_server.IsEnabled());
@@ -171,7 +216,7 @@ TEST_P(BluetoothClassicTest, CanCancelConnect) {
  BluetoothSocket socket_for_server;
  CountDownLatch accept_latch(1);
  EXPECT_TRUE(bt_server.StartAcceptingConnections(
-      std::string(kServiceName),
+      std::string(kServiceName1),
      {
          .accepted_cb =
              [&socket_for_server, &accept_latch](const std::string& service_id,
@@ -182,20 +227,196 @@ TEST_P(BluetoothClassicTest, CanCancelConnect) {
      }));
  CancellationFlag flag(true);
  BluetoothSocket socket_for_client =
-      bt_client.Connect(discovered_device, std::string(kServiceName), &flag);
+      bt_client.Connect(discovered_device, std::string(kServiceName1), &flag);
  // If FeatureFlag is disabled, Cancelled is false as no-op.
  if (!feature_flags.enable_cancellation_flag) {
    EXPECT_TRUE(accept_latch.Await(kWaitDuration).result());
-    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName)));
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
    EXPECT_TRUE(socket_for_server.IsValid());
    EXPECT_TRUE(socket_for_client.IsValid());
    EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid());
    EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid());
  } else {
    EXPECT_FALSE(accept_latch.Await(kWaitDuration).result());
-    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName)));
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
    EXPECT_FALSE(socket_for_server.IsValid());
    EXPECT_FALSE(socket_for_client.IsValid());
+
+    // Expect an invalid socket from stopping during the first attempt to
+    // connect, because `Connect` returned immediatley when it checked for
+    // cancellation.
+    EXPECT_EQ(1, bt_client.connect_attempts_count(std::string(kServiceName1)));
+  }
+}
+
+TEST_P(BluetoothClassicTest, CanCancelDuringConnect) {
+  FeatureFlags feature_flags = GetParam();
+  env_.SetFeatureFlags(feature_flags);
+
+  constexpr absl::string_view kDeviceName{"Simulated BT device #1"};
+  constexpr absl::string_view kServiceName1{"service name"};
+
+  BluetoothRadio& radio_for_client = *radio_a_;
+  BluetoothRadio& radio_for_server = *radio_b_;
+  TestBluetoothClassic& bt_client = *bt_a_;
+  TestBluetoothClassic& bt_server = *bt_b_;
+
+  // Simulate the flag being cancelled during connection attempt.
+  medium_a_->CancelDuringConnectToService();
+
+  EXPECT_TRUE(radio_for_client.IsEnabled());
+  EXPECT_TRUE(radio_for_server.IsEnabled());
+
+  EXPECT_TRUE(bt_server.TurnOnDiscoverability(std::string(kDeviceName)));
+  EXPECT_EQ(radio_for_server.GetBluetoothAdapter().GetName(),
+            std::string(kDeviceName));
+  CountDownLatch latch(1);
+  BluetoothDevice discovered_device;
+  EXPECT_TRUE(bt_client.StartDiscovery({
+      .device_discovered_cb =
+          [&latch, &discovered_device](BluetoothDevice& device) {
+            discovered_device = device;
+            NEARBY_LOG(INFO, "Discovered device=%p [impl=%p]", &device,
+                       &device.GetImpl());
+            latch.CountDown();
+          },
+  }));
+  EXPECT_TRUE(latch.Await(kWaitDuration).result());
+  EXPECT_TRUE(bt_server.TurnOffDiscoverability());
+  ASSERT_TRUE(discovered_device.IsValid());
+  BluetoothSocket socket_for_server;
+  CountDownLatch accept_latch(1);
+  EXPECT_TRUE(bt_server.StartAcceptingConnections(
+      std::string(kServiceName1),
+      {
+          .accepted_cb =
+              [&socket_for_server, &accept_latch](const std::string& service_id,
+                                                  BluetoothSocket socket) {
+                socket_for_server = std::move(socket);
+                accept_latch.CountDown();
+              },
+      }));
+  CancellationFlag flag;
+  BluetoothSocket socket_for_client =
+      bt_client.Connect(discovered_device, std::string(kServiceName1), &flag);
+  // If FeatureFlag is disabled, Cancelled is false as no-op.
+  if (!feature_flags.enable_cancellation_flag) {
+    EXPECT_TRUE(accept_latch.Await(kWaitDuration).result());
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
+    EXPECT_TRUE(socket_for_server.IsValid());
+    EXPECT_TRUE(socket_for_client.IsValid());
+    EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid());
+    EXPECT_TRUE(socket_for_client.GetRemoteDevice().IsValid());
+  } else {
+    EXPECT_FALSE(accept_latch.Await(kWaitDuration).result());
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
+    EXPECT_FALSE(socket_for_server.IsValid());
+    EXPECT_FALSE(socket_for_client.IsValid());
+
+    // Since the flag was cancelled during the initial `AttemptToConnect`,
+    // except only one attempt instead of the usual three, because the
+    // cancellation flag should short-circuit the lengthy connection attempts
+    // during shutdown. Because of the way the iteration happens, the check for
+    // is cancelled happens after the counter has already been incremented, but
+    // before the attempt actually occurs.
+    EXPECT_EQ(2, bt_client.connect_attempts_count(std::string(kServiceName1)));
+  }
+}
+
+TEST_P(BluetoothClassicTest, CanCancelDuringConnect_MultipleEndpoints) {
+  FeatureFlags feature_flags = GetParam();
+  env_.SetFeatureFlags(feature_flags);
+
+  constexpr absl::string_view kDeviceName{"Simulated BT device #1"};
+  constexpr absl::string_view kServiceName1{"service name"};
+  constexpr absl::string_view kServiceName2{"anotherservice name"};
+
+  BluetoothRadio& radio_for_client = *radio_a_;
+  BluetoothRadio& radio_for_server = *radio_b_;
+  TestBluetoothClassic& bt_client = *bt_a_;
+  TestBluetoothClassic& bt_server = *bt_b_;
+  EXPECT_TRUE(radio_for_client.IsEnabled());
+  EXPECT_TRUE(radio_for_server.IsEnabled());
+
+  EXPECT_TRUE(bt_server.TurnOnDiscoverability(std::string(kDeviceName)));
+  EXPECT_EQ(radio_for_server.GetBluetoothAdapter().GetName(),
+            std::string(kDeviceName));
+  CountDownLatch latch(1);
+  BluetoothDevice discovered_device;
+  EXPECT_TRUE(bt_client.StartDiscovery({
+      .device_discovered_cb =
+          [&latch, &discovered_device](BluetoothDevice& device) {
+            discovered_device = device;
+            NEARBY_LOG(INFO, "Discovered device=%p [impl=%p]", &device,
+                       &device.GetImpl());
+            latch.CountDown();
+          },
+  }));
+  EXPECT_TRUE(latch.Await(kWaitDuration).result());
+  EXPECT_TRUE(bt_server.TurnOffDiscoverability());
+  ASSERT_TRUE(discovered_device.IsValid());
+  BluetoothSocket socket_for_server;
+  CountDownLatch accept_latch(1);
+
+  EXPECT_TRUE(bt_server.StartAcceptingConnections(
+      std::string(kServiceName1),
+      {
+          .accepted_cb =
+              [&socket_for_server, &accept_latch](const std::string& service_id,
+                                                  BluetoothSocket socket) {
+                socket_for_server = std::move(socket);
+                accept_latch.CountDown();
+              },
+      }));
+  CancellationFlag flag;
+  BluetoothSocket socket_for_client1 =
+      bt_client.Connect(discovered_device, std::string(kServiceName1), &flag);
+
+  // Simulate the flag being cancelled during connection attempt to a different
+  // endpoint.
+  medium_a_->CancelDuringConnectToService();
+  EXPECT_TRUE(bt_server.StartAcceptingConnections(
+      std::string(kServiceName2),
+      {
+          .accepted_cb =
+              [&socket_for_server, &accept_latch](const std::string& service_id,
+                                                  BluetoothSocket socket) {
+                socket_for_server = std::move(socket);
+                accept_latch.CountDown();
+              },
+      }));
+
+  BluetoothSocket socket_for_client2 =
+      bt_client.Connect(discovered_device, std::string(kServiceName2), &flag);
+
+  // If FeatureFlag is disabled, Cancelled is false as no-op.
+  if (!feature_flags.enable_cancellation_flag) {
+    EXPECT_TRUE(accept_latch.Await(kWaitDuration).result());
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName2)));
+    EXPECT_TRUE(socket_for_server.IsValid());
+    EXPECT_TRUE(socket_for_client1.IsValid());
+    EXPECT_TRUE(socket_for_client2.IsValid());
+    EXPECT_TRUE(socket_for_server.GetRemoteDevice().IsValid());
+    EXPECT_TRUE(socket_for_client1.GetRemoteDevice().IsValid());
+    EXPECT_TRUE(socket_for_client2.GetRemoteDevice().IsValid());
+  } else {
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
+    EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName2)));
+    EXPECT_TRUE(socket_for_client1.IsValid());
+    EXPECT_FALSE(socket_for_client2.IsValid());
+
+    // Since the flag was cancelled during the initial `AttemptToConnect`,
+    // except only one attempt instead of the usual three, because the
+    // cancellation flag should short-circuit the lengthy connection attempts
+    // during shutdown. Because of the way the iteration happens, the check for
+    // is cancelled happens after the counter has already been incremented, but
+    // before the attempt actually occurs.
+    EXPECT_EQ(2, bt_client.connect_attempts_count(std::string(kServiceName2)));
+
+    // With the first service name, we expect one attempt count since it
+    // succeeded.
+    EXPECT_EQ(1, bt_client.connect_attempts_count(std::string(kServiceName1)));
  }
 }

@@ -250,7 +471,7 @@ TEST_F(BluetoothClassicTest, CanStopDiscovery) {

 TEST_F(BluetoothClassicTest, CanStartAcceptingConnections) {
  constexpr absl::string_view kDeviceName{"Simulated BT device #1"};
-  constexpr absl::string_view kServiceName{"service name"};
+  constexpr absl::string_view kServiceName1{"service name"};

  BluetoothRadio& radio_for_client = *radio_a_;
  BluetoothRadio& radio_for_server = *radio_b_;
@@ -277,11 +498,11 @@ TEST_F(BluetoothClassicTest, CanStartAcceptingConnections) {
  EXPECT_TRUE(bt_server.TurnOffDiscoverability());
  EXPECT_TRUE(discovered_device.IsValid());
  EXPECT_TRUE(
-      bt_server.StartAcceptingConnections(std::string(kServiceName), {}));
+      bt_server.StartAcceptingConnections(std::string(kServiceName1), {}));
  // Allow StartAcceptingConnections do something, before stopping it.
  // This is best effort, because no callbacks are invoked in this scenario.
  SystemClock::Sleep(kWaitDuration);
-  EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName)));
+  EXPECT_TRUE(bt_server.StopAcceptingConnections(std::string(kServiceName1)));
 }

 }  // namespace
@@ -69,7 +69,7 @@ class BluetoothSocket final {
  // Socket created by a default public constructor is not valid, because
  // it is missing platform implementation.
  // The only way to obtain a valid socket is through connection, such as
-  // an object returned by either BluetoothClassicMedium::ConnectTotService or
+  // an object returned by either BluetoothClassicMedium::ConnectToService or
  // BluetoothServerSocket::Accept().
  // These methods may also return an invalid socket if connection failed for
  // any reason.
@@ -160,8 +160,7 @@ class BluetoothPairing final {

 // Container of operations that can be performed over the Bluetooth Classic
 // medium.
-class BluetoothClassicMedium final
-    : public api::BluetoothClassicMedium::Observer {
+class BluetoothClassicMedium : public api::BluetoothClassicMedium::Observer {
 public:
  using Platform = api::ImplementationPlatform;
  struct DiscoveryCallback {
@@ -252,9 +251,9 @@ class BluetoothClassicMedium final
  //
  // Returns a new BluetoothSocket. On Success, BluetoothSocket::IsValid()
  // returns true.
-  BluetoothSocket ConnectToService(BluetoothDevice& remote_device,
-                                   const std::string& service_uuid,
-                                   CancellationFlag* cancellation_flag);
+  virtual BluetoothSocket ConnectToService(BluetoothDevice& remote_device,
+                                           const std::string& service_uuid,
+                                           CancellationFlag* cancellation_flag);

  // https://developer.android.com/reference/android/bluetooth/BluetoothAdapter.html#listenUsingInsecureRfcommWithServiceRecord
  //