diff --git a/connections/implementation/base_pcp_handler.cc b/connections/implementation/base_pcp_handler.cc index 15a7d566..dfc043a3 100644 --- a/connections/implementation/base_pcp_handler.cc +++ b/connections/implementation/base_pcp_handler.cc @@ -136,6 +136,12 @@ void BasePcpHandler::Shutdown() { // Stop all the ongoing Runnables (as gracefully as possible). NEARBY_LOGS(INFO) << "BasePcpHandler(" << strategy_.GetName() << ") is bringing down executors."; + + encryption_runner_.Shutdown(); + + // Stop discovery of Bluetooth Classic. + mediums_->GetBluetoothClassic().StopAllDiscovery(); + serial_executor_.Shutdown(); alarm_executor_.Shutdown(); NEARBY_LOGS(INFO) << "BasePcpHandler(" << strategy_.GetName() @@ -513,6 +519,12 @@ Status BasePcpHandler::WaitForResult(const std::string& method_name, void BasePcpHandler::RunOnPcpHandlerThread(const std::string& name, Runnable runnable) { + if (closed_.Get()) { + NEARBY_LOGS(WARNING) << "Skip to run PCP Handler task " << name + << " due to PCP Handler is closed"; + return; + } + serial_executor_.Execute(name, std::move(runnable)); } diff --git a/connections/implementation/base_pcp_handler.h b/connections/implementation/base_pcp_handler.h index 5a3c5560..042b8768 100644 --- a/connections/implementation/base_pcp_handler.h +++ b/connections/implementation/base_pcp_handler.h @@ -647,6 +647,7 @@ class BasePcpHandler : public PcpHandler, void OptionsAllowed(const BooleanMediumSelector& allowed, std::ostringstream& result) const; + AtomicBoolean closed_{false}; ScheduledExecutor alarm_executor_; SingleThreadExecutor serial_executor_; Mutex discovered_endpoint_mutex_; @@ -682,7 +683,6 @@ class BasePcpHandler : public PcpHandler, Strategy strategy_{PcpToStrategy(pcp_)}; EncryptionRunner encryption_runner_; BwuManager* bwu_manager_; - AtomicBoolean closed_{false}; }; } // namespace connections diff --git a/connections/implementation/encryption_runner.cc b/connections/implementation/encryption_runner.cc index 43146980..6cbfee83 100644 --- a/connections/implementation/encryption_runner.cc +++ b/connections/implementation/encryption_runner.cc @@ -31,6 +31,7 @@ #include "internal/platform/cancelable_alarm.h" #include "internal/platform/exception.h" #include "internal/platform/logging.h" +#include "internal/platform/scheduled_executor.h" namespace nearby { namespace connections { @@ -356,12 +357,7 @@ class ClientRunnable final { } // namespace -EncryptionRunner::~EncryptionRunner() { - // Stop all the ongoing Runnables (as gracefully as possible). - client_executor_.Shutdown(); - server_executor_.Shutdown(); - alarm_executor_.Shutdown(); -} +EncryptionRunner::~EncryptionRunner() { Shutdown(); } void EncryptionRunner::StartServer(ClientProxy* client, const std::string& endpoint_id, @@ -381,6 +377,17 @@ void EncryptionRunner::StartClient(ClientProxy* client, client_executor_.Execute("encryption-client", std::move(runnable)); } +void EncryptionRunner::Shutdown() { + if (is_stopped_.Set(true)) { + return; + } + + // Stop all the ongoing Runnables (as gracefully as possible). + client_executor_.Shutdown(); + server_executor_.Shutdown(); + alarm_executor_.Shutdown(); +} + void EncryptionRunner::ResultListener::CallSuccessCallback( const std::string& endpoint_id, std::unique_ptr ukey2, diff --git a/connections/implementation/encryption_runner.h b/connections/implementation/encryption_runner.h index 7c2b401f..e8e8c186 100644 --- a/connections/implementation/encryption_runner.h +++ b/connections/implementation/encryption_runner.h @@ -15,13 +15,14 @@ #ifndef CORE_INTERNAL_ENCRYPTION_RUNNER_H_ #define CORE_INTERNAL_ENCRYPTION_RUNNER_H_ +#include #include #include "securegcm/ukey2_handshake.h" #include "absl/functional/any_invocable.h" #include "connections/implementation/client_proxy.h" #include "connections/implementation/endpoint_channel.h" -#include "connections/listeners.h" +#include "internal/platform/atomic_boolean.h" #include "internal/platform/byte_array.h" #include "internal/platform/scheduled_executor.h" #include "internal/platform/single_thread_executor.h" @@ -78,7 +79,11 @@ class EncryptionRunner { EndpointChannel* endpoint_channel, ResultListener result_listener); + // @AnyThread + void Shutdown(); + private: + AtomicBoolean is_stopped_{false}; ScheduledExecutor alarm_executor_; SingleThreadExecutor server_executor_; SingleThreadExecutor client_executor_; diff --git a/connections/implementation/mediums/bluetooth_classic.h b/connections/implementation/mediums/bluetooth_classic.h index dd60c035..4115af09 100644 --- a/connections/implementation/mediums/bluetooth_classic.h +++ b/connections/implementation/mediums/bluetooth_classic.h @@ -75,6 +75,9 @@ class BluetoothClassic { // if it is the last call to stop discovery, will disable BT discovery mode. bool StopDiscovery(const std::string& serviceId) ABSL_LOCKS_EXCLUDED(mutex_); + // Stops BT discovery for all services. + void StopAllDiscovery() ABSL_LOCKS_EXCLUDED(mutex_); + // Starts a worker thread, creates a BT server socket, associates it with a // service ID; in a worker thread repeatedly calls ServerSocket::Accept(). // Any connected sockets returned from Accept() are passed to a callback. @@ -177,8 +180,6 @@ class BluetoothClassic { bool IsDiscoveringLocked(const std::string& serviceId) const ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); - void StopAllDiscovery() ABSL_LOCKS_EXCLUDED(mutex_); - // Establishes connection to BT service that was might be started on another // device with StartAcceptingConnections() using the same service_id. // Blocks until connection is established, or server-side is terminated. @@ -234,12 +235,10 @@ class BluetoothClassic { // Whether the multiplex feature is enabled. bool is_multiplex_enabled_ = NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_connections_feature:: - kEnableMultiplex); + config_package_nearby::nearby_connections_feature::kEnableMultiplex); // A map of Bluetooth MacAddress -> MultiplexSocket. - absl::flat_hash_map + absl::flat_hash_map multiplex_sockets_ ABSL_GUARDED_BY(mutex_); }; diff --git a/internal/platform/bluetooth_classic.cc b/internal/platform/bluetooth_classic.cc index 20d0e305..2fc38fd5 100644 --- a/internal/platform/bluetooth_classic.cc +++ b/internal/platform/bluetooth_classic.cc @@ -99,6 +99,7 @@ bool BluetoothClassicMedium::StartDiscovery(DiscoveryCallback callback) { [this](api::BluetoothDevice& device) { VLOG(1) << "BT .device_discovered_cb for " << device.GetName(); MutexLock lock(&mutex_); + if (!discovery_enabled_) return; auto pair = devices_.emplace( &device, std::make_unique()); auto& context = *pair.first->second; @@ -111,26 +112,26 @@ bool BluetoothClassicMedium::StartDiscovery(DiscoveryCallback callback) { context.device = BluetoothDevice(&device); LOG(INFO) << "Adding device=" << context.device.GetMacAddress() << ",impl=" << &device; - if (!discovery_enabled_) return; discovery_callback_.device_discovered_cb(context.device); }, .device_name_changed_cb = [this](api::BluetoothDevice& device) { VLOG(1) << "BT .device_name_changed_cb for " << device.GetName(); MutexLock lock(&mutex_); + if (!discovery_enabled_) return; // If the device is not already in devices_, we should not be able // to change its name. if (devices_.find(&device) == devices_.end()) return; auto& context = *devices_[&device]; LOG(INFO) << "Renaming device=" << context.device.GetMacAddress() << ",impl=" << &device; - if (!discovery_enabled_) return; discovery_callback_.device_name_changed_cb(context.device); }, .device_lost_cb = [this](api::BluetoothDevice& device) { VLOG(1) << "BT .device_lost_cb for " << device.GetMacAddress(); MutexLock lock(&mutex_); + if (!discovery_enabled_) return; auto item = devices_.extract(&device); if (!item) { LOG(WARNING) << "Removing unknown device: " @@ -140,7 +141,6 @@ bool BluetoothClassicMedium::StartDiscovery(DiscoveryCallback callback) { auto& context = *item.mapped(); LOG(INFO) << "Removing device=" << context.device.GetMacAddress() << ",impl=" << &device; - if (!discovery_enabled_) return; discovery_callback_.device_lost_cb(context.device); }, }); diff --git a/internal/platform/implementation/windows/bluetooth_classic_medium.cc b/internal/platform/implementation/windows/bluetooth_classic_medium.cc index 707e2ddd..a012a468 100644 --- a/internal/platform/implementation/windows/bluetooth_classic_medium.cc +++ b/internal/platform/implementation/windows/bluetooth_classic_medium.cc @@ -191,6 +191,7 @@ bool BluetoothClassicMedium::StopDiscovery() { if (IsWatcherStarted()) { result = StopScanning(); + discovery_callback_ = {}; } return result; @@ -659,8 +660,10 @@ winrt::fire_and_forget BluetoothClassicMedium::DeviceWatcher_Updated( LOG(INFO) << "Updated device name:" << mac_address_to_bluetooth_device_map_[mac_address]->GetName(); - discovery_callback_.device_name_changed_cb( - *mac_address_to_bluetooth_device_map_[mac_address]); + if (discovery_callback_.device_name_changed_cb != nullptr) { + discovery_callback_.device_name_changed_cb( + *mac_address_to_bluetooth_device_map_[mac_address]); + } } }