From 7e9975a309240a7c050f8f319e4a1e912fbf0f54 Mon Sep 17 00:00:00 2001 From: Krishna Satish Deshkulkarni Shankaranarayana Date: Tue, 5 May 2026 15:47:46 +0200 Subject: [PATCH 1/2] mw:com: fix flaky test The root case is as the following, when the ProxyMethod is constructed it calls RegisterMethod that populates a map that holds a ref to the ProxyMethod and during the destruction this Proxy method does not do anything so this map still stays populated, then the ServiceDiscovery client can iterate through this map and try to call MarkUnsubscribe() on already dead proxymethod object, leading to use after free case. --- score/mw/com/impl/bindings/lola/proxy.cpp | 10 ++++++++++ score/mw/com/impl/bindings/lola/proxy.h | 1 + score/mw/com/impl/bindings/lola/proxy_method.cpp | 5 +++++ score/mw/com/impl/bindings/lola/proxy_method.h | 2 ++ 4 files changed, 18 insertions(+) diff --git a/score/mw/com/impl/bindings/lola/proxy.cpp b/score/mw/com/impl/bindings/lola/proxy.cpp index e8cb56091..396fed7b3 100644 --- a/score/mw/com/impl/bindings/lola/proxy.cpp +++ b/score/mw/com/impl/bindings/lola/proxy.cpp @@ -929,4 +929,14 @@ void Proxy::RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(was_inserted, "Method IDs must be unique!"); } +void Proxy::UnregisterMethod(const UniqueMethodIdentifier method_id) noexcept +{ + std::lock_guard lock{proxy_method_registration_mutex_}; + const auto number_of_elements_removed = proxy_methods_.erase(method_id); + if (number_of_elements_removed == 0U) + { + score::mw::log::LogWarn("lola") << "UnregisterMethod with method ID that was never registered. Ignoring."; + } +} + } // 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..15696396d 100644 --- a/score/mw/com/impl/bindings/lola/proxy.h +++ b/score/mw/com/impl/bindings/lola/proxy.h @@ -199,6 +199,7 @@ class Proxy : public ProxyBinding } void RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept; + void UnregisterMethod(const UniqueMethodIdentifier method_id) noexcept; private: static std::atomic current_proxy_instance_counter_; diff --git a/score/mw/com/impl/bindings/lola/proxy_method.cpp b/score/mw/com/impl/bindings/lola/proxy_method.cpp index a9412c1fb..9ab9b4f23 100644 --- a/score/mw/com/impl/bindings/lola/proxy_method.cpp +++ b/score/mw/com/impl/bindings/lola/proxy_method.cpp @@ -48,6 +48,11 @@ ProxyMethod::ProxyMethod(Proxy& proxy, proxy.RegisterMethod(proxy_method_instance_identifier_.unique_method_identifier, *this); } +ProxyMethod::~ProxyMethod() noexcept +{ + proxy_.UnregisterMethod(proxy_method_instance_identifier_.unique_method_identifier); +} + score::Result> ProxyMethod::GetInArgsBuffer(std::size_t queue_position) { if (!is_subscribed_) diff --git a/score/mw/com/impl/bindings/lola/proxy_method.h b/score/mw/com/impl/bindings/lola/proxy_method.h index 32ea30b6a..08ca27bde 100644 --- a/score/mw/com/impl/bindings/lola/proxy_method.h +++ b/score/mw/com/impl/bindings/lola/proxy_method.h @@ -41,6 +41,8 @@ class ProxyMethod : public ProxyMethodBinding ProxyMethodInstanceIdentifier proxy_method_instance_identifier, const TypeErasedCallQueue::TypeErasedElementInfo type_erased_element_info); + ~ProxyMethod() noexcept override; + /// \brief Allocates storage for the in-arguments of a method call at the given queue position. /// /// See ProxyMethodBinding for details From 74a7f3b5ca813711424fe65ddfdb7007a3d60455 Mon Sep 17 00:00:00 2001 From: Krishna Satish Deshkulkarni Shankaranarayana Date: Thu, 7 May 2026 18:34:16 +0200 Subject: [PATCH 2/2] mw::com : fix flaky test with proper destruction order --- docs/sphinx/index.rst | 5 +++ score/mw/com/README.md | 4 ++ score/mw/com/impl/bindings/lola/proxy.cpp | 26 +++++++++++++ score/mw/com/impl/bindings/lola/proxy.h | 2 + .../impl/bindings/lola/proxy_event_common.cpp | 5 --- .../impl/bindings/lola/proxy_event_common.h | 2 +- score/mw/com/impl/proxy_base.cpp | 16 ++++++++ score/mw/com/impl/proxy_base.h | 2 + score/mw/com/impl/proxy_binding.h | 2 + score/mw/com/impl/proxy_event_base.cpp | 2 + score/mw/com/impl/traits.h | 38 +++++++++++++++++++ 11 files changed, 98 insertions(+), 6 deletions(-) diff --git a/docs/sphinx/index.rst b/docs/sphinx/index.rst index 1d6e7ba66..70ed7cd9a 100644 --- a/docs/sphinx/index.rst +++ b/docs/sphinx/index.rst @@ -1,6 +1,11 @@ Communication Middleware (mw::com) ================================== +.. image:: _static/lola_logo.svg + :alt: LoLa logo + :align: center + :width: 560 + Welcome to the documentation for Communication Middleware (mw::com), including the LoLa (Low Latency) implementation and Message Passing library. diff --git a/score/mw/com/README.md b/score/mw/com/README.md index e16ae063f..74ee3eafc 100644 --- a/score/mw/com/README.md +++ b/score/mw/com/README.md @@ -21,6 +21,10 @@ The notion of a `technical binding` also comes from the `ara::com` AUTOSAR stand It is even common, that implementations of the `ara::com` standard come up with several different `technical binding`s (e.g. one for local and one for network communication). +## LoLa IPC overview + +![LoLa IPC overview](./design/events_fields/Lola_IPC_Overview.svg) + ## Documentation for users If you are an adaptive application developer in the IPNEXT project, and you want to use `mw::com` to do local diff --git a/score/mw/com/impl/bindings/lola/proxy.cpp b/score/mw/com/impl/bindings/lola/proxy.cpp index 396fed7b3..bc24fe0f2 100644 --- a/score/mw/com/impl/bindings/lola/proxy.cpp +++ b/score/mw/com/impl/bindings/lola/proxy.cpp @@ -658,6 +658,16 @@ void Proxy::UnregisterEventBinding(const std::string_view service_element_name) // 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_}; + // During destruction the event_bindings_ map would be cleared and then the EventBindingRegistrationGuard would call + // UnregisterEventBinding. In that case, the event_bindings_ map would be empty and we would ignore the + // UnregisterEventBinding call. We added this check to avoid logging warnings about unregistering event bindings + // that were never registered during destruction. + if (event_bindings_.size() == 0U) + { + score::mw::log::LogDebug("lola") + << "UnregisterEventBinding called but there are no registered event bindings. Ignoring."; + return; + } const auto number_of_elements_removed = event_bindings_.erase(service_element_name); if (number_of_elements_removed == 0U) { @@ -929,6 +939,7 @@ void Proxy::RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(was_inserted, "Method IDs must be unique!"); } +<<<<<<< Updated upstream void Proxy::UnregisterMethod(const UniqueMethodIdentifier method_id) noexcept { std::lock_guard lock{proxy_method_registration_mutex_}; @@ -936,6 +947,21 @@ void Proxy::UnregisterMethod(const UniqueMethodIdentifier method_id) noexcept if (number_of_elements_removed == 0U) { score::mw::log::LogWarn("lola") << "UnregisterMethod with method ID that was never registered. Ignoring."; +======= +void Proxy::ShutdownBinding() noexcept +{ + // this causes the destruction of the FindServiceGuard which calls StopFindService so we prevent any future + // notifications from servicediscovery thread. + find_service_guard_.reset(); + { + 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(); +>>>>>>> Stashed changes } } diff --git a/score/mw/com/impl/bindings/lola/proxy.h b/score/mw/com/impl/bindings/lola/proxy.h index 15696396d..d0b5bed23 100644 --- a/score/mw/com/impl/bindings/lola/proxy.h +++ b/score/mw/com/impl/bindings/lola/proxy.h @@ -201,6 +201,8 @@ class Proxy : public ProxyBinding void RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept; void UnregisterMethod(const UniqueMethodIdentifier method_id) noexcept; + void ShutdownBinding() noexcept override; + private: static std::atomic current_proxy_instance_counter_; 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 1071ad29e..ccbbfe1bf 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 55565b1dc..85b2c6f15 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/proxy_base.cpp b/score/mw/com/impl/proxy_base.cpp index 26209a6b3..49e1169c0 100644 --- a/score/mw/com/impl/proxy_base.cpp +++ b/score/mw/com/impl/proxy_base.cpp @@ -174,6 +174,22 @@ Result ProxyBase::SetupMethods() return {}; } +void ProxyBase::Unsubscribe() noexcept +{ + if (proxy_binding_ != nullptr) + { + proxy_binding_->ShutdownBinding(); + } + for (auto& event : events_) + { + event.second.get().Unsubscribe(); + } + for (auto& field : fields_) + { + field.second.get().Unsubscribe(); + } +} + 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..8e710b4ad 100644 --- a/score/mw/com/impl/proxy_base.h +++ b/score/mw/com/impl/proxy_base.h @@ -116,6 +116,8 @@ class ProxyBase */ const HandleType& GetHandle() const noexcept; + void Unsubscribe() noexcept; + protected: using ProxyEvents = std::map>; using ProxyFields = std::map>; diff --git a/score/mw/com/impl/proxy_binding.h b/score/mw/com/impl/proxy_binding.h index c24524d36..7441d5e31 100644 --- a/score/mw/com/impl/proxy_binding.h +++ b/score/mw/com/impl/proxy_binding.h @@ -62,6 +62,8 @@ class ProxyBinding virtual void UnregisterEventBinding(const std::string_view service_element_name) noexcept = 0; virtual Result SetupMethods() = 0; + + virtual void ShutdownBinding() noexcept = 0; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/proxy_event_base.cpp b/score/mw/com/impl/proxy_event_base.cpp index 4e748c0d8..a56375aeb 100644 --- a/score/mw/com/impl/proxy_event_base.cpp +++ b/score/mw/com/impl/proxy_event_base.cpp @@ -101,6 +101,8 @@ ProxyEventBase::~ProxyEventBase() noexcept << "Proxy event instance destroyed while still holding SamplePtr instances, terminating."; std::terminate(); } + + this->Unsubscribe(); } ProxyEventBase::ProxyEventBase(ProxyEventBase&&) noexcept = default; diff --git a/score/mw/com/impl/traits.h b/score/mw/com/impl/traits.h index 1c51a0587..4c351d7db 100644 --- a/score/mw/com/impl/traits.h +++ b/score/mw/com/impl/traits.h @@ -358,6 +358,38 @@ class ProxyWrapperClass : public Interface return proxy_wrapper; } + ~ProxyWrapperClass() + { + if (is_proxy_owner_.IsSet()) + { + this->Unsubscribe(); + } + } + + ProxyWrapperClass(const ProxyWrapperClass&) = delete; + ProxyWrapperClass& operator=(const ProxyWrapperClass&) = delete; + + ProxyWrapperClass(ProxyWrapperClass&& other) noexcept + : Interface{std::move(static_cast&&>(other))}, + is_proxy_owner_{std::move(other.is_proxy_owner_)} + { + } + + ProxyWrapperClass& operator=(ProxyWrapperClass&& other) noexcept + { + if (&other != this) + { + if (is_proxy_owner_.IsSet()) + { + this->Unsubscribe(); + } + + Interface::operator=(std::move(static_cast&&>(other))); + is_proxy_owner_ = std::move(other.is_proxy_owner_); + } + return *this; + } + private: /// \brief Constructs ProxyWrapperClass explicit ProxyWrapperClass(HandleType instance_handle, std::unique_ptr proxy_binding) @@ -379,6 +411,12 @@ class ProxyWrapperClass : public Interface } static std::optional>>> creation_results_; + + /// \brief Flag which is checked before calling Unsubscribe in the destructor of this class + /// + /// This flag is always set for a Proxy except when a Proxy is moved. In this case, this flag will be cleared + /// in the moved-from class so that that object doesn't call Unsubscribe on destruction. + FlagOwner is_proxy_owner_{true}; }; template