diff --git a/cpp/core/internal/BUILD b/cpp/core/internal/BUILD index 5667fb55..ff017980 100644 --- a/cpp/core/internal/BUILD +++ b/cpp/core/internal/BUILD @@ -76,7 +76,6 @@ cc_library( "pcp_manager.h", "service_controller.h", "service_controller_router.h", - "stoppable_service_controller.h", "webrtc_bwu_handler.h", "webrtc_endpoint_channel.h", "wifi_lan_bwu_handler.h", diff --git a/cpp/core/internal/mock_service_controller.h b/cpp/core/internal/mock_service_controller.h index b701644c..9f2c0e27 100644 --- a/cpp/core/internal/mock_service_controller.h +++ b/cpp/core/internal/mock_service_controller.h @@ -31,6 +31,7 @@ namespace connections { */ class MockServiceController : public ServiceController { public: + MOCK_METHOD(void, Stop, (), (override)); MOCK_METHOD(Status, StartAdvertising, (ClientProxy * client, const std::string& service_id, const ConnectionOptions& options, diff --git a/cpp/core/internal/offline_service_controller.cc b/cpp/core/internal/offline_service_controller.cc index 5debdce8..fab7a4f7 100644 --- a/cpp/core/internal/offline_service_controller.cc +++ b/cpp/core/internal/offline_service_controller.cc @@ -31,48 +31,57 @@ void OfflineServiceController::Stop() { Status OfflineServiceController::StartAdvertising( ClientProxy* client, const std::string& service_id, const ConnectionOptions& options, const ConnectionRequestInfo& info) { + if (stop_) return {Status::kOutOfOrderApiCall}; return pcp_manager_.StartAdvertising(client, service_id, options, info); } void OfflineServiceController::StopAdvertising(ClientProxy* client) { + if (stop_) return; pcp_manager_.StopAdvertising(client); } Status OfflineServiceController::StartDiscovery( ClientProxy* client, const std::string& service_id, const ConnectionOptions& options, const DiscoveryListener& listener) { + if (stop_) return {Status::kOutOfOrderApiCall}; return pcp_manager_.StartDiscovery(client, service_id, options, listener); } void OfflineServiceController::StopDiscovery(ClientProxy* client) { + if (stop_) return; pcp_manager_.StopDiscovery(client); } void OfflineServiceController::InjectEndpoint( ClientProxy* client, const std::string& service_id, const OutOfBandConnectionMetadata& metadata) { + if (stop_) return; pcp_manager_.InjectEndpoint(client, service_id, metadata); } Status OfflineServiceController::RequestConnection( ClientProxy* client, const std::string& endpoint_id, const ConnectionRequestInfo& info, const ConnectionOptions& options) { + if (stop_) return {Status::kOutOfOrderApiCall}; return pcp_manager_.RequestConnection(client, endpoint_id, info, options); } Status OfflineServiceController::AcceptConnection( ClientProxy* client, const std::string& endpoint_id, const PayloadListener& listener) { + if (stop_) return {Status::kOutOfOrderApiCall}; return pcp_manager_.AcceptConnection(client, endpoint_id, listener); } Status OfflineServiceController::RejectConnection( ClientProxy* client, const std::string& endpoint_id) { + if (stop_) return {Status::kOutOfOrderApiCall}; return pcp_manager_.RejectConnection(client, endpoint_id); } void OfflineServiceController::InitiateBandwidthUpgrade( ClientProxy* client, const std::string& endpoint_id) { + if (stop_) return; NEARBY_LOGS(INFO) << "Client " << client->GetClientId() << " initiated a manual bandwidth upgrade with endpoint id=" << endpoint_id; @@ -82,16 +91,19 @@ void OfflineServiceController::InitiateBandwidthUpgrade( void OfflineServiceController::SendPayload( ClientProxy* client, const std::vector& endpoint_ids, Payload payload) { + if (stop_) return; payload_manager_.SendPayload(client, endpoint_ids, std::move(payload)); } Status OfflineServiceController::CancelPayload(ClientProxy* client, std::int64_t payload_id) { + if (stop_) return {Status::kOutOfOrderApiCall}; return payload_manager_.CancelPayload(client, payload_id); } void OfflineServiceController::DisconnectFromEndpoint( ClientProxy* client, const std::string& endpoint_id) { + if (stop_) return; endpoint_manager_.UnregisterEndpoint(client, endpoint_id); } diff --git a/cpp/core/internal/offline_service_controller.h b/cpp/core/internal/offline_service_controller.h index 1c29b83d..9e1d0e8d 100644 --- a/cpp/core/internal/offline_service_controller.h +++ b/cpp/core/internal/offline_service_controller.h @@ -75,7 +75,7 @@ class OfflineServiceController : public ServiceController { void DisconnectFromEndpoint(ClientProxy* client, const std::string& endpoint_id) override; - void Stop(); + void Stop() override; private: // Note that the order of declaration of these is crucial, because we depend diff --git a/cpp/core/internal/service_controller.h b/cpp/core/internal/service_controller.h index 2f577e53..1fa3f646 100644 --- a/cpp/core/internal/service_controller.h +++ b/cpp/core/internal/service_controller.h @@ -48,6 +48,13 @@ class ServiceController { ServiceController(const ServiceController&) = delete; ServiceController& operator=(const ServiceController&) = delete; + // Stops and disables service controller. + // + // When service controller is stopped all API call fail early. + // Note that all Core, ClientProxy objects referencing this service + // controller are affected. + virtual void Stop() = 0; + // Starts advertising an endpoint for a local app. virtual Status StartAdvertising(ClientProxy* client, const std::string& service_id, diff --git a/cpp/core/internal/service_controller_router.cc b/cpp/core/internal/service_controller_router.cc index 5e0df102..672edbbf 100644 --- a/cpp/core/internal/service_controller_router.cc +++ b/cpp/core/internal/service_controller_router.cc @@ -54,7 +54,7 @@ ServiceControllerRouter::~ServiceControllerRouter() { .GetFlags() .disable_released_service_controller) { if (service_controller_) { - service_controller_->Shutdown(); + service_controller_->Stop(); } } else { service_controller_.reset(); @@ -441,7 +441,7 @@ void ServiceControllerRouter::ReleaseServiceControllerForClient( if (FeatureFlags::GetInstance() .GetFlags() .disable_released_service_controller) { - service_controller_->Shutdown(); + service_controller_->Stop(); } if (clients_.empty()) { @@ -491,8 +491,7 @@ Status ServiceControllerRouter::UpdateCurrentServiceControllerAndStrategy( return {Status::kError}; } - service_controller_ = absl::make_unique( - service_controller_factory_()); + service_controller_.reset(service_controller_factory_()); current_strategy_ = strategy; return {Status::kSuccess}; diff --git a/cpp/core/internal/service_controller_router.h b/cpp/core/internal/service_controller_router.h index 83956cd5..8c9e7039 100644 --- a/cpp/core/internal/service_controller_router.h +++ b/cpp/core/internal/service_controller_router.h @@ -21,7 +21,6 @@ #include "core/internal/client_proxy.h" #include "core/internal/service_controller.h" -#include "core/internal/stoppable_service_controller.h" #include "core/options.h" #include "core/params.h" #include "platform/base/runnable.h" @@ -120,7 +119,7 @@ class ServiceControllerRouter { absl::flat_hash_set clients_; std::function service_controller_factory_; - std::unique_ptr service_controller_; + std::unique_ptr service_controller_; Strategy current_strategy_; SingleThreadExecutor serializer_; }; diff --git a/cpp/core/internal/stoppable_service_controller.h b/cpp/core/internal/stoppable_service_controller.h deleted file mode 100644 index 3a46e165..00000000 --- a/cpp/core/internal/stoppable_service_controller.h +++ /dev/null @@ -1,145 +0,0 @@ -// Copyright 2020 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef CORE_INTERNAL_STOPPABLE_SERVICE_CONTROLLER_H_ -#define CORE_INTERNAL_STOPPABLE_SERVICE_CONTROLLER_H_ - -#include - -#include "core/internal/service_controller.h" -#include "core/status.h" -#include "platform/public/atomic_boolean.h" - -namespace location { -namespace nearby { -namespace connections { - -// A ServiceController proxy that can be shut down. -// When shut down, the API calls are not forwarded to the real controller. -// StoppableServiceController takes over ownership of ServiceController. -class StoppableServiceController : public ServiceController { - public: - explicit StoppableServiceController(ServiceController* controller) - : service_controller_{controller} {} - ~StoppableServiceController() override = default; - - void Shutdown() { stopped_.Set(true); } - - Status StartAdvertising(ClientProxy* client, const std::string& service_id, - const ConnectionOptions& options, - const ConnectionRequestInfo& info) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->StartAdvertising(client, service_id, options, - info); - } - - void StopAdvertising(ClientProxy* client) override { - if (stopped_) { - return; - } - service_controller_->StopAdvertising(client); - } - - Status StartDiscovery(ClientProxy* client, const std::string& service_id, - const ConnectionOptions& options, - const DiscoveryListener& listener) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->StartDiscovery(client, service_id, options, - listener); - } - void StopDiscovery(ClientProxy* client) override { - if (stopped_) { - return; - } - service_controller_->StopDiscovery(client); - } - - void InjectEndpoint(ClientProxy* client, const std::string& service_id, - const OutOfBandConnectionMetadata& metadata) override { - if (stopped_) { - return; - } - service_controller_->InjectEndpoint(client, service_id, metadata); - } - - Status RequestConnection(ClientProxy* client, const std::string& endpoint_id, - const ConnectionRequestInfo& info, - const ConnectionOptions& options) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->RequestConnection(client, endpoint_id, info, - options); - } - Status AcceptConnection(ClientProxy* client, const std::string& endpoint_id, - const PayloadListener& listener) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->AcceptConnection(client, endpoint_id, listener); - } - Status RejectConnection(ClientProxy* client, - const std::string& endpoint_id) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->RejectConnection(client, endpoint_id); - } - - void InitiateBandwidthUpgrade(ClientProxy* client, - const std::string& endpoint_id) override { - if (stopped_) { - return; - } - service_controller_->InitiateBandwidthUpgrade(client, endpoint_id); - } - - void SendPayload(ClientProxy* client, - const std::vector& endpoint_ids, - Payload payload) override { - if (stopped_) { - return; - } - service_controller_->SendPayload(client, endpoint_ids, std::move(payload)); - } - - Status CancelPayload(ClientProxy* client, Payload::Id payload_id) override { - if (stopped_) { - return {Status::kError}; - } - return service_controller_->CancelPayload(client, payload_id); - } - - void DisconnectFromEndpoint(ClientProxy* client, - const std::string& endpoint_id) override { - if (stopped_) { - return; - } - service_controller_->DisconnectFromEndpoint(client, endpoint_id); - } - - private: - std::unique_ptr service_controller_; - AtomicBoolean stopped_{false}; -}; - -} // namespace connections -} // namespace nearby -} // namespace location - -#endif // CORE_INTERNAL_STOPPABLE_SERVICE_CONTROLLER_H_