feat: made EventDispatcher and EventDispatcherWithWeakptr exception s…

…afe when executing an action (#525) * feat: made EventDispatcher and EventDispatcherWithWeakptr exception safe when executing an action * feat: added tests for handling exceptions
philips-software · Jan 15, 2024 · 46a118c · 46a118c
1 parent d05d1f3
commit 46a118c
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 7 deletions.
diff --git a/infra/event/EventDispatcher.cpp b/infra/event/EventDispatcher.cpp
@@ -63,10 +63,25 @@ namespace infra
     {
         if (scheduledActions[scheduledActionsPopIndex].second)
         {
-            scheduledActions[scheduledActionsPopIndex].first();
-            scheduledActions[scheduledActionsPopIndex].first = nullptr;
-            scheduledActions[scheduledActionsPopIndex].second = false;
-            scheduledActionsPopIndex = (scheduledActionsPopIndex + 1) % scheduledActions.size();
+            struct ExceptionSafePop
+            {
+                ExceptionSafePop(const ExceptionSafePop&) = delete;
+                ExceptionSafePop& operator=(const ExceptionSafePop&) = delete;
+
+                ~ExceptionSafePop()
+                {
+                    worker.scheduledActions[worker.scheduledActionsPopIndex].first = nullptr;
+                    worker.scheduledActions[worker.scheduledActionsPopIndex].second = false;
+                    worker.scheduledActionsPopIndex = (worker.scheduledActionsPopIndex + 1) % worker.scheduledActions.size();
+                }
+
+                EventDispatcherWorkerImpl& worker;
+            };
+
+            ExceptionSafePop popAction{ *this };
+
+            scheduledActions[scheduledActionsPopIndex]
+                .first();
         }
     }
 

diff --git a/infra/event/EventDispatcherWithWeakPtr.cpp b/infra/event/EventDispatcherWithWeakPtr.cpp
@@ -79,10 +79,23 @@ namespace infra
     {
         if (scheduledActions[scheduledActionsPopIndex].second)
         {
+            struct ExceptionSafePop
+            {
+                ExceptionSafePop(const ExceptionSafePop&) = delete;
+                ExceptionSafePop& operator=(const ExceptionSafePop&) = delete;
+
+                ~ExceptionSafePop()
+                {
+                    worker.scheduledActions[worker.scheduledActionsPopIndex].first.Destruct();
+                    worker.scheduledActions[worker.scheduledActionsPopIndex].second = false;
+                    worker.scheduledActionsPopIndex = (worker.scheduledActionsPopIndex + 1) % worker.scheduledActions.size();
+                }
+
+                EventDispatcherWithWeakPtrWorker& worker;
+            };
+
+            ExceptionSafePop popAction{ *this };
             scheduledActions[scheduledActionsPopIndex].first->Execute();
-            scheduledActions[scheduledActionsPopIndex].first.Destruct();
-            scheduledActions[scheduledActionsPopIndex].second = false;
-            scheduledActionsPopIndex = (scheduledActionsPopIndex + 1) % scheduledActions.size();
         }
     }
 

diff --git a/infra/event/test/TestEventDispatcher.cpp b/infra/event/test/TestEventDispatcher.cpp
@@ -3,6 +3,7 @@
 #include "infra/util/test_helper/MockCallback.hpp"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <stdexcept>
 
 class EventDispatcherTest
     : public testing::Test
@@ -162,3 +163,28 @@ TEST_F(EventDispatcherTest, execute_a_lot)
         ExecuteAllActions();
     }
 }
+
+TEST_F(EventDispatcherTest, scheduled_action_can_throw)
+{
+
+    infra::EventDispatcher::Instance().Schedule([]
+        {
+            throw std::exception{};
+        });
+    infra::EventDispatcher::Instance().Schedule([]
+        {
+            throw std::runtime_error{ "message" };
+        });
+
+    infra::MockCallback<void()> callback;
+    EXPECT_CALL(callback, callback());
+
+    infra::EventDispatcher::Instance().Schedule([&callback, this]()
+        {
+            callback.callback();
+        });
+
+    EXPECT_THROW(ExecuteAllActions(), std::exception);
+    EXPECT_THROW(ExecuteAllActions(), std::runtime_error);
+    EXPECT_NO_THROW(ExecuteAllActions());
+}
diff --git a/infra/event/test/TestEventDispatcherWithWeakPtr.cpp b/infra/event/test/TestEventDispatcherWithWeakPtr.cpp
@@ -4,6 +4,7 @@
 #include "infra/util/test_helper/MockCallback.hpp"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <stdexcept>
 
 class EventDispatcherWithWeakPtrTest
     : public testing::Test
@@ -159,3 +160,28 @@ TEST_F(EventDispatcherWithWeakPtrDetailedTest, ExecuteUntil_executes_until_secon
             return done;
         });
 }
+
+TEST_F(EventDispatcherWithWeakPtrDetailedTest, scheduled_action_can_throw)
+{
+
+    infra::EventDispatcherWithWeakPtr::Instance().Schedule([]
+        {
+            throw std::exception{};
+        });
+    infra::EventDispatcherWithWeakPtr::Instance().Schedule([]
+        {
+            throw std::runtime_error{ "message" };
+        });
+
+    infra::MockCallback<void()> callback;
+    EXPECT_CALL(callback, callback());
+
+    infra::EventDispatcherWithWeakPtr::Instance().Schedule([&callback, this]()
+        {
+            callback.callback();
+        });
+
+    EXPECT_THROW(ExecuteAllActions(), std::exception);
+    EXPECT_THROW(ExecuteAllActions(), std::runtime_error);
+    EXPECT_NO_THROW(ExecuteAllActions());
+}