Fix flaky test

score/mw/com/impl/BUILD

@@ -56,6 +56,7 @@ cc_library(
         "//score/mw/com:__subpackages__",
     ],
     deps = [
+        ":flag_owner",
         ":generic_proxy_event",
         ":handle_type",
         ":proxy_base",
@@ -428,6 +429,7 @@ cc_library(
     ],
     deps = [
         ":error",
+        ":flag_owner",
         ":proxy_binding",
         ":proxy_event_binding",
         ":sample_reference_tracker",

score/mw/com/impl/bindings/lola/BUILD

@@ -358,6 +358,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",

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; release state-machine state explicitly.
+    void TearDown() override
+    {
+        if (generic_proxy_event_ != nullptr)
+        {
+            generic_proxy_event_->Unsubscribe();
+        }
+        ProxyMockedMemoryFixture::TearDown();
+    }
+
     std::unique_ptr<GenericProxyEvent> generic_proxy_event_{nullptr};
 };
 TEST_F(LolaGenericProxyEventFixture, CanConstructAGenericProxyEvent)

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<HandleType> 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<FindServiceHandle>(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<FindServiceHandle> service_availability_change_handle_;
-};
-
 std::atomic<ProxyInstanceIdentifier::ProxyInstanceCounter> Proxy::current_proxy_instance_counter_{0U};
 
 ElementFqId Proxy::EventNameToElementFqIdConverter::Convert(const std::string_view event_name) const noexcept
@@ -430,20 +386,20 @@ Proxy::Proxy(std::shared_ptr<memory::shared::ManagedMemoryResource> control,
       offered_state_machine_{},
       are_proxy_methods_setup_{false},
       filesystem_{filesystem},
-      find_service_guard_{std::make_unique<FindServiceGuard>(
-          [this](ServiceHandleContainer<HandleType> 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_{}
 {
+    StartProxyAutoReconnect();
 }
 
-Proxy::~Proxy() = default;
+Proxy::~Proxy()
+{
+    if (!prepare_unsubscribe_called_ || !finalize_unsubscribe_called_)
+    {
+        score::mw::log::LogFatal("lola")
+            << "Proxy destroyed without prior PrepareUnsubscribe + FinalizeUnsubscribe. Terminating.";
+        std::terminate();
+    }
+}
 
 void Proxy::ServiceAvailabilityChangeHandler(const bool is_service_available)
 {
@@ -929,4 +885,61 @@ void Proxy::RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod&
     SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(was_inserted, "Method IDs must be unique!");
 }
 
+void Proxy::PrepareUnsubscribe()
+{
+    StopProxyAutoReconnect();
+    prepare_unsubscribe_called_ = true;
+}
+
+void Proxy::FinalizeUnsubscribe()
+{
+    {
+        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_unsubscribe_called_ = true;
+}
+
+void Proxy::StartProxyAutoReconnect()
+{
+    auto& service_discovery = impl::Runtime::getInstance().GetServiceDiscovery();
+    const auto find_service_handle_result = service_discovery.StartFindService(
+        [this](ServiceHandleContainer<HandleType> 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()
+{
+    if (!find_service_handle_.has_value())
+    {
+        return;
+    }
+    // StopFindService waits for any in-flight handler to finish; must run before we take
+    // proxy_event_registration_mutex_ since the handler also takes it.
+    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

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,15 @@ class Proxy : public ProxyBinding
 
     void RegisterMethod(const UniqueMethodIdentifier method_id, ProxyMethod& proxy_method) noexcept;
 
+    void PrepareUnsubscribe() override;
+    void FinalizeUnsubscribe() override;
+
   private:
     static std::atomic<ProxyInstanceIdentifier::ProxyInstanceCounter> current_proxy_instance_counter_;
 
+    void StartProxyAutoReconnect();
+    void StopProxyAutoReconnect();
+
     void ServiceAvailabilityChangeHandler(const bool is_service_available);
     void InitializeSharedMemoryForMethods(
         memory::shared::ManagedMemoryResource& memory_resource,
@@ -261,9 +267,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<FindServiceGuard> find_service_guard_;
+    /// Handle returned by ServiceDiscovery::StartFindService. Held so we can call StopFindService later from
+    /// StopProxyAutoReconnect().
+    std::optional<FindServiceHandle> find_service_handle_;
+
+    /// Set by PrepareUnsubscribe / FinalizeUnsubscribe respectively. ~Proxy terminates unless both were called.
+    bool prepare_unsubscribe_called_{false};
+    bool finalize_unsubscribe_called_{false};
 };
 
 template <typename EventSampleType>

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<void> ProxyEventCommon::Subscribe(const std::size_t max_sample_count)
 {
     std::stringstream sstream{};

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;

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; release state-machine state explicitly.
+    void TearDown() override
+    {
+        if (proxy_event_ != nullptr)
+        {
+            proxy_event_->Unsubscribe();
+        }
+        ProxyMockedMemoryFixture::TearDown();
+    }
+
   protected:
     ::testing::MockFunction<void()> event_handler_{};
     std::unique_ptr<T> proxy_event_{nullptr};
@@ -280,7 +290,8 @@ TYPED_TEST(LolaProxyEventCommonFixture, DestroyingTheProxyEventWillUnregisterEve
     // Then the receive handler should not be unregistered
     EXPECT_FALSE(handler_unregistered);
 
-    // and when the ProxyEvent is destroyed
+    // and when the ProxyEvent is unsubscribed and destroyed
+    this->proxy_event_->Unsubscribe();
     this->proxy_event_.reset();
 
     // Then the receive handler should be unregistered

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; release state-machine state explicitly.
+    void TearDown() override
+    {
+        if (test_proxy_event_ != nullptr)
+        {
+            test_proxy_event_->Unsubscribe();
+        }
+        ProxyMockedMemoryFixture::TearDown();
+    }
+
     std::unique_ptr<ProxyEventType> test_proxy_event_{nullptr};
     std::unique_ptr<SampleReferenceTracker> sample_reference_tracker_{};
 };

score/mw/com/impl/bindings/lola/skeleton.cpp

@@ -358,6 +358,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 +371,8 @@ auto Skeleton::PrepareOffer(SkeletonEventBindings& events,
 auto Skeleton::PrepareStopOffer(std::optional<UnregisterShmObjectTraceCallback> 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 +446,16 @@ auto Skeleton::PrepareStopOffer(std::optional<UnregisterShmObjectTraceCallback>
     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_;

score/mw/com/impl/bindings/lola/skeleton.h

@@ -99,7 +99,7 @@ class Skeleton final : public SkeletonBinding
              std::unique_ptr<score::memory::shared::FlockMutexAndLock<score::memory::shared::ExclusiveFlockMutex>>
                  service_instance_existence_flock_mutex_and_lock);
 
-    ~Skeleton() noexcept override = default;
+    ~Skeleton() noexcept override;
 
     Skeleton(const Skeleton&) = delete;
     Skeleton& operator=(const Skeleton&) = delete;
@@ -240,6 +240,12 @@ class Skeleton final : public SkeletonBinding
     /// This scope will be manually expired in PrepareStopOffer which will prevent any Proxy from subscribing to
     /// this method.
     safecpp::Scope<> on_service_method_subscribed_handler_scope_;
+
+    /// Set by PrepareOffer / PrepareStopOffer respectively. ~Skeleton terminates if PrepareOffer was called but
+    /// PrepareStopOffer was not — i.e. the skeleton registered method handlers and is being destroyed without
+    /// unregistering them.
+    bool prepare_offer_called_{false};
+    bool prepare_stop_offer_called_{false};
 };
 
 template <typename SampleType>