From c090b574bdf3855472801db7c51e728ef7f3263a Mon Sep 17 00:00:00 2001
From: Matthias Killat <matthias.killat@apex.ai>
Date: Tue, 24 Jan 2023 19:32:58 +0100
Subject: [PATCH] iox-#1640 Add concurrent guard test

Signed-off-by: Matthias Killat <matthias.killat@apex.ai>
---
 .../design_pattern/static_lifetime_guard.inl  |  2 -
 .../test_static_lifetime_guard.cpp            | 60 ++++++++++++++++++-
 .../include/iceoryx_hoofs/testing/barrier.hpp |  2 +-
 3 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/static_lifetime_guard.inl b/iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/static_lifetime_guard.inl
index 877b0a5db18..65a76538c0b 100644
--- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/static_lifetime_guard.inl
+++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/design_pattern/static_lifetime_guard.inl
@@ -19,7 +19,6 @@
 
 #include "iceoryx_hoofs/design_pattern/static_lifetime_guard.hpp"
 
-#include <atomic>
 #include <thread>
 
 namespace iox
@@ -126,7 +125,6 @@ void StaticLifetimeGuard<T>::destroy()
     }
 }
 
-
 } // namespace design_pattern
 } // namespace iox
 
diff --git a/iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp b/iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp
index f73c4911101..c015cc91faa 100644
--- a/iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp
+++ b/iceoryx_hoofs/test/moduletests/test_static_lifetime_guard.cpp
@@ -14,10 +14,14 @@
 //
 // SPDX-License-Identifier: Apache-2.0
 
+#include "iceoryx_hoofs/cxx/vector.hpp"
 #include "iceoryx_hoofs/design_pattern/static_lifetime_guard.hpp"
 
+#include "iceoryx_hoofs/testing/barrier.hpp"
 #include "test.hpp"
-#include <iostream>
+
+#include <chrono>
+#include <thread>
 
 namespace
 {
@@ -51,6 +55,16 @@ struct Fou
     uint32_t id;
 };
 
+// delay is only needed for multithreaded test
+template <uint64_t N>
+struct DelayedFou : public Fou<N>
+{
+    explicit DelayedFou(std::chrono::nanoseconds delay)
+    {
+        std::this_thread::sleep_for(delay);
+    }
+};
+
 constexpr uint32_t FIRST_INSTANCE_ID{1};
 constexpr uint32_t SECOND_INSTANCE_ID{2};
 
@@ -103,7 +117,7 @@ struct TestTypes : public TestGuard<N>
 // shorten the initialization via macro, needed due to __LINE__
 #define INIT_TEST using T = TestTypes<__LINE__>
 
-// each test uses its own type for maximum separation, so we d not need to reset anything
+// each test uses its own type for maximum separation, so we do not need to reset anything
 class StaticLifetimeGuard_test : public Test
 {
   public:
@@ -266,7 +280,7 @@ TEST_F(StaticLifetimeGuard_test, destructionAtZeroCountWorks)
         // we ignore the guard of testInstance() by setting it to 1,
         // hence when guard is destroyed the instance will be destroyed as well
         auto oldCount = T::setCount(1);
-        EXPECT_EQ(oldCount, 2);
+        ASSERT_EQ(oldCount, 2);
 
         EXPECT_EQ(T::Foo::ctorCalled, 0);
         EXPECT_EQ(T::Foo::dtorCalled, 0);
@@ -315,4 +329,44 @@ TEST_F(StaticLifetimeGuard_test, constructionAfterDestructionWorks)
     EXPECT_EQ(T::Foo::dtorCalled, 1);
 }
 
+// note that this test cannot guarantee concurrent correctness due to thread scheduling
+// being unpredictable
+TEST_F(StaticLifetimeGuard_test, instanceCtorIsConcurrentlyCalledExactlyOnce)
+{
+    using Instance = DelayedFou<1>;
+    using Sut = iox::design_pattern::StaticLifetimeGuard<Instance>;
+    constexpr uint32_t NUM_THREADS = 8;
+
+    EXPECT_EQ(Instance::ctorCalled, 0);
+
+    // wait at the barrier to ensure threads were started and increase the
+    // concurrent execution probability (but cannot guarantee concurrent execution)
+    Barrier barrier(2);
+    auto createInstance = [&barrier]() {
+        barrier.notify();
+        barrier.wait();
+        // all threads have notfied (but may pass wait in any order ...)
+
+        // cannot wait too long otherwise we slow down the tests too much,
+        // cannot be optimized away, as it has has side effects (counting)
+        Sut::instance(std::chrono::milliseconds(1));
+    };
+
+    iox::cxx::vector<std::thread, NUM_THREADS> threads;
+
+    for (uint32_t i = 0; i < NUM_THREADS; ++i)
+    {
+        threads.emplace_back(createInstance);
+    }
+
+    // each join can only return once each thread arrived at the barrier and
+    // called Sut::instance()
+    for (auto& t : threads)
+    {
+        t.join();
+    }
+
+    EXPECT_EQ(Instance::ctorCalled, 1);
+}
+
 } // namespace
diff --git a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/barrier.hpp b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/barrier.hpp
index 14d22343c0b..99eb6ac5e11 100644
--- a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/barrier.hpp
+++ b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/barrier.hpp
@@ -25,7 +25,7 @@
 class Barrier
 {
   public:
-    Barrier(uint32_t requiredCount = 0)
+    explicit Barrier(uint32_t requiredCount = 0)
         : m_requiredCount(requiredCount)
     {
     }