From e8205154e6c31edf0bb90428724fc74acce77375 Mon Sep 17 00:00:00 2001 From: hai007 Date: Fri, 5 Mar 2021 11:47:11 -0800 Subject: [PATCH] Internal change PiperOrigin-RevId: 361192393 --- cpp/core/internal/bwu_manager.cc | 78 +++++++------------ cpp/core/internal/endpoint_channel_manager.cc | 4 +- .../internal/service_controller_router.cc | 21 ++--- cpp/platform/base/feature_flags.h | 8 -- 4 files changed, 33 insertions(+), 78 deletions(-) diff --git a/cpp/core/internal/bwu_manager.cc b/cpp/core/internal/bwu_manager.cc index cf2ad287..4a21bd1c 100644 --- a/cpp/core/internal/bwu_manager.cc +++ b/cpp/core/internal/bwu_manager.cc @@ -139,11 +139,7 @@ void BwuManager::InitiateBwuForEndpoint(ClientProxy* client, if (in_progress_upgrades_.contains(endpoint_id)) { return; } - if (FeatureFlags::GetInstance() - .GetFlags() - .disallow_out_of_order_bwu_avail_event) { - CancelRetryUpgradeAlarm(endpoint_id); - } + CancelRetryUpgradeAlarm(endpoint_id); auto channel = channel_manager_->GetChannelForEndpoint(endpoint_id); @@ -335,17 +331,9 @@ void BwuManager::OnIncomingConnection( const std::string& endpoint_id = introduction.endpoint_id(); ClientProxy* mapped_client; - if (FeatureFlags::GetInstance() - .GetFlags() - .disallow_out_of_order_bwu_avail_event) { - const auto item = in_progress_upgrades_.find(endpoint_id); - if (item == in_progress_upgrades_.end()) return; - mapped_client = item->second; - } else { - auto item = in_progress_upgrades_.extract(endpoint_id); - if (item.empty()) return; - mapped_client = item.mapped(); - } + const auto item = in_progress_upgrades_.find(endpoint_id); + if (item == in_progress_upgrades_.end()) return; + mapped_client = item->second; CancelRetryUpgradeAlarm(endpoint_id); if (mapped_client == nullptr) { // This was never a fully EstablishedConnection, no need to provide a @@ -418,34 +406,30 @@ void BwuManager::ProcessBwuPathAvailableEvent( if (in_progress_upgrades_.contains(endpoint_id)) { NEARBY_LOG(INFO, "Invoking duplicate ProcessBwuPathAvailableEvent for %s", endpoint_id.c_str()); - if (FeatureFlags::GetInstance() - .GetFlags() - .disallow_out_of_order_bwu_avail_event) { - NEARBY_LOG(ERROR, - "BandwidthUpgradeManager received a duplicate bandwidth " - "upgrade for endpoint %s. We're out of sync with the remote " - "device and cannot recover; closing all channels.", - endpoint_id.c_str()); + NEARBY_LOG(ERROR, + "BandwidthUpgradeManager received a duplicate bandwidth " + "upgrade for endpoint %s. We're out of sync with the remote " + "device and cannot recover; closing all channels.", + endpoint_id.c_str()); - auto item = previous_endpoint_channels_.extract(endpoint_id); - if (!item.empty()) { - std::shared_ptr previous_endpoint_channel = - item.mapped(); - if (previous_endpoint_channel) { - previous_endpoint_channel->Close(DisconnectionReason::UNFINISHED); - } + auto item = previous_endpoint_channels_.extract(endpoint_id); + if (!item.empty()) { + std::shared_ptr previous_endpoint_channel = + item.mapped(); + if (previous_endpoint_channel) { + previous_endpoint_channel->Close(DisconnectionReason::UNFINISHED); } - std::shared_ptr new_channel = - channel_manager_->GetChannelForEndpoint(endpoint_id); - if (new_channel) { - // The upgraded channel never finished upgrading, and therefore is still - // paused. - new_channel->Resume(); - new_channel->Close(DisconnectionReason::UNFINISHED); - } - - return; } + std::shared_ptr new_channel = + channel_manager_->GetChannelForEndpoint(endpoint_id); + if (new_channel) { + // The upgraded channel never finished upgrading, and therefore is still + // paused. + new_channel->Resume(); + new_channel->Close(DisconnectionReason::UNFINISHED); + } + + return; } Medium medium = parser::UpgradePathInfoMediumToMedium(upgrade_path_info.medium()); @@ -467,11 +451,7 @@ void BwuManager::ProcessBwuPathAvailableEvent( return; } - if (FeatureFlags::GetInstance() - .GetFlags() - .disallow_out_of_order_bwu_avail_event) { - in_progress_upgrades_.emplace(endpoint_id, client); - } + in_progress_upgrades_.emplace(endpoint_id, client); RunUpgradeProtocol(client, endpoint_id, std::move(channel)); } @@ -753,11 +733,7 @@ void BwuManager::ProcessSafeToClosePriorChannelEvent( // Report the success to the client client->OnBandwidthChanged(endpoint_id, channel->GetMedium()); - if (FeatureFlags::GetInstance() - .GetFlags() - .disallow_out_of_order_bwu_avail_event) { - in_progress_upgrades_.erase(endpoint_id); - } + in_progress_upgrades_.erase(endpoint_id); } void BwuManager::ProcessUpgradeFailureEvent( diff --git a/cpp/core/internal/endpoint_channel_manager.cc b/cpp/core/internal/endpoint_channel_manager.cc index 3f78346d..f099fa57 100644 --- a/cpp/core/internal/endpoint_channel_manager.cc +++ b/cpp/core/internal/endpoint_channel_manager.cc @@ -145,9 +145,7 @@ bool EndpointChannelManager::ChannelState::RemoveEndpoint( if (channel) { // If the channel was paused (i.e. during a bandwidth upgrade negotiation) // we resume to ensure the thread won't hang when trying to write to it. - if (FeatureFlags::GetInstance().GetFlags().resume_before_disconnect) { - channel->Resume(); - } + channel->Resume(); channel->Write(parser::ForDisconnection()); SystemClock::Sleep(kDataTransferDelay); diff --git a/cpp/core/internal/service_controller_router.cc b/cpp/core/internal/service_controller_router.cc index 672edbbf..0e1932ae 100644 --- a/cpp/core/internal/service_controller_router.cc +++ b/cpp/core/internal/service_controller_router.cc @@ -50,14 +50,8 @@ const std::size_t kMaxEndpointInfoLength = 131u; ServiceControllerRouter::~ServiceControllerRouter() { NEARBY_LOG(INFO, "ServiceControllerRouter going down."); - if (FeatureFlags::GetInstance() - .GetFlags() - .disable_released_service_controller) { - if (service_controller_) { - service_controller_->Stop(); - } - } else { - service_controller_.reset(); + if (service_controller_) { + service_controller_->Stop(); } // And make sure that cleanup is the last thing we do. serializer_.Shutdown(); @@ -134,9 +128,8 @@ void ServiceControllerRouter::InjectEndpoint( ClientProxy* client, absl::string_view service_id, const OutOfBandConnectionMetadata& metadata, const ResultCallback& callback) { - RouteToServiceController( - [this, client, service_id = std::string(service_id), metadata, - callback]() { + RouteToServiceController([this, client, service_id = std::string(service_id), + metadata, callback]() { // Currently, Bluetooth is the only supported medium for endpoint injection. if (metadata.medium != Medium::BLUETOOTH || metadata.remote_bluetooth_mac_address.size() != kMacAddressLength) { @@ -438,11 +431,7 @@ void ServiceControllerRouter::ReleaseServiceControllerForClient( clients_.erase(client); // service_controller_ won't be released here. Instead, in destructor. - if (FeatureFlags::GetInstance() - .GetFlags() - .disable_released_service_controller) { - service_controller_->Stop(); - } + service_controller_->Stop(); if (clients_.empty()) { current_strategy_ = Strategy{}; diff --git a/cpp/platform/base/feature_flags.h b/cpp/platform/base/feature_flags.h index ee4a89cd..ff30b432 100644 --- a/cpp/platform/base/feature_flags.h +++ b/cpp/platform/base/feature_flags.h @@ -27,14 +27,6 @@ class FeatureFlags { // Holds for all the feature flags. struct Flags { bool enable_cancellation_flag = false; - bool resume_before_disconnect = true; - // Disable ServiceController API (using StoppableServiceController) when - // ServiceController is released to prevent calls to that API from - // other threads. - bool disable_released_service_controller = true; - // Ignore subsequent BWU Available events when we're still processing the - // first one. - bool disallow_out_of_order_bwu_avail_event = true; bool enable_async_bandwidth_upgrade = true; // Let endpoint_manager erase deleted endpoint from endpoints_ inside // function RemoveEndpoint.