diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index 29037f276..57b30e0c9 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -94,6 +94,7 @@ cc_library( "//score/mw/com:__subpackages__", ], deps = [ + ":flag_owner", ":generic_proxy_event", ":handle_type", ":proxy_base", @@ -466,6 +467,7 @@ cc_library( ], deps = [ ":error", + ":flag_owner", ":proxy_binding", ":proxy_event_binding", ":sample_reference_tracker", @@ -473,6 +475,7 @@ cc_library( "//score/mw/com/impl/tracing:proxy_event_tracing_data", "@score_baselibs//score/language/futurecpp", "@score_baselibs//score/language/safecpp/scoped_function:scope", + "@score_baselibs//score/scope_exit", ], ) diff --git a/score/mw/com/impl/bindings/lola/BUILD b/score/mw/com/impl/bindings/lola/BUILD index 9436dfbad..c7a2d6183 100644 --- a/score/mw/com/impl/bindings/lola/BUILD +++ b/score/mw/com/impl/bindings/lola/BUILD @@ -397,6 +397,7 @@ cc_library( ":subscription_state_machine", ":transaction_log_id", ":transaction_log_rollback_executor", + "//score/mw/com/impl:find_service_handle", "//score/mw/com/impl:generic_proxy_event_binding", "//score/mw/com/impl:instance_identifier", "//score/mw/com/impl:instance_specifier", @@ -1117,7 +1118,10 @@ cc_unit_test( cc_unit_test( name = "proxy_test", srcs = ["proxy_test.cpp"], - features = COMPILER_WARNING_FEATURES, + features = COMPILER_WARNING_FEATURES + [ + # SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED catches an exception, so disable aborts_upon_exception. + "-aborts_upon_exception", + ], deps = [ ":proxy", ":shm_path_builder_mock", @@ -1125,6 +1129,7 @@ cc_unit_test( "//score/mw/com/impl/bindings/lola/test:transaction_log_test_resources", "//score/mw/com/impl/bindings/mock_binding", "//score/mw/com/impl/configuration", + "@score_baselibs//score/language/futurecpp:futurecpp_test_support", "@score_baselibs//score/result", ], ) diff --git a/score/mw/com/impl/bindings/lola/generic_proxy_event_test.cpp b/score/mw/com/impl/bindings/lola/generic_proxy_event_test.cpp index 34d4cc51d..72d65492e 100644 --- a/score/mw/com/impl/bindings/lola/generic_proxy_event_test.cpp +++ b/score/mw/com/impl/bindings/lola/generic_proxy_event_test.cpp @@ -39,6 +39,16 @@ class LolaGenericProxyEventFixture : public LolaProxyEventResources return *this; } + // ProxyEventCommon no longer Unsubscribes on destruction, so do it explicitly. + void TearDown() override + { + if (generic_proxy_event_ != nullptr) + { + generic_proxy_event_->Unsubscribe(); + } + ProxyMockedMemoryFixture::TearDown(); + } + std::unique_ptr generic_proxy_event_{nullptr}; }; TEST_F(LolaGenericProxyEventFixture, CanConstructAGenericProxyEvent) diff --git a/score/mw/com/impl/bindings/lola/proxy.cpp b/score/mw/com/impl/bindings/lola/proxy.cpp index e8cb56091..b1b8b3852 100644 --- a/score/mw/com/impl/bindings/lola/proxy.cpp +++ b/score/mw/com/impl/bindings/lola/proxy.cpp @@ -263,50 +263,6 @@ ServiceDataStorage& GetServiceDataStorage(const memory::shared::ManagedMemoryRes } // namespace detail_proxy -class FindServiceGuard final -{ - public: - // Suppress "AUTOSAR C++14 A15-5-3" rule findings. This rule states: "The std::terminate() function shall not be - // called implicitly". std::terminate() is implicitly called from 'find_service_handle_result.value()' in case - // find_service_handle_result doesn't have a value but as we check before with 'has_value()' so no way for throwing - // std::bad_optional_access which leds to std::terminate(). - // coverity[autosar_cpp14_a15_5_3_violation : FALSE] - FindServiceGuard(FindServiceHandler find_service_handler, - EnrichedInstanceIdentifier enriched_instance_identifier) - : service_availability_change_handle_{nullptr} - { - auto& service_discovery = impl::Runtime::getInstance().GetServiceDiscovery(); - const auto find_service_handle_result = service_discovery.StartFindService( - std::move(find_service_handler), std::move(enriched_instance_identifier)); - if (!find_service_handle_result.has_value()) - { - score::mw::log::LogFatal("lola") - << "StartFindService failed with error" << find_service_handle_result.error() << ". Terminating."; - std::terminate(); - } - service_availability_change_handle_ = std::make_unique(find_service_handle_result.value()); - } - - ~FindServiceGuard() noexcept - { - auto& service_discovery = impl::Runtime::getInstance().GetServiceDiscovery(); - const auto stop_find_service_result = service_discovery.StopFindService(*service_availability_change_handle_); - if (!stop_find_service_result.has_value()) - { - score::mw::log::LogError("lola") - << "StopFindService failed with error" << stop_find_service_result.error() << ". Ignoring error."; - } - } - - FindServiceGuard(const FindServiceGuard&) = delete; - FindServiceGuard& operator=(const FindServiceGuard&) = delete; - FindServiceGuard(FindServiceGuard&& other) = delete; - FindServiceGuard& operator=(FindServiceGuard&& other) = delete; - - private: - std::unique_ptr service_availability_change_handle_; -}; - std::atomic Proxy::current_proxy_instance_counter_{0U}; ElementFqId Proxy::EventNameToElementFqIdConverter::Convert(const std::string_view event_name) const noexcept @@ -430,20 +386,22 @@ Proxy::Proxy(std::shared_ptr control, offered_state_machine_{}, are_proxy_methods_setup_{false}, filesystem_{filesystem}, - find_service_guard_{std::make_unique( - [this](ServiceHandleContainer service_handle_container, FindServiceHandle) { - // Suppress Autosar C++14 A8-5-3 states that auto variables shall not be initialized using braced - // initialization. This is a false positive, we don't use auto here - // coverity[autosar_cpp14_a8_5_3_violation : FALSE] - std::lock_guard lock{proxy_event_registration_mutex_}; - is_service_instance_available_ = !service_handle_container.empty(); - ServiceAvailabilityChangeHandler(is_service_instance_available_); - }, - EnrichedInstanceIdentifier{handle_})} + find_service_handle_{}, + prepare_deinitialize_called_{false}, + finalize_deinitialize_called_{false} { + StartProxyAutoReconnect(); } -Proxy::~Proxy() = default; +Proxy::~Proxy() +{ + if (!prepare_deinitialize_called_ || !finalize_deinitialize_called_) + { + score::mw::log::LogFatal("lola") + << "Proxy destroyed without prior PrepareDeinitialize + FinalizeDeinitialize. Terminating."; + std::terminate(); + } +} void Proxy::ServiceAvailabilityChangeHandler(const bool is_service_available) { @@ -929,4 +887,70 @@ void Proxy::RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(was_inserted, "Method IDs must be unique!"); } +void Proxy::PrepareDeinitialize() +{ + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(!prepare_deinitialize_called_, + "PrepareDeinitialize must only be called once."); + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE( + !finalize_deinitialize_called_, + "Defensive programming: FinalizeDeinitialize must not have been called before PrepareDeinitialize (This should " + "never fail since we check this in FinalizeDeinitialize)."); + + StopProxyAutoReconnect(); + prepare_deinitialize_called_ = true; +} + +void Proxy::FinalizeDeinitialize() +{ + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(!finalize_deinitialize_called_, + "FinalizeDeinitialize must only be called once."); + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE( + prepare_deinitialize_called_, "PrepareDeinitialize must have been called before FinalizeDeinitialize."); + + { + std::lock_guard lock{proxy_event_registration_mutex_}; + event_bindings_.clear(); + is_service_instance_available_ = false; + } + { + std::lock_guard lock{proxy_method_registration_mutex_}; + proxy_methods_.clear(); + } + finalize_deinitialize_called_ = true; +} + +void Proxy::StartProxyAutoReconnect() +{ + auto& service_discovery = impl::Runtime::getInstance().GetServiceDiscovery(); + const auto find_service_handle_result = service_discovery.StartFindService( + [this](ServiceHandleContainer service_handle_container, FindServiceHandle) { + std::lock_guard lock{proxy_event_registration_mutex_}; + is_service_instance_available_ = !service_handle_container.empty(); + ServiceAvailabilityChangeHandler(is_service_instance_available_); + }, + EnrichedInstanceIdentifier{handle_}); + if (!find_service_handle_result.has_value()) + { + score::mw::log::LogFatal("lola") << "StartFindService failed with error" << find_service_handle_result.error() + << ". Terminating."; + std::terminate(); + } + find_service_handle_ = find_service_handle_result.value(); +} + +void Proxy::StopProxyAutoReconnect() +{ + SCORE_LANGUAGE_FUTURECPP_PRECONDITION_PRD_MESSAGE( + find_service_handle_.has_value(), + "StopProxyAutoReconnect: find_service_handle_ must be set in StartProxyAutoReconnect"); + auto& service_discovery = impl::Runtime::getInstance().GetServiceDiscovery(); + const auto stop_find_service_result = service_discovery.StopFindService(*find_service_handle_); + if (!stop_find_service_result.has_value()) + { + score::mw::log::LogError("lola") << "StopFindService failed with error" << stop_find_service_result.error() + << ". Ignoring error."; + } + find_service_handle_.reset(); +} + } // namespace score::mw::com::impl::lola diff --git a/score/mw/com/impl/bindings/lola/proxy.h b/score/mw/com/impl/bindings/lola/proxy.h index c4cca4320..d393f66a5 100644 --- a/score/mw/com/impl/bindings/lola/proxy.h +++ b/score/mw/com/impl/bindings/lola/proxy.h @@ -27,6 +27,7 @@ #include "score/mw/com/impl/configuration/lola_service_instance_id.h" #include "score/mw/com/impl/configuration/lola_service_type_deployment.h" #include "score/mw/com/impl/configuration/quality_type.h" +#include "score/mw/com/impl/find_service_handle.h" #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/proxy_binding.h" #include "score/mw/com/impl/proxy_event_binding_base.h" @@ -62,7 +63,6 @@ namespace score::mw::com::impl::lola { class IShmPathBuilder; -class FindServiceGuard; namespace detail_proxy { @@ -200,9 +200,20 @@ class Proxy : public ProxyBinding void RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept; + /// \brief Stops auto-reconnect for this proxy and marks it ready for destruction. + /// \pre Must be called exactly once on a given Proxy. Calling it a second time will terminate. + void PrepareDeinitialize() override; + + /// \brief Clears event and method registration state and marks the proxy ready for destruction. + /// \note Idempotent: calling it more than once is safe. + void FinalizeDeinitialize() override; + private: static std::atomic current_proxy_instance_counter_; + void StartProxyAutoReconnect(); + void StopProxyAutoReconnect(); + void ServiceAvailabilityChangeHandler(const bool is_service_available); void InitializeSharedMemoryForMethods( memory::shared::ManagedMemoryResource& memory_resource, @@ -261,9 +272,13 @@ class Proxy : public ProxyBinding score::filesystem::Filesystem filesystem_; - // We make find_service_guard_ the last member variable since it registers a handler which accesses member variables - // of this class, so they should be initialised first. - std::unique_ptr find_service_guard_; + /// Handle returned by ServiceDiscovery::StartFindService which is called for proxy auto reconnect, Held so we can + /// call StopFindService later from StopProxyAutoReconnect(). + std::optional find_service_handle_; + + /// Set by PrepareDeinitialize / FinalizeDeinitialize respectively. ~Proxy terminates unless both were called. + bool prepare_deinitialize_called_; + bool finalize_deinitialize_called_; }; template diff --git a/score/mw/com/impl/bindings/lola/proxy_event_common.cpp b/score/mw/com/impl/bindings/lola/proxy_event_common.cpp index 552265166..5e75d0495 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event_common.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_event_common.cpp @@ -42,11 +42,6 @@ ProxyEventCommon::ProxyEventCommon(Proxy& parent, const ElementFqId element_fq_i { } -ProxyEventCommon::~ProxyEventCommon() -{ - Unsubscribe(); -} - Result ProxyEventCommon::Subscribe(const std::size_t max_sample_count) { std::stringstream sstream{}; diff --git a/score/mw/com/impl/bindings/lola/proxy_event_common.h b/score/mw/com/impl/bindings/lola/proxy_event_common.h index 7086baa6c..68ede7059 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event_common.h +++ b/score/mw/com/impl/bindings/lola/proxy_event_common.h @@ -53,7 +53,7 @@ class ProxyEventCommon final public: ProxyEventCommon(Proxy& parent, const ElementFqId element_fq_id, const std::string_view event_name); - ~ProxyEventCommon(); + ~ProxyEventCommon() = default; ProxyEventCommon(const ProxyEventCommon&) = delete; ProxyEventCommon(ProxyEventCommon&&) noexcept = delete; diff --git a/score/mw/com/impl/bindings/lola/proxy_event_common_test.cpp b/score/mw/com/impl/bindings/lola/proxy_event_common_test.cpp index 2a1f0fbe1..239cc08d7 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event_common_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_event_common_test.cpp @@ -100,6 +100,16 @@ class LolaProxyEventCommonFixture : public ProxyMockedMemoryFixture UnregisterEventNotification(QualityType::kASIL_QM, kElementFqId, my_handler_no, kDummyPid)); } + // ProxyEventCommon no longer Unsubscribes on destruction, so do it explicitly. + void TearDown() override + { + if (proxy_event_ != nullptr) + { + proxy_event_->Unsubscribe(); + } + ProxyMockedMemoryFixture::TearDown(); + } + protected: ::testing::MockFunction event_handler_{}; std::unique_ptr proxy_event_{nullptr}; @@ -194,7 +204,10 @@ TYPED_TEST(LolaProxyEventCommonFixture, UnsubscribeImmediatelyAfterSubscribing) TYPED_TEST(LolaProxyEventCommonFixture, UnsubscribingWillUnregisterEventHandler) { - this->RecordProperty("Verifies", "SCR-21293524"); + // SCR-20236391 and SCR-20237033 are split with CallsUnsubscribeOnDestruction in traits_test.cpp. + // this test covers Unsubscribe triggering UnregisterEventNotification, the other covers proxy + // destruction triggering Unsubscribe on the events and fields. + this->RecordProperty("Verifies", "SCR-21293524, SCR-20236391, SCR-20237033"); this->RecordProperty( "Description", "Checks that calling Unsubscribe while currently subscribed will unregister a registered event " @@ -240,53 +253,6 @@ TYPED_TEST(LolaProxyEventCommonFixture, UnsubscribingWillUnregisterEventHandler) EXPECT_TRUE(handler_unregistered); } -TYPED_TEST(LolaProxyEventCommonFixture, DestroyingTheProxyEventWillUnregisterEventHandler) -{ - this->RecordProperty("Verifies", "SCR-20236391, SCR-20237033"); - this->RecordProperty( - "Description", - "Checks that a registered event handler will be unregistered when the (generic) proxy event is destroyed."); - this->RecordProperty("TestType", "Requirements-based test"); - this->RecordProperty("Priority", "1"); - this->RecordProperty("DerivationTechnique", "Analysis of requirements"); - - const std::size_t max_sample_count{1U}; - constexpr const IMessagePassingService::HandlerRegistrationNoType my_handler_no = 37U; - bool handler_unregistered{false}; - - // Expecting that a receive handler will be registered - EXPECT_CALL(*this->mock_service_, - RegisterEventNotification(QualityType::kASIL_QM, kElementFqId, _, this->kDummyPid)) - .WillOnce(Return(my_handler_no)); - - // and the same receive handler will be unregistered - EXPECT_CALL(*this->mock_service_, - UnregisterEventNotification(QualityType::kASIL_QM, kElementFqId, my_handler_no, this->kDummyPid)) - .WillOnce(InvokeWithoutArgs([&handler_unregistered]() { - handler_unregistered = true; - })); - - // Given a proxy that unsubscribes while waiting for being subscribed correctly - this->InitialiseProxyAndEvent(); - - // When we subscribe (sending a subscribe message to the producer) - std::ignore = this->proxy_event_->Subscribe(max_sample_count); - - // and Register a receive handler - safecpp::Scope<> event_receive_handler_scope{}; - std::ignore = - this->proxy_event_->SetReceiveHandler(FromMockFunction(event_receive_handler_scope, this->event_handler_)); - - // Then the receive handler should not be unregistered - EXPECT_FALSE(handler_unregistered); - - // and when the ProxyEvent is destroyed - this->proxy_event_.reset(); - - // Then the receive handler should be unregistered - EXPECT_TRUE(handler_unregistered); -} - TYPED_TEST(LolaProxyEventCommonFixture, DoubleSubscribe) { const std::size_t max_sample_count{kMaxNumSlots / 2U}; diff --git a/score/mw/com/impl/bindings/lola/proxy_event_test.cpp b/score/mw/com/impl/bindings/lola/proxy_event_test.cpp index 06aea0d11..a8cdea8d3 100644 --- a/score/mw/com/impl/bindings/lola/proxy_event_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_event_test.cpp @@ -143,6 +143,16 @@ class LolaProxyEventFixture : public LolaProxyEventResources return test_proxy_event_->GetNewSamples(std::move(receiver), guard_factory); } + // ProxyEventCommon no longer Unsubscribes on destruction, so do it explicitly. + void TearDown() override + { + if (test_proxy_event_ != nullptr) + { + test_proxy_event_->Unsubscribe(); + } + ProxyMockedMemoryFixture::TearDown(); + } + std::unique_ptr test_proxy_event_{nullptr}; std::unique_ptr sample_reference_tracker_{}; }; diff --git a/score/mw/com/impl/bindings/lola/proxy_test.cpp b/score/mw/com/impl/bindings/lola/proxy_test.cpp index aa48abea2..edd512d99 100644 --- a/score/mw/com/impl/bindings/lola/proxy_test.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_test.cpp @@ -26,6 +26,8 @@ #include "score/result/result.h" +#include + #include #include #include @@ -552,6 +554,60 @@ TEST_F(ProxyAutoReconnectDeathTest, ProxyCreateWillTerminateIfStartFindServiceRe EXPECT_DEATH(start_find_service_fails(), ".*"); } +TEST_F(ProxyAutoReconnectDeathTest, PrepareDeinitializeTerminatesWhenCalledTwice) +{ + // Given a proxy on which PrepareDeinitialize has already been called + InitialiseProxyWithConstructor(identifier_); + PrepareDeinitialize(); + + // When calling PrepareDeinitialize a second time + // Then the program terminates + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED(proxy_->PrepareDeinitialize()); +} + +TEST_F(ProxyAutoReconnectDeathTest, FinalizeDeinitializeTerminatesWhenCalledTwice) +{ + // Given a proxy on which PrepareDeinitialize and FinalizeDeinitialize has already been called + InitialiseProxyWithConstructor(identifier_); + PrepareDeinitialize(); + FinalizeDeinitialize(); + + // When calling FinalizeDeinitialize a second time + // Then the program terminates + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED(proxy_->FinalizeDeinitialize()); +} + +TEST_F(ProxyAutoReconnectDeathTest, ProxyDestructionTerminatesWhenPrepareDeinitializeNotCalled) +{ + // Given a proxy on which PrepareDeinitialize has NOT been called + InitialiseProxyWithConstructor(identifier_); + + // When the proxy is destroyed + // Then ~Proxy terminates because PrepareDeinitialize was never called + EXPECT_DEATH(proxy_.reset(), ".*"); +} + +TEST_F(ProxyAutoReconnectDeathTest, ProxyDestructionTerminatesWhenFinalizeDeinitializeNotCalled) +{ + // Given a proxy on which PrepareDeinitialize has been called + InitialiseProxyWithConstructor(identifier_); + PrepareDeinitialize(); + + // When the proxy is destroyed + // Then ~Proxy terminates because FinalizeDeinitialize was never called + EXPECT_DEATH(proxy_.reset(), ".*"); +} + +TEST_F(ProxyAutoReconnectDeathTest, FinalizeDeinitializeBeforeCallingPrepareDeinitializeTerminates) +{ + // Given a proxy on which PrepareDeinitialize has not been called + InitialiseProxyWithConstructor(identifier_); + + // When calling FinalizeDeinitialize before PrepareDeinitialize + // Then the program terminates + SCORE_LANGUAGE_FUTURECPP_EXPECT_CONTRACT_VIOLATED(proxy_->FinalizeDeinitialize()); +} + TEST_F(ProxyEventBindingFixture, RegisteringEventBindingWillCallNotifyServiceInstanceChangedAvailabilityOnBinding) { mock_binding::ProxyEventBase mock_proxy_event_base_binding{}; diff --git a/score/mw/com/impl/bindings/lola/skeleton.cpp b/score/mw/com/impl/bindings/lola/skeleton.cpp index 1f0982fce..403609a37 100644 --- a/score/mw/com/impl/bindings/lola/skeleton.cpp +++ b/score/mw/com/impl/bindings/lola/skeleton.cpp @@ -228,6 +228,8 @@ Skeleton::Skeleton(const InstanceIdentifier& identifier, method_subscription_registration_guard_asil_b_{}, was_old_shm_region_reopened_{false}, filesystem_{std::move(filesystem)}, + prepare_offer_called_{false}, + prepare_stop_offer_called_{false}, method_call_handler_scope_{}, on_service_method_subscribed_handler_scope_{} { @@ -358,6 +360,7 @@ auto Skeleton::PrepareOffer(SkeletonEventBindings& events, method_subscription_registration_guard_asil_b_.emplace(std::move(asil_b_registration_result).value()); } + prepare_offer_called_ = true; return {}; } @@ -370,6 +373,8 @@ auto Skeleton::PrepareOffer(SkeletonEventBindings& events, auto Skeleton::PrepareStopOffer(std::optional unregister_shm_object_callback) noexcept -> void { + prepare_stop_offer_called_ = true; + if (unregister_shm_object_callback.has_value()) { // Suppress "AUTOSAR C++14 A15-4-2" rule finding. This rule states: "I a function is declared to be @@ -443,6 +448,16 @@ auto Skeleton::PrepareStopOffer(std::optional memory_manager_.Reset(); } +Skeleton::~Skeleton() noexcept +{ + if (prepare_offer_called_ && !prepare_stop_offer_called_) + { + score::mw::log::LogFatal("lola") + << "Skeleton was offered but destroyed without prior PrepareStopOffer. Terminating."; + std::terminate(); + } +} + QualityType Skeleton::GetInstanceQualityType() const { return quality_type_; diff --git a/score/mw/com/impl/bindings/lola/skeleton.h b/score/mw/com/impl/bindings/lola/skeleton.h index 321f7602e..b0539bd59 100644 --- a/score/mw/com/impl/bindings/lola/skeleton.h +++ b/score/mw/com/impl/bindings/lola/skeleton.h @@ -99,7 +99,7 @@ class Skeleton final : public SkeletonBinding std::unique_ptr> service_instance_existence_flock_mutex_and_lock); - ~Skeleton() noexcept override = default; + ~Skeleton() noexcept override; Skeleton(const Skeleton&) = delete; Skeleton& operator=(const Skeleton&) = delete; @@ -229,6 +229,11 @@ class Skeleton final : public SkeletonBinding score::filesystem::Filesystem filesystem_; + /// Set by PrepareOffer / PrepareStopOffer respectively. ~Skeleton terminates if PrepareOffer was called but + /// PrepareStopOffer was not. + bool prepare_offer_called_; + bool prepare_stop_offer_called_; + /// \brief Scope that is passed to the MethodCallHandler handler that is registered for each ProxyMethod /// /// This scope will be manually expired in PrepareStopOffer which will prevent any ProxyMethod from calling a diff --git a/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp b/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp index bd05f8aec..942b00524 100644 --- a/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp +++ b/score/mw/com/impl/bindings/lola/skeleton_method_handling_test.cpp @@ -223,6 +223,20 @@ class SkeletonMethodHandlingFixture : public SkeletonMockedMemoryFixture return *this; } + void TearDown() override + { + if (skeleton_ != nullptr && !prepare_stop_offer_called_) + { + skeleton_->PrepareStopOffer({}); + } + } + + void PrepareStopOffer() + { + skeleton_->PrepareStopOffer({}); + prepare_stop_offer_called_ = true; + } + ProxyInstanceIdentifier proxy_instance_identifier_qm_{kDummyApplicationId, kDummyProxyInstanceCounterQm}; ProxyInstanceIdentifier proxy_instance_identifier_b_{kDummyApplicationId, kDummyProxyInstanceCounterAsilB}; const UniqueMethodIdentifier foo_unique_method_id_{test::kFooMethodId, MethodType::kMethod}; @@ -258,6 +272,9 @@ class SkeletonMethodHandlingFixture : public SkeletonMockedMemoryFixture std::optional captured_method_subscribed_handler_b_{}; safecpp::Scope<> method_call_registration_guard_scope_{}; + + private: + bool prepare_stop_offer_called_{false}; }; using SkeletonPrepareOfferFixture = SkeletonMethodHandlingFixture; @@ -399,18 +416,19 @@ TEST_F(SkeletonPrepareOfferFixture, PrepareOfferWillNotCallUnregisterSubscribedM { GivenAnAsilBSkeletonWithTwoMethods(); - // Expecting that RegisterOnServiceMethodSubscribedHandler will be called for QM and ASIL-B - EXPECT_CALL(message_passing_mock_, - RegisterOnServiceMethodSubscribedHandler(QualityType::kASIL_QM, skeleton_instance_identifier_, _, _)); - EXPECT_CALL(message_passing_mock_, - RegisterOnServiceMethodSubscribedHandler(QualityType::kASIL_B, skeleton_instance_identifier_, _, _)); - - // Expecting that UnregisterOnServiceMethodSubscribedHandler will not be called for each method for QM and ASIL-B - EXPECT_CALL(message_passing_mock_, UnregisterOnServiceMethodSubscribedHandler(_, _)).Times(0); + // and given that UnregisterOnServiceMethodSubscribedHandler flips a flag so we can verify that it is not called + auto unregister_called = std::make_shared(false); + ON_CALL(message_passing_mock_, UnregisterOnServiceMethodSubscribedHandler(_, _)) + .WillByDefault(InvokeWithoutArgs([unregister_called] { + *unregister_called = true; + })); // When calling PrepareOffer score::cpp::ignore = skeleton_->PrepareOffer( kEmptyEventBindings, kEmptyFieldBindings, std::move(kEmptyRegisterShmObjectTraceCallback)); + + // Then UnregisterOnServiceMethodSubscribedHandler was not called during PrepareOffer + EXPECT_FALSE(*unregister_called); } TEST_F(SkeletonPrepareOfferFixture, CallingAsilBWillUnregisterQmHandlerOnAsilBRegistrationFailure) @@ -469,7 +487,7 @@ TEST_F(SkeletonPrepareStopOfferFixture, PrepareStopOfferExpiresScopeOfMethodCall kDummyPid); // and given that PrepareStopOffer was called - skeleton_->PrepareStopOffer({}); + PrepareStopOffer(); // When calling the method call handlers const auto method_call_handler_result_1 = std::invoke(method_call_handler_1.value(), 0U); @@ -485,7 +503,7 @@ TEST_F(SkeletonPrepareStopOfferFixture, PrepareStopOfferExpiresScopeOfSubscribeM GivenASkeletonWithTwoMethods().WhichCapturesRegisteredMethodSubscribedHandlers().WhichIsOffered(); // and given that PrepareStopOffer was called - skeleton_->PrepareStopOffer({}); + PrepareStopOffer(); // When calling a ServiceMethodSubscribedHandler ASSERT_TRUE(captured_method_subscribed_handler_qm_.has_value()); @@ -511,7 +529,7 @@ TEST_F(SkeletonPrepareStopOfferFixture, PrepareStopOfferDestroysPointerToSharedM // When calling PrepareStopOffer const auto shm_resource_ref_counter_after_opening = mock_method_memory_resource_qm_.use_count(); - skeleton_->PrepareStopOffer({}); + PrepareStopOffer(); // Then the reference counter for the methods SharedMemoryResource should be decremented, indicating that it's // been deleted from the Skeleton's state @@ -529,7 +547,7 @@ TEST_F(SkeletonPrepareStopOfferFixture, UnregistersQmAndAsilBSubscribedMethodHan UnregisterOnServiceMethodSubscribedHandler(QualityType::kASIL_B, skeleton_instance_identifier_)); // When calling PrepareStopOffer - skeleton_->PrepareStopOffer({}); + PrepareStopOffer(); } TEST_F(SkeletonPrepareStopOfferFixture, UnregistersAllRegisteredMethodCallHandlers) @@ -561,7 +579,7 @@ TEST_F(SkeletonPrepareStopOfferFixture, UnregistersAllRegisteredMethodCallHandle EXPECT_TRUE(scoped_handler_result_2.has_value()); // When calling PrepareStopOffer - skeleton_->PrepareStopOffer({}); + PrepareStopOffer(); } using SkeletonOnServiceMethodsSubscribedFixture = SkeletonMethodHandlingFixture; @@ -993,7 +1011,7 @@ TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingAsilBWithoutInArgsOrRet EXPECT_TRUE(scoped_handler_result.has_value()); } -TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingAsilBWillNotCallUnregisterMethodCallHandler) +TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingForBothAsilAndQmDoesNotCallUnregisterMethodCallHandler) { GivenAnAsilBSkeletonWithTwoMethods().WhichCapturesRegisteredMethodSubscribedHandlers().WhichIsOffered(); @@ -1002,14 +1020,17 @@ TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingAsilBWillNotCallUnregis RegisterMethodCallHandler(QualityType::kASIL_QM, foo_proxy_method_identifier_qm_, _, _)); EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(QualityType::kASIL_QM, dumb_proxy_method_identifier_qm_, _, _)); - EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(QualityType::kASIL_B, foo_proxy_method_identifier_b_, _, _)); EXPECT_CALL(message_passing_mock_, RegisterMethodCallHandler(QualityType::kASIL_B, dumb_proxy_method_identifier_b_, _, _)); - // Expecting that UnregisterMethodCallHandler will not be called for each method for QM and ASIL-B - EXPECT_CALL(message_passing_mock_, UnregisterMethodCallHandler(_, _)).Times(0); + // and given that UnregisterMethodCallHandler flips a flag so we can verify that it is not called + auto unregister_called = std::make_shared(false); + ON_CALL(message_passing_mock_, UnregisterMethodCallHandler(_, _)) + .WillByDefault(InvokeWithoutArgs([unregister_called] { + *unregister_called = true; + })); // When calling the registered method subscribed handler for both QM and AsilB ASSERT_TRUE(captured_method_subscribed_handler_qm_.has_value()); @@ -1025,6 +1046,9 @@ TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingAsilBWillNotCallUnregis test::kAllowedAsilBMethodConsumer, kDummyPid); EXPECT_TRUE(scoped_handler_result_2.has_value()); + + // Then UnregisterMethodCallHandler was not called during handler invocation + EXPECT_FALSE(*unregister_called); } TEST_F(SkeletonOnServiceMethodsSubscribedFixture, CallingWillUnregisterRegisteredMethodCallHandlersOnSubscriptionError) diff --git a/score/mw/com/impl/bindings/lola/subscription_state_machine_events_test.cpp b/score/mw/com/impl/bindings/lola/subscription_state_machine_events_test.cpp index 94407a6f7..2cb84db78 100644 --- a/score/mw/com/impl/bindings/lola/subscription_state_machine_events_test.cpp +++ b/score/mw/com/impl/bindings/lola/subscription_state_machine_events_test.cpp @@ -64,6 +64,7 @@ class StateMachineEventsFixture : public LolaProxyEventResources // Specifically, it's important that the Unsubscribe is recorded so that when ~TransactionLogRegistrationGuard // unregisters the TransactionLog, there are no open transactions. state_machine_.UnsubscribeEvent(); + ProxyMockedMemoryFixture::TearDown(); } void EnterSubscriptionPending(const std::size_t max_samples) noexcept diff --git a/score/mw/com/impl/bindings/lola/test/proxy_component_test.cpp b/score/mw/com/impl/bindings/lola/test/proxy_component_test.cpp index b8d7b7719..c70708118 100644 --- a/score/mw/com/impl/bindings/lola/test/proxy_component_test.cpp +++ b/score/mw/com/impl/bindings/lola/test/proxy_component_test.cpp @@ -92,6 +92,12 @@ class ProxyWithRealMemFixture : public ::testing::Test void TearDown() override { + if (created_proxy_ != nullptr) + { + created_proxy_->PrepareDeinitialize(); + created_proxy_->FinalizeDeinitialize(); + } + for (const auto& file : shm_files_) { std::ignore = score::filesystem::IStandardFilesystem::instance().Remove(std::string{"/dev/shm"} + file); @@ -165,6 +171,8 @@ class ProxyWithRealMemFixture : public ::testing::Test std::unique_ptr config_store_{nullptr}; + std::unique_ptr created_proxy_{nullptr}; + std::vector shm_files_{}; RuntimeMockGuard runtime_mock_{}; lola::RuntimeMock lola_runtime_mock_{}; @@ -191,10 +199,10 @@ TEST_F(ProxyWithRealMemFixture, IsEventProvidedOnlyReturnsTrueIfEventIsInSharedM ON_CALL(service_discovery_mock_, StartFindService(::testing::_, EnrichedInstanceIdentifier{handle})) .WillByDefault(::testing::Return(make_FindServiceHandle(10U))); - auto proxy = Proxy::Create(handle); - ASSERT_NE(proxy, nullptr); - EXPECT_TRUE(proxy->IsEventProvided(kEventName)); - EXPECT_FALSE(proxy->IsEventProvided(kNonProvidedEventName)); + created_proxy_ = Proxy::Create(handle); + ASSERT_NE(created_proxy_, nullptr); + EXPECT_TRUE(created_proxy_->IsEventProvided(kEventName)); + EXPECT_FALSE(created_proxy_->IsEventProvided(kNonProvidedEventName)); } class ProxyServiceDiscoveryFixture : public ProxyWithRealMemFixture diff --git a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp index 884d739c8..2933e79a5 100644 --- a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp +++ b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.cpp @@ -91,6 +91,37 @@ void ProxyMockedMemoryFixture::ExpectDataSegmentOpened() })); } +void ProxyMockedMemoryFixture::TearDown() +{ + // lola Proxy terminates on destruction if PrepareDeinitialize + FinalizeDeinitialize were not called first, + // so we run them here for tests that constructed a proxy directly. PrepareDeinitialize is non-idempotent + // (its precondition fires on a second call), so we skip it if the test already invoked it via the + // PrepareDeinitialize() wrapper. + if (proxy_ != nullptr) + { + if (!prepare_deinitialize_called_in_test_) + { + proxy_->PrepareDeinitialize(); + } + if (!finalize_deinitialize_called_in_test_) + { + proxy_->FinalizeDeinitialize(); + } + } +} + +void ProxyMockedMemoryFixture::PrepareDeinitialize() +{ + proxy_->PrepareDeinitialize(); + prepare_deinitialize_called_in_test_ = true; +} + +void ProxyMockedMemoryFixture::FinalizeDeinitialize() +{ + proxy_->FinalizeDeinitialize(); + finalize_deinitialize_called_in_test_ = true; +} + void ProxyMockedMemoryFixture::InitialiseProxyWithConstructor(const InstanceIdentifier& instance_identifier) { EnrichedInstanceIdentifier enriched_instance_identifier{instance_identifier}; diff --git a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h index a8240c6cc..24e03f01c 100644 --- a/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h +++ b/score/mw/com/impl/bindings/lola/test/proxy_event_test_resources.h @@ -169,9 +169,17 @@ class ProxyMockedMemoryFixture : public ::testing::Test ProxyMockedMemoryFixture() noexcept; + void TearDown() override; + void InitialiseProxyWithConstructor(const InstanceIdentifier& instance_identifier); void InitialiseProxyWithCreate(const InstanceIdentifier& instance_identifier); + /// \brief Calls PrepareDeinitialize on proxy_ and marks it so TearDown skips its own call. + void PrepareDeinitialize(); + + /// \brief Calls FinalizeDeinitialize on proxy_ and marks it so TearDown skips its own call. + void FinalizeDeinitialize(); + void ExpectControlSegmentOpened(); void ExpectDataSegmentOpened(); void InitialiseDummySkeletonEvent(const ElementFqId element_fq_id, @@ -213,6 +221,10 @@ class ProxyMockedMemoryFixture : public ::testing::Test ::testing::NiceMock> binding_runtime_{false, mock_service_}; std::unique_ptr proxy_{nullptr}; + + private: + bool prepare_deinitialize_called_in_test_{false}; + bool finalize_deinitialize_called_in_test_{false}; }; class LolaProxyEventResources : public ProxyMockedMemoryFixture diff --git a/score/mw/com/impl/bindings/mock_binding/proxy.h b/score/mw/com/impl/bindings/mock_binding/proxy.h index 812581d18..f0eb4b796 100644 --- a/score/mw/com/impl/bindings/mock_binding/proxy.h +++ b/score/mw/com/impl/bindings/mock_binding/proxy.h @@ -32,6 +32,8 @@ class Proxy : public ProxyBinding MOCK_METHOD(void, RegisterEventBinding, (std::string_view, ProxyEventBindingBase&), (noexcept, override)); MOCK_METHOD(void, UnregisterEventBinding, (std::string_view), (noexcept, override)); MOCK_METHOD(Result, SetupMethods, (), (override)); + MOCK_METHOD(void, PrepareDeinitialize, (), (override)); + MOCK_METHOD(void, FinalizeDeinitialize, (), (override)); }; class ProxyFacade : public ProxyBinding @@ -61,6 +63,16 @@ class ProxyFacade : public ProxyBinding return proxy_.SetupMethods(); } + void PrepareDeinitialize() override + { + proxy_.PrepareDeinitialize(); + } + + void FinalizeDeinitialize() override + { + proxy_.FinalizeDeinitialize(); + } + private: Proxy& proxy_; }; diff --git a/score/mw/com/impl/generic_proxy.cpp b/score/mw/com/impl/generic_proxy.cpp index 3e2091287..bb5d9c70e 100644 --- a/score/mw/com/impl/generic_proxy.cpp +++ b/score/mw/com/impl/generic_proxy.cpp @@ -86,10 +86,38 @@ Result GenericProxy::Create(HandleType instance_handle) noexcept } GenericProxy::GenericProxy(std::unique_ptr proxy_binding, HandleType instance_handle) - : ProxyBase{std::move(proxy_binding), std::move(instance_handle)} + : ProxyBase{std::move(proxy_binding), std::move(instance_handle)}, is_proxy_owner_{true} { } +GenericProxy::~GenericProxy() noexcept +{ + if (is_proxy_owner_.IsSet()) + { + this->Deinitialize(); + } +} + +GenericProxy::GenericProxy(GenericProxy&& other) noexcept + : ProxyBase{std::move(other)}, events_{std::move(other.events_)}, is_proxy_owner_{std::move(other.is_proxy_owner_)} +{ +} + +GenericProxy& GenericProxy::operator=(GenericProxy&& other) noexcept +{ + if (&other != this) + { + if (is_proxy_owner_.IsSet()) + { + this->Deinitialize(); + } + ProxyBase::operator=(std::move(other)); + events_ = std::move(other.events_); + is_proxy_owner_ = std::move(other.is_proxy_owner_); + } + return *this; +} + void GenericProxy::FillEventMap(const std::vector& event_names) noexcept { for (const auto event_name : event_names) diff --git a/score/mw/com/impl/generic_proxy.h b/score/mw/com/impl/generic_proxy.h index afd71027c..638e623b7 100644 --- a/score/mw/com/impl/generic_proxy.h +++ b/score/mw/com/impl/generic_proxy.h @@ -18,6 +18,7 @@ #ifndef SCORE_MW_COM_IMPL_GENERIC_PROXY_H #define SCORE_MW_COM_IMPL_GENERIC_PROXY_H +#include "score/mw/com/impl/flag_owner.h" #include "score/mw/com/impl/generic_proxy_event.h" #include "score/mw/com/impl/handle_type.h" #include "score/mw/com/impl/proxy_base.h" @@ -69,6 +70,14 @@ class GenericProxy : public ProxyBase */ static Result Create(HandleType instance_handle) noexcept; + ~GenericProxy() noexcept; + + GenericProxy(const GenericProxy&) = delete; + GenericProxy& operator=(const GenericProxy&) = delete; + + GenericProxy(GenericProxy&& other) noexcept; + GenericProxy& operator=(GenericProxy&& other) noexcept; + /** * \api * \brief Returns a reference to the event map. @@ -82,6 +91,10 @@ class GenericProxy : public ProxyBase void FillEventMap(const std::vector& event_names) noexcept; EventMap events_; + + /// Flag which is checked before calling Unsubscribe in the destructor. + /// Cleared on move so the moved-from instance does not call Unsubscribe. + FlagOwner is_proxy_owner_; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/mocking/proxy_event_mock_test.cpp b/score/mw/com/impl/mocking/proxy_event_mock_test.cpp index 30966ed05..1b77119ae 100644 --- a/score/mw/com/impl/mocking/proxy_event_mock_test.cpp +++ b/score/mw/com/impl/mocking/proxy_event_mock_test.cpp @@ -47,6 +47,8 @@ class ProxyEventFieldMockFixture : public ::testing::Test ProxyEventFieldMockFixture() { unit_.InjectMock(proxy_service_element_mock_); + + ON_CALL(this->proxy_service_element_mock_, Subscribe(_)).WillByDefault(Return(Result{})); } ProxyEventFieldMock proxy_service_element_mock_{}; @@ -104,7 +106,8 @@ TYPED_TEST(ProxyEventFieldMockFixture, SubscribeReturnsErrorWhenMockReturnsError TYPED_TEST(ProxyEventFieldMockFixture, UnsubscribeDispatchesToMockAfterInjectingMock) { - // Given a ProxyEvent constructed with an empty binding and an injected mock + // Given a ProxyEvent constructed with an empty binding and an injected mock which is currently subscribed + std::ignore = this->unit_.Subscribe(kDummyMaxSampleCount); // Expecting that Unsubscribe will be called on the mock EXPECT_CALL(this->proxy_service_element_mock_, Unsubscribe()); diff --git a/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp b/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp index 674527449..edc0d410c 100644 --- a/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp +++ b/score/mw/com/impl/mocking/proxy_wrapper_class_test_view_test.cpp @@ -172,12 +172,22 @@ TEST_F(ProxyWrapperTestClassCreateFixture, CallingFunctionsOnMockProxyDispatches proxy.some_event_2.InjectMock(proxy_event_mock_2); proxy.some_field.InjectMock(proxy_field_mock); - // Expecting that OfferService will be called on the Proxy mock and Unsubscribe on the event and field mocks + // Expecting that Subscribe on each event and field mock returns success (so the per-event subscribed + // flag is set; otherwise the later Unsubscribe is a silent no-op). + EXPECT_CALL(proxy_event_mock, Subscribe(_)).WillOnce(Return(score::Result{})); + EXPECT_CALL(proxy_event_mock_2, Subscribe(_)).WillOnce(Return(score::Result{})); + EXPECT_CALL(proxy_field_mock, Subscribe(_)).WillOnce(Return(score::Result{})); + + // Expecting that Unsubscribe is called on each event and field mock exactly once. EXPECT_CALL(proxy_event_mock, Unsubscribe()); EXPECT_CALL(proxy_event_mock_2, Unsubscribe()); EXPECT_CALL(proxy_field_mock, Unsubscribe()); - // When calling Unsubscribe on the events and fields. + // When subscribing and then unsubscribing the events and fields. + std::ignore = proxy.some_event.Subscribe(1); + std::ignore = proxy.some_event_2.Subscribe(1); + std::ignore = proxy.some_field.Subscribe(1); + proxy.some_event.Unsubscribe(); proxy.some_event_2.Unsubscribe(); proxy.some_field.Unsubscribe(); @@ -217,10 +227,14 @@ TEST_F(ProxyWrapperTestClassEventsOnlyCreateFixture, CallingFunctionsOnMockProxy auto& proxy_event_mock = std::get<0>(events_tuple).mock; proxy.some_event.InjectMock(proxy_event_mock); + // Expecting that Subscribe returns success on the mock + EXPECT_CALL(proxy_event_mock, Subscribe(1)).WillOnce(Return(score::Result{})); + // Expecting that Unsubscribe is called on the event EXPECT_CALL(proxy_event_mock, Unsubscribe()); - // When calling Unsubscribe on the event + // When subscribing and then unsubscribing the event + std::ignore = proxy.some_event.Subscribe(1); proxy.some_event.Unsubscribe(); } @@ -256,10 +270,14 @@ TEST_F(ProxyWrapperTestClassFieldsOnlyCreateFixture, CallingFunctionsOnMockProxy auto& proxy_field_mock = (std::get<0>(fields_tuple).mock); proxy.some_field.InjectMock(proxy_field_mock); + // Expecting that Subscribe returns success on the mock + EXPECT_CALL(proxy_field_mock, Subscribe(1)).WillOnce(Return(score::Result{})); + // Expecting that Unsubscribe is called on the field EXPECT_CALL(proxy_field_mock, Unsubscribe()); - // When calling Unsubscribe on the field + // When subscribing and then unsubscribing the field + std::ignore = proxy.some_field.Subscribe(1); proxy.some_field.Unsubscribe(); } diff --git a/score/mw/com/impl/plumbing/proxy_binding_factory_test.cpp b/score/mw/com/impl/plumbing/proxy_binding_factory_test.cpp index f3079c474..27c7338f9 100644 --- a/score/mw/com/impl/plumbing/proxy_binding_factory_test.cpp +++ b/score/mw/com/impl/plumbing/proxy_binding_factory_test.cpp @@ -76,7 +76,21 @@ class SkeletonBindingGuard std::unique_ptr skeleton_binding_; }; -using ProxyBindingFactoryCreateFixture = lola::ProxyMockedMemoryFixture; +class ProxyBindingFactoryCreateFixture : public lola::ProxyMockedMemoryFixture +{ + protected: + void TearDown() override + { + if (created_binding_ != nullptr) + { + created_binding_->PrepareDeinitialize(); + created_binding_->FinalizeDeinitialize(); + } + lola::ProxyMockedMemoryFixture::TearDown(); + } + + std::unique_ptr created_binding_{nullptr}; +}; score::cpp::optional GetInstanceId(score::mw::com::impl::InstanceIdentifier identifier) { const InstanceIdentifierView identifier_view{identifier}; @@ -108,10 +122,10 @@ TEST_F(ProxyBindingFactoryCreateFixture, CanCreateLoLaProxy) .WillByDefault(Return(make_FindServiceHandle(10U))); // When creating a proxy with that - const auto result = ProxyBindingFactory::Create(handle); + created_binding_ = ProxyBindingFactory::Create(handle); // Then no nullptr is returned - EXPECT_NE(result, nullptr); + EXPECT_NE(created_binding_, nullptr); } TEST_F(ProxyBindingFactoryCreateFixture, CannotCreateBlank) diff --git a/score/mw/com/impl/plumbing/proxy_event_field_binding_factory_test.cpp b/score/mw/com/impl/plumbing/proxy_event_field_binding_factory_test.cpp index 5e61cec99..e954c9839 100644 --- a/score/mw/com/impl/plumbing/proxy_event_field_binding_factory_test.cpp +++ b/score/mw/com/impl/plumbing/proxy_event_field_binding_factory_test.cpp @@ -140,6 +140,20 @@ class ProxyServiceElementBindingFactoryParamaterisedFixture : public lola::Proxy ServiceElementTypes service_element_type_{GetParam()}; std::unique_ptr proxy_base_{nullptr}; DummyInstanceIdentifierBuilder dummy_instance_identifier_builder_{}; + + void TearDown() override + { + if (proxy_base_ != nullptr) + { + auto* const binding = ProxyBaseView{*proxy_base_}.GetBinding(); + if (binding != nullptr) + { + binding->PrepareDeinitialize(); + binding->FinalizeDeinitialize(); + } + } + lola::ProxyMockedMemoryFixture::TearDown(); + } }; INSTANTIATE_TEST_CASE_P(ProxyServiceElementBindingFactoryParamaterisedFixture, diff --git a/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp b/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp index a4a7eaa6c..30b9718e3 100644 --- a/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp +++ b/score/mw/com/impl/plumbing/proxy_method_binding_factory_test.cpp @@ -100,6 +100,20 @@ class ProxyMethodFactoryFixture : public lola::ProxyMockedMemoryFixture return ProxyBaseView{*proxy_base_}.GetBinding(); } + void TearDown() override + { + if (proxy_base_ != nullptr) + { + auto* const binding = ProxyBaseView{*proxy_base_}.GetBinding(); + if (binding != nullptr) + { + binding->PrepareDeinitialize(); + binding->FinalizeDeinitialize(); + } + } + lola::ProxyMockedMemoryFixture::TearDown(); + } + private: std::unique_ptr proxy_base_{nullptr}; DummyInstanceIdentifierBuilder dummy_instance_identifier_builder_{}; diff --git a/score/mw/com/impl/proxy_base.cpp b/score/mw/com/impl/proxy_base.cpp index 26209a6b3..b0ea4a26f 100644 --- a/score/mw/com/impl/proxy_base.cpp +++ b/score/mw/com/impl/proxy_base.cpp @@ -174,6 +174,26 @@ Result ProxyBase::SetupMethods() return {}; } +void ProxyBase::Deinitialize() +{ + if (proxy_binding_ != nullptr) + { + proxy_binding_->PrepareDeinitialize(); + } + for (auto& event : events_) + { + event.second.get().Unsubscribe(); + } + for (auto& field : fields_) + { + field.second.get().Unsubscribe(); + } + if (proxy_binding_ != nullptr) + { + proxy_binding_->FinalizeDeinitialize(); + } +} + ProxyBaseView::ProxyBaseView(ProxyBase& proxy_base) noexcept : proxy_base_(proxy_base) {} ProxyBinding* ProxyBaseView::GetBinding() noexcept diff --git a/score/mw/com/impl/proxy_base.h b/score/mw/com/impl/proxy_base.h index 60e608832..67e7c0920 100644 --- a/score/mw/com/impl/proxy_base.h +++ b/score/mw/com/impl/proxy_base.h @@ -131,6 +131,11 @@ class ProxyBase ProxyBase(ProxyBase&& other) noexcept; ProxyBase& operator=(ProxyBase&& other) noexcept; + /// \brief Deinitialize all events and fields and tears down binding-level state. + /// Must only be invoked from the wrapper class destructor (ProxyWrapperClass / GenericProxy). + /// Calling this from user code leaves the binding in a torn-down state mid-life. + void Deinitialize(); + bool AreBindingsValid() const noexcept { const bool is_proxy_binding_valid{proxy_binding_ != nullptr}; diff --git a/score/mw/com/impl/proxy_binding.h b/score/mw/com/impl/proxy_binding.h index c24524d36..f3171a0ff 100644 --- a/score/mw/com/impl/proxy_binding.h +++ b/score/mw/com/impl/proxy_binding.h @@ -62,6 +62,29 @@ class ProxyBinding virtual void UnregisterEventBinding(const std::string_view service_element_name) noexcept = 0; virtual Result SetupMethods() = 0; + + /// \brief Contains all cleanup logic for the binding that needs to be executed before any of the service elements + /// are deinitialized (Called by ProxyBase::Deinitialize). + /// \pre Must be called exactly once per binding lifetime. + /// + /// Class hierarchy: + /// ProxyWrapperClass : Interface + /// --> Interface = MyInterface (declares some_event/some_field/some_method as members) + /// --> ProxyTrait::Base = ProxyBase (holds references to those events/fields in events_/fields_) + /// + /// C++ destruction order: + /// 1. ~ProxyWrapperClass body + /// 2. ~MyInterface body, then its members (some_event/some_field/some_method) destroyed + /// 3. ~ProxyBase body, then its members (events_ map of references, proxy_binding_) destroyed + /// + /// By step 3 the events and fields are gone, so any binding callback that still touches them would be + /// a use-after-free. PrepareDeinitialize has to run at step 1 while they are still around. + virtual void PrepareDeinitialize() = 0; + + /// \brief Contains all cleanup logic for the binding that needs to be executed after any of the service elements + /// are deinitialized (Called by ProxyBase::Deinitialize). + /// \pre Must be called after PrepareDeinitialize. + virtual void FinalizeDeinitialize() = 0; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/proxy_binding_test.cpp b/score/mw/com/impl/proxy_binding_test.cpp index fd7ba4dd2..44aa425a5 100644 --- a/score/mw/com/impl/proxy_binding_test.cpp +++ b/score/mw/com/impl/proxy_binding_test.cpp @@ -35,6 +35,8 @@ class MyProxy final : public ProxyBinding { return {}; } + void PrepareDeinitialize() override {} + void FinalizeDeinitialize() override {} }; TEST(ProxyBindingTest, ProxyBindingShouldNotBeCopyable) diff --git a/score/mw/com/impl/proxy_event_base.cpp b/score/mw/com/impl/proxy_event_base.cpp index 3b82ac999..5d9e1d63a 100644 --- a/score/mw/com/impl/proxy_event_base.cpp +++ b/score/mw/com/impl/proxy_event_base.cpp @@ -17,6 +17,7 @@ #include "score/mw/com/impl/proxy_binding.h" #include "score/mw/com/impl/scoped_event_receive_handler.h" #include "score/mw/com/impl/tracing/proxy_event_tracing.h" +#include "score/scope_exit/scope_exit.h" #include "score/mw/log/logging.h" #include "score/result/result.h" @@ -88,6 +89,7 @@ ProxyEventBase::ProxyEventBase(ProxyBase& proxy_base, event_binding_registration_guard_{ std::make_unique(proxy_binding_ptr, binding_base_.get(), event_name)}, proxy_event_base_mock_{nullptr}, + is_subscribed_flag_{false}, receive_handler_scope_{} { } @@ -116,7 +118,12 @@ Result ProxyEventBase::Subscribe(const std::size_t max_sample_count) noexc { if (proxy_event_base_mock_ != nullptr) { - return proxy_event_base_mock_->Subscribe(max_sample_count); + auto mock_result = proxy_event_base_mock_->Subscribe(max_sample_count); + if (mock_result.has_value()) + { + is_subscribed_flag_.Set(); + } + return mock_result; } tracing::TraceSubscribe(tracing_data_, *binding_base_, max_sample_count); @@ -142,17 +149,31 @@ Result ProxyEventBase::Subscribe(const std::size_t max_sample_count) noexc return MakeUnexpected(ComErrc::kMaxSampleCountNotRealizable); } } + is_subscribed_flag_.Set(); return {}; } void ProxyEventBase::Unsubscribe() noexcept { + // Unsubscribe before a successful Subscribe is a silent no-op. + if (!is_subscribed_flag_.IsSet()) + { + return; + } + + utils::ScopeExit clear_subscribed_flag_on_exit{[&is_subscribed_flag_ = is_subscribed_flag_] { + is_subscribed_flag_.Clear(); + }}; + if (proxy_event_base_mock_ != nullptr) { proxy_event_base_mock_->Unsubscribe(); return; } + SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(binding_base_ != nullptr, + "binding_base_ must be set if Subscribe completed successfully."); + tracing::TraceUnsubscribe(tracing_data_, *binding_base_); if (GetSubscriptionState() != SubscriptionState::kNotSubscribed) diff --git a/score/mw/com/impl/proxy_event_base.h b/score/mw/com/impl/proxy_event_base.h index 55ebcb83a..85ac17a43 100644 --- a/score/mw/com/impl/proxy_event_base.h +++ b/score/mw/com/impl/proxy_event_base.h @@ -15,6 +15,7 @@ #define SCORE_MW_COM_IMPL_PROXY_EVENT_BASE_H #include "score/mw/com/impl/event_receive_handler.h" +#include "score/mw/com/impl/flag_owner.h" #include "score/mw/com/impl/proxy_binding.h" #include "score/mw/com/impl/proxy_event_binding_base.h" #include "score/mw/com/impl/sample_reference_tracker.h" @@ -220,6 +221,7 @@ class ProxyEventBase /// (because trying to expire the scope in which we are running, would lead to a deadlock) void ExpireReceiveHandlerScopeIfNotInHandler(); + FlagOwner is_subscribed_flag_; safecpp::Scope<> receive_handler_scope_; std::shared_ptr receive_handler_ptr_; /// \brief thread local variable, which indicates, whether the current thread is within the call context of an diff --git a/score/mw/com/impl/proxy_event_base_test.cpp b/score/mw/com/impl/proxy_event_base_test.cpp index 39521fd82..780c59a62 100644 --- a/score/mw/com/impl/proxy_event_base_test.cpp +++ b/score/mw/com/impl/proxy_event_base_test.cpp @@ -77,6 +77,7 @@ class ProxyEventBaseFixture : public ::testing::Test std::make_unique(empty_proxy_, std::move(mock_service_element_binding_ptr), kEventName); ON_CALL(*mock_service_element_binding_, SetReceiveHandler(_)).WillByDefault(Return(score::Result{})); + ON_CALL(*mock_service_element_binding_, Subscribe(_)).WillByDefault(Return(score::Result{})); } DummyProxy empty_proxy_{std::make_unique(), @@ -257,17 +258,22 @@ TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileSubscribedCa this->RecordProperty("Priority", "1"); this->RecordProperty("DerivationTechnique", "Analysis of requirements"); + const std::uint8_t max_sample_count{1U}; + // Given a Service Element, that is connected to a mock binding this->CreateServiceElement(); - // and that GetSubscriptionState will be called once and indicate that the Proxy is already subscribed. + // and given that GetSubscriptionState returns kNotSubscribed then kSubscribed EXPECT_CALL(*this->mock_service_element_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) .WillOnce(Return(SubscriptionState::kSubscribed)); // Expecting that the Unsubscribe will be called on the binding EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()); - // When Unsubscribe is called on the ProxyBase + // When Subscribe and then Unsubscribe is called on the ProxyElement + std::ignore = this->service_element_->Subscribe(max_sample_count); + this->service_element_->Unsubscribe(); } @@ -280,17 +286,22 @@ TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileSubscription this->RecordProperty("Priority", "1"); this->RecordProperty("DerivationTechnique", "Analysis of requirements"); + const std::uint8_t max_sample_count{1U}; + // Given a Service Element, that is connected to a mock binding this->CreateServiceElement(); - // and that GetSubscriptionState will be called once and indicate that the Proxy subscription is pending. + // and given that GetSubscriptionState returns kNotSubscribed then kSubscriptionPending EXPECT_CALL(*this->mock_service_element_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) .WillOnce(Return(SubscriptionState::kSubscriptionPending)); // Expecting that the Unsubscribe will be called on the binding EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()); - // When Unsubscribe is called on the ProxyBase + // When Subscribe and then Unsubscribe is called on the ProxyElement + std::ignore = this->service_element_->Subscribe(max_sample_count); + this->service_element_->Unsubscribe(); } @@ -305,10 +316,6 @@ TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileNotSubscribe // Given a Service Element, that is connected to a mock binding this->CreateServiceElement(); - // and that GetSubscriptionState will be called once and indicate that the Proxy is not subscribed. - EXPECT_CALL(*this->mock_service_element_binding_, GetSubscriptionState()) - .WillOnce(Return(SubscriptionState::kNotSubscribed)); - // Expecting that the Unsubscribe will not be called on the binding EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()).Times(0); @@ -316,24 +323,51 @@ TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeWhileNotSubscribe this->service_element_->Unsubscribe(); } +TYPED_TEST(ProxyEventBaseUnsubscribeFixture, CallingUnsubscribeTwiceAfterSubscribeOnlyDispatchesToBindingOnce) +{ + const std::uint8_t max_sample_count{1U}; + + // Given a Service Element, that is connected to a mock binding + this->CreateServiceElement(); + + // and given that GetSubscriptionState returns kNotSubscribed then kSubscribed + EXPECT_CALL(*this->mock_service_element_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) + .WillOnce(Return(SubscriptionState::kSubscribed)); + + // Expecting that Unsubscribe will be called on the binding exactly once across both Unsubscribe calls + EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()).Times(1); + + // When Subscribe is called and then Unsubscribe is called twice on the Service Element + std::ignore = this->service_element_->Subscribe(max_sample_count); + this->service_element_->Unsubscribe(); + this->service_element_->Unsubscribe(); + + // Then Unsubscribe is dispatched to the binding only once +} + TYPED_TEST(ProxyEventBaseUnsubscribeDeathTest, CallingUnsubscribeWhileASampleIsStillReferencedTerminates) { const auto CallUnsubscribeWhileSampleIsStillAllocated = [this]() { + const std::uint8_t max_sample_count{1U}; + // Given a Service Element, that is connected to a mock binding this->CreateServiceElement(); - // Expecting that GetSubscriptionState will be called once and indicate that the Proxy is not subscribed. + // and given that GetSubscriptionState returns kNotSubscribed then kSubscribed EXPECT_CALL(*this->mock_service_element_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) .WillOnce(Return(SubscriptionState::kSubscribed)); + // Expecting that Unsubscribe will be called on the binding + EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()); + + std::ignore = this->service_element_->Subscribe(max_sample_count); + // and that a sample has been allocated and not yet released auto& tracker = ProxyEventBaseAttorney{*this->service_element_}.GetSampleReferenceTracker(); - tracker.Reset(1); const auto allocation_result = tracker.Allocate(1); - // and that the Unsubscribe will be called on the binding - EXPECT_CALL(*this->mock_service_element_binding_, Unsubscribe()); - // Then we terminate when Unsubscribe is called on the ProxyBase this->service_element_->Unsubscribe(); }; diff --git a/score/mw/com/impl/proxy_event_test.cpp b/score/mw/com/impl/proxy_event_test.cpp index 0bc254371..3a231da96 100644 --- a/score/mw/com/impl/proxy_event_test.cpp +++ b/score/mw/com/impl/proxy_event_test.cpp @@ -72,19 +72,19 @@ struct ProxyEventStruct { using SampleType = TestSampleType; using ProxyEventType = ProxyEvent; - using MockProxyEventType = StrictMock>; + using MockProxyEventType = NiceMock>; }; struct GenericProxyEventStruct { using SampleType = void; using ProxyEventType = GenericProxyEvent; - using MockProxyEventType = StrictMock; + using MockProxyEventType = NiceMock; }; struct ProxyFieldStruct { using SampleType = TestSampleType; using ProxyEventType = ProxyField; - using MockProxyEventType = StrictMock>; + using MockProxyEventType = NiceMock>; }; /// \brief Templated test fixture for ProxyEvent functionality that works for both ProxyEvent and GenericProxyEvent @@ -108,6 +108,7 @@ class ProxyEventFixture : public ::testing::Test mock_proxy_event_{*mock_proxy_event_ptr_}, proxy_event_{empty_proxy_, std::move(mock_proxy_event_ptr_), kEventName} { + ON_CALL(mock_proxy_event_, Subscribe(_)).WillByDefault(Return(score::Result{})); } ProxyBase empty_proxy_; @@ -257,24 +258,27 @@ TYPED_TEST(ProxyEventDeathTest, DieOnUnsubscribingWhileHoldingSamplePtrs) const std::size_t max_num_samples{1}; - EXPECT_CALL(Base::mock_proxy_event_, Subscribe(max_num_samples)); - EXPECT_CALL(Base::mock_proxy_event_, GetNewSamples(_, _)); - EXPECT_CALL(Base::mock_proxy_event_, Unsubscribe()).Times(::testing::AtMost(1)); + // Given a subscribed proxy event that has delivered a sample which is still held - Base::mock_proxy_event_.PushFakeSample(3U); EXPECT_CALL(Base::mock_proxy_event_, GetSubscriptionState()) - .WillOnce(::testing::Return(SubscriptionState::kNotSubscribed)); + .WillOnce(Return(SubscriptionState::kNotSubscribed)) + .WillRepeatedly(Return(SubscriptionState::kSubscribed)); - score::cpp::optional> ptr{}; + Base::mock_proxy_event_.PushFakeSample(3U); std::ignore = Base::proxy_event_.Subscribe(max_num_samples); + + score::cpp::optional> held_sample{}; Result num_samples = Base::proxy_event_.GetNewSamples( - [&ptr](SamplePtr new_sample) { - ptr = std::move(new_sample); + [&held_sample](SamplePtr sample) { + held_sample = std::move(sample); }, - 1U); + max_num_samples); ASSERT_TRUE(num_samples.has_value()); - ASSERT_TRUE(ptr.has_value()); + ASSERT_TRUE(held_sample.has_value()); EXPECT_EQ(*num_samples, 1U); + + // When Unsubscribe is called while still holding the sample + // Then the process terminates EXPECT_DEATH(Base::proxy_event_.Unsubscribe(), ".*"); } @@ -282,9 +286,21 @@ TYPED_TEST(ProxyEventFixture, UnsubscribeWhileNotHoldingSamplePtrs) { using Base = ProxyEventFixture; - Base::mock_proxy_event_.PushFakeSample(3U); + // Given a subscribed proxy event that received a sample but no longer holds it EXPECT_CALL(Base::mock_proxy_event_, GetSubscriptionState()) - .WillOnce(::testing::Return(SubscriptionState::kNotSubscribed)); + .WillOnce(Return(SubscriptionState::kNotSubscribed)) + .WillRepeatedly(Return(SubscriptionState::kSubscribed)); + + Base::mock_proxy_event_.PushFakeSample(3U); + std::ignore = Base::proxy_event_.Subscribe(1U); + + std::ignore = Base::proxy_event_.GetNewSamples( + [](SamplePtr) noexcept { + // The sample goes out of the scope here. so, nothing is held when Unsubscribe is called next. + }, + 1U); + + // When Unsubscribe is called while NOT holding any sample Base::proxy_event_.Unsubscribe(); // Then nothing bad happens diff --git a/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp b/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp index 6549d0700..78f6d1f99 100644 --- a/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp +++ b/score/mw/com/impl/tracing/test/proxy_event_tracing_test.cpp @@ -103,6 +103,10 @@ class ProxyEventTracingFixture : public ::testing::Test ExpectProxyServiceElementBindingCreation(proxy_service_element_binding_factory_mock_guard_); ON_CALL(*mock_proxy_event_binding_, SetReceiveHandler(_)).WillByDefault(Return(score::Result{})); + ON_CALL(*mock_proxy_event_binding_, GetBindingType()).WillByDefault(Return(BindingType::kLoLa)); + ON_CALL(*mock_proxy_event_binding_, GetSubscriptionState()) + .WillByDefault(Return(SubscriptionState::kNotSubscribed)); + ON_CALL(*mock_proxy_event_binding_, Subscribe(_)).WillByDefault(Return(score::Result{})); } void CreateProxy() @@ -480,17 +484,11 @@ TYPED_TEST(ProxyEventTracingSubscribeFixture, SubscribeCallsAreTracedWhenEnabled return {}; }))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that GetSubscriptionState is called once and indicates that the Service Element is currently not // subscribed EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) .WillOnce(Return(SubscriptionState::kNotSubscribed)); - // and that Subscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Subscribe(max_sample_count)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -550,17 +548,11 @@ TYPED_TEST(ProxyEventTracingSubscribeFixture, return MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance); }))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that GetSubscriptionState is called once and indicates that the Service Element is currently not // subscribed EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) .WillOnce(Return(SubscriptionState::kNotSubscribed)); - // and that Subscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Subscribe(max_sample_count)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -628,17 +620,11 @@ TYPED_TEST(ProxyEventTracingSubscribeFixture, return MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints); }))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that GetSubscriptionState is called once and indicates that the Service Element is currently not // subscribed EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) .WillOnce(Return(SubscriptionState::kNotSubscribed)); - // and that Subscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Subscribe(max_sample_count)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -687,9 +673,6 @@ TYPED_TEST(ProxyEventTracingSubscribeFixture, SubscribeCallsAreNotTracedWhenDisa EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) .WillOnce(Return(SubscriptionState::kNotSubscribed)); - // and that Subscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Subscribe(max_sample_count)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -715,6 +698,8 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, UnsubscribeCallsAreTracedWhenEna this->CreateServiceElementInstanceIdentifierView( typename ProxyEventTracingFixture::TracePointType{}); + const std::size_t max_sample_count{1U}; + // Expecting that the runtime returns a mocked TracingRuntime and TracingFilterConfig StrictMock tracing_runtime_mock{}; EXPECT_CALL(this->runtime_mock_guard_.runtime_mock_, GetTracingRuntime()).WillOnce(Return(&tracing_runtime_mock)); @@ -726,6 +711,11 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, UnsubscribeCallsAreTracedWhenEna typename ProxyEventTracingFixture::TracePointType>( expected_enabled_trace_points, kServiceIdentifierStringView, kServiceElementName, kInstanceSpecifierStringView); + // and given that GetSubscriptionState returns kNotSubscribed then kSubscribed + EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) + .WillOnce(Return(SubscriptionState::kSubscribed)); + // Then a trace call relating to Unsubscribe should be called with no data tracing::ITracingRuntime::TracePointType trace_point_type{ ProxyEventTracingFixture::TracePointType::UNSUBSCRIBE}; @@ -738,20 +728,15 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, UnsubscribeCallsAreTracedWhenEna 0U)) .WillOnce(Return(Result{})); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - - // and that GetSubscriptionState is called once and indicates that the Service Element is currently subscribed - EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) - .WillOnce(Return(SubscriptionState::kSubscribed)); - // and that Unsubscribe will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, Unsubscribe()); // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); - // and unsubscribe is called on the event + // and Subscribe and then Unsubscribe are called on the event + std::ignore = this->proxy_->my_service_element_.Subscribe(max_sample_count); + this->proxy_->my_service_element_.Unsubscribe(); } @@ -768,13 +753,14 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, ProxyEventTracingData expected_enabled_trace_points{}; expected_enabled_trace_points.enable_unsubscribe = true; - expected_enabled_trace_points.enable_subscribe = true; expected_enabled_trace_points.enable_call_receive_handler = true; const tracing::ServiceElementInstanceIdentifierView expected_service_element_instance_identifier_view = this->CreateServiceElementInstanceIdentifierView( typename ProxyEventTracingFixture::TracePointType{}); + const std::size_t max_sample_count{1U}; + // Expecting that the runtime returns a mocked TracingRuntime and TracingFilterConfig tracing::TracingRuntimeMock tracing_runtime_mock{}; EXPECT_CALL(this->runtime_mock_guard_.runtime_mock_, GetTracingRuntime()).WillOnce(Return(&tracing_runtime_mock)); @@ -799,20 +785,12 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - - // and that GetSubscriptionState is called once and indicates that the Service Element is currently subscribed - EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) - .WillOnce(Return(SubscriptionState::kSubscribed)); - - // and that Unsubscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Unsubscribe()); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); - // and unsubscribe is called on the event + // and Subscribe and then Unsubscribe are called on the event + std::ignore = this->proxy_->my_service_element_.Subscribe(max_sample_count); + this->proxy_->my_service_element_.Unsubscribe(); // Then the specific trace point instance should now be disabled @@ -837,13 +815,14 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, ProxyEventTracingData expected_enabled_trace_points{}; expected_enabled_trace_points.enable_unsubscribe = true; - expected_enabled_trace_points.enable_subscribe = true; expected_enabled_trace_points.enable_call_receive_handler = true; const tracing::ServiceElementInstanceIdentifierView expected_service_element_instance_identifier_view = this->CreateServiceElementInstanceIdentifierView( typename ProxyEventTracingFixture::TracePointType{}); + const std::size_t max_sample_count{1U}; + // Expecting that the runtime returns a mocked TracingRuntime and TracingFilterConfig tracing::TracingRuntimeMock tracing_runtime_mock{}; EXPECT_CALL(this->runtime_mock_guard_.runtime_mock_, GetTracingRuntime()).WillOnce(Return(&tracing_runtime_mock)); @@ -868,20 +847,12 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - - // and that GetSubscriptionState is called once and indicates that the Service Element is currently subscribed - EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) - .WillOnce(Return(SubscriptionState::kSubscribed)); - - // and that Unsubscribe will be called on the binding - EXPECT_CALL(*this->mock_proxy_event_binding_, Unsubscribe()); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); - // and unsubscribe is called on the event + // and Subscribe and then Unsubscribe are called on the event + std::ignore = this->proxy_->my_service_element_.Subscribe(max_sample_count); + this->proxy_->my_service_element_.Unsubscribe(); // Then all trace point instances should now be disabled @@ -919,17 +890,22 @@ TYPED_TEST(ProxyEventTracingUnsubscribeFixture, UnsubscribeCallsAreNotTracedWhen // Then a trace call relating to Unsubscribe should never be called - // and that GetSubscriptionState is called once and indicates that the Service Element is currently subscribed + const std::size_t max_sample_count{1U}; + + // and given that GetSubscriptionState returns kNotSubscribed then kSubscribed EXPECT_CALL(*this->mock_proxy_event_binding_, GetSubscriptionState()) + .WillOnce(Return(SubscriptionState::kNotSubscribed)) .WillOnce(Return(SubscriptionState::kSubscribed)); - // and that Unsubscribe will be called on the binding + // and given that Unsubscribe is called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, Unsubscribe()); // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); - // and unsubscribe is called on the event + // and Subscribe and then Unsubscribe are called on the event + std::ignore = this->proxy_->my_service_element_.Subscribe(max_sample_count); + this->proxy_->my_service_element_.Unsubscribe(); } @@ -974,9 +950,6 @@ TYPED_TEST(ProxyEventTracingSetReceiveHandlerFixture, SetReceiveHandlerCallsAreT 0U)) .WillOnce(Return(Result{})); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that SetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, SetReceiveHandler(_)); @@ -1031,9 +1004,6 @@ TYPED_TEST(ProxyEventTracingSetReceiveHandlerFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that SetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, SetReceiveHandler(_)); @@ -1095,9 +1065,6 @@ TYPED_TEST(ProxyEventTracingSetReceiveHandlerFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that SetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, SetReceiveHandler(_)); @@ -1202,9 +1169,6 @@ TYPED_TEST(ProxyEventTracingReceiveHandlerCallbackFixture, ReceiveHandlerCallbac nullptr, 0U)); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1271,9 +1235,6 @@ TYPED_TEST(ProxyEventTracingReceiveHandlerCallbackFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1347,9 +1308,6 @@ TYPED_TEST(ProxyEventTracingReceiveHandlerCallbackFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1460,9 +1418,6 @@ TYPED_TEST(ProxyEventTracingUnsetReceiveHandlerFixture, UnsetReceiveHandlerCalls nullptr, 0U)); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that UnsetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, UnsetReceiveHandler()); @@ -1520,9 +1475,6 @@ TYPED_TEST(ProxyEventTracingUnsetReceiveHandlerFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that UnsetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, UnsetReceiveHandler()); @@ -1587,9 +1539,6 @@ TYPED_TEST(ProxyEventTracingUnsetReceiveHandlerFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // and that UnsetReceiveHandler will be called on the binding EXPECT_CALL(*this->mock_proxy_event_binding_, UnsetReceiveHandler()); @@ -1693,9 +1642,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesFixture, GetNewSamplesCallsAreTracedWhe nullptr, 0U)); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1748,9 +1694,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1811,9 +1754,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1920,9 +1860,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesCallbackFixture, GetNewSamplesCallbackC nullptr, 0U)); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -1995,9 +1932,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesCallbackFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableTracePointInstance))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); @@ -2077,9 +2011,6 @@ TYPED_TEST(ProxyEventTracingGetNewSamplesCallbackFixture, 0U)) .WillOnce(Return(MakeUnexpected(tracing::TraceErrorCode::TraceErrorDisableAllTracePoints))); - // and that GetBindingType is called on the proxy event binding - EXPECT_CALL(*this->mock_proxy_event_binding_, GetBindingType()).WillOnce(Return(BindingType::kLoLa)); - // When a Proxy containing a ProxyEvent is created based on a lola deployment this->CreateProxy(); diff --git a/score/mw/com/impl/traits.h b/score/mw/com/impl/traits.h index 1c51a0587..891933ee5 100644 --- a/score/mw/com/impl/traits.h +++ b/score/mw/com/impl/traits.h @@ -274,7 +274,7 @@ class SkeletonWrapperClass : public Interface private: explicit SkeletonWrapperClass(const InstanceIdentifier& instance_id, std::unique_ptr skeleton_binding) - : Interface{std::move(skeleton_binding), instance_id} + : Interface{std::move(skeleton_binding), instance_id}, is_service_owner_{true} { } @@ -304,7 +304,7 @@ class SkeletonWrapperClass : public Interface /// /// This flag is always set for a Skeleton except when a Skeleton is moved. In this case, this flag will be cleared /// in the moved-from class so that that object doesn't call StopFindService on destruction. - FlagOwner is_service_owner_{true}; + FlagOwner is_service_owner_; }; template