fix: Consider reentrant

include/nodec/observers.hpp

@@ -5,9 +5,15 @@
 #include <vector>
 #include <unordered_map>
 
+/**
+ * @note Implementation refs:
+ * https://chromium.googlesource.com/chromium/src.git/+/master/base/observer_list.h
+ * 
+ */
+
 namespace nodec {
 
-template<class IObserver, typename Mutex = std::mutex>
+template<class IObserver, typename Mutex = std::recursive_mutex>
 class Observers {
 
 public:
@@ -27,9 +33,7 @@ class Observers {
             return;
         }
         auto index = iter->second;
-        observers_[index] = observers_.back();
-        observer_indices_[observers_[index]] = index;
-        observers_.pop_back();
+        observers_[index] = nullptr;
         observer_indices_.erase(iter);
     }
 
@@ -42,9 +46,27 @@ class Observers {
     template<typename Func>
     void each(Func func) {
         std::lock_guard<Mutex> lock(mutex_);
-        for (auto observer : observers_) {
+
+        std::size_t n = observers_.size();
+        for (std::size_t i = 0; i < n; ++i) {
+            auto observer = observers_[i];
+            if (observer == nullptr) continue;
             func(observer);
         }
+
+        // Remove all empty slots.
+        std::size_t sz = 0;
+        for (std::size_t i = 0; i < n; ++i) {
+            if (observers_[i] == nullptr) continue;
+            observers_[sz] = observers_[i];
+            observer_indices_[observers_[i]] = sz;
+            ++sz;
+        }
+    }
+
+    std::size_t size() const {
+        std::lock_guard<Mutex> lock(mutex_);
+        return observer_indices_.size();
     }
 
 private:

tests/CMakeLists.txt

@@ -19,6 +19,7 @@ add_basic_test("logging_bench" logging/bench.cpp)
 add_basic_test("gfx" gfx/gfx.cpp)
 add_basic_test("math" math/math.cpp)
 add_basic_test("matrix4x4" matrix4x4/matrix4x4.cpp)
+add_basic_test("observers" observers/observers.cpp)
 add_basic_test("optional__basic" optional/basic.cpp)
 add_basic_test("quaternion" quaternion/quaternion.cpp)
 add_basic_test("random" random/random.cpp)

tests/observers/observers.cpp

@@ -0,0 +1,85 @@
+#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
+#include <doctest.h>
+
+#include <functional>
+
+#include <nodec/observers.hpp>
+
+TEST_CASE("Check to notify on not reentrant") {
+    class IObserver {
+    public:
+        virtual void on_notify() = 0;
+    };
+
+    class TestObserver : public IObserver {
+    public:
+        void on_notify() override {
+            notified = true;
+        }
+
+        bool notified = false;
+    };
+
+    SUBCASE("add and remove") {
+        nodec::Observers<IObserver> observers;
+
+        TestObserver observer1;
+
+        observers.add_observer(&observer1);
+        CHECK(observer1.notified == false);
+
+        observers.each([](IObserver *observer) {
+            observer->on_notify();
+        });
+        CHECK(observer1.notified == true);
+
+        observers.remove_observer(&observer1);
+
+        observer1.notified = false;
+        observers.each([](IObserver *observer) {
+            observer->on_notify();
+        });
+        CHECK(observer1.notified == false);
+    }
+}
+
+TEST_CASE("Check to notify on reentrant") {
+    class IObserver {
+    public:
+        virtual void on_notify() = 0;
+    };
+
+    class TestObserver : public IObserver {
+    public:
+        void on_notify() override {
+            callback();
+        }
+
+        std::function<void()> callback;
+    };
+
+    SUBCASE("add while notify") {
+        nodec::Observers<IObserver> observers;
+
+        TestObserver observer1;
+        TestObserver observer2;
+        bool notified_1 = false;
+        bool notified_2 = false;
+        observer1.callback = [&]() {
+            observers.add_observer(&observer2);
+            notified_1 = true;
+        };
+        observer2.callback = [&]() {
+            notified_2 = true;
+        };
+
+        observers.add_observer(&observer1);
+
+        observers.each([](IObserver *observer) {
+            observer->on_notify();
+        });
+
+        CHECK(notified_1 == true);
+        CHECK(notified_2 == false);
+    }
+}