Merge pull request #1 from rmed/feature/tree-memory-management

Allow component tree to be alocated in the heap

docs/base-package.md

@@ -10,6 +10,12 @@ Implemented in `kouta::base::Component`.
 
 This is the **base type for any asynchronous element that requires access to the event loop**. A `Component` **does not own the I/O context**, and instead is expected to have a **parent** from which it can reference said context (hence the aforementioned tree-like architecture).
 
+Apart from providing access to the event loop, the **parent** also **keeps track of its children**. This allows for components to be allocated in the **heap** via the `new` keyword if desired so that, when a `Component` is deleted, it will automatically attempt to `delete` its children and remove itself from its parent.
+
+In the case of objects allocated in the **stack**, each object will be deleted in reverse creation order, which should prevent any issues with calling `delete` on non-heap objects due to objects removing themselves from the parent's children list.
+
+**Note**: take extra care when mixing stack and heap allocations, as this may cause invalid deletions.
+
 These components can be chained as many times as needed, as long as there is one parent at the root of the tree which provides the I/O context (see `kouta::base::Component::context()`).
 
 Note that all components deriving from a single parent will be executed **in the same thread**.
@@ -53,6 +59,8 @@ Implemented in `kouta::base::Root`.
 
 As opposed to the base `Component` explained above, the `Root` **owns the I/O context** and is supposed to serve as the **parent** for other components that want to share the same event loop.
 
+While a **parent** can be provided to the root, it is only used when dealing with heap-allocated objects in order to guarantee that the `Root` is deleted with its parent.
+
 Note that starting the event loop of a `Root` (`kouta::base::Root::run()`) **blocks the current thread** and will only be unblocked after the event loop is terminated (e.g. via the `kouta::base::Root::stop()` method).
 
 ```cpp
@@ -111,7 +119,10 @@ The *problem* with the `Root` type is that there is a one-to-one relation betwee
 
 The `Branch` type was introduced for such cases. It **is a `Root`** (owns the I/O context), but manages an `std::thread` internally. As opposed to the `Root`, starting the event loop via `kouta::base::Branch::run()` **will not block**, and instead start the event loop in the child thread.
 
-It is supposed to wrap a `Component`, allowing external components to post events to the wrapped `Component`, as well as to the `Branch` itself.
+It is supposed to wrap a `Component`, allowing external components to post events to the wrapped `Component`, as well as to the `Branch` itself. In addition, the `Branch` assumes that the wrapped `Component` expects a pointer to a parent as its first argument (which will be set to the `Branch` itself).
+
+The **parent** provided to the `Branch` is only used when dealing with heap-allocated objects in order to guarantee that the `Root` is deleted with its parent.
+
 
 ```cpp
 #include <iostream>

include/kouta/base/branch.hpp

@@ -18,7 +18,8 @@ namespace kouta::base
     /// method. As opposed to the original method defined in @ref Root, the one specified here will launch the thread
     /// (and the event loop) and return immediately.
     ///
-    /// @tparam TWrapped            Wrapped @ref Component type.
+    /// @tparam TWrapped            Wrapped @ref Component type. IT is assumed that the first argument of the component
+    ///                             will be a pointer to a parent component (which will be set to this Branch).
     template<class TWrapped>
         requires std::is_base_of_v<Component, TWrapped>
     class Branch : public Root
@@ -31,14 +32,20 @@ namespace kouta::base
 
         /// @brief Branch component constructor.
         ///
-        /// @tparam TArgs           Types of the arguments to provide the wrapped component.
+        /// @details
+        /// This constructor will register the Branch object with the parent **only to manage the memory deallocation**
+        /// in case the object was allocated on the heap. Regardless of having a parent, the Branch owns its event loop.
+        ///
+        /// @tparam TArgs               Types of the arguments to provide the wrapped component.
         ///
-        /// @param[in] args         Arguments to provide the wrapped component.
+        /// @param[in] parent           Parent component. The lifetime of the parent must surpass that of the
+        ///                             child.
+        /// @param[in] args             Arguments to provide the wrapped component.
         template<class... TArgs>
-        Branch(TArgs... args)
-            : Root{}
+        Branch(Component* parent, TArgs... args)
+            : Root{parent}
             , m_worker{}
-            , m_component{args...}
+            , m_component{this, args...}  // Assuming first argument is the parent component
         {
         }
 
@@ -109,7 +116,7 @@ namespace kouta::base
             m_component.post(method, args...);
         }
 
-        /// @brief Inherit @ref Root::post() to allow posting events to the worker itself.
+        /// @brief Inherit @ref Root::post() to allow posting events to the branch itself.
         ///
         /// @note This may be used to post a call to the @ref stop() method.
         using Root::post;

include/kouta/base/component.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <vector>
+
 #include <boost/asio.hpp>
 
 namespace kouta::base
@@ -8,6 +10,8 @@ namespace kouta::base
     ///
     /// @details
     /// A component provies access to the underlying event loop which, by default, belongs to the parent component.
+    /// Moreover, specifying a parent will add the component to its children list and make sure that the component is
+    /// deleted when the parent is destroyed (if the component was allocated on the heap).
     class Component
     {
     public:
@@ -16,11 +20,14 @@ namespace kouta::base
 
         /// @brief Constructor.
         ///
-        /// @param[in] parent           Parent component. Note that the parent provides access to the event loop,
-        ///                             hence its lifetime must, at least, surpass that of the child.
-        explicit Component(Component* parent = nullptr)
+        /// @param[in] parent           Parent component. The lifetime of the parent must surpass that of the child.
+        explicit Component(Component* parent)
             : m_parent{parent}
         {
+            if (m_parent)
+            {
+                m_parent->add_child(this);
+            }
         }
 
         // Not copyable
@@ -31,7 +38,32 @@ namespace kouta::base
         Component(Component&&) = delete;
         Component& operator=(Component&&) = delete;
 
-        virtual ~Component() = default;
+        /// @brief Component destructor.
+        ///
+        /// @details
+        /// The component will go through its list of children and delete them one at a time. This is only useful if the
+        /// components were allocated in the heap, as stack-allocated ones will probably have been deleted automatically
+        /// prior to calling this destructor.
+        ///
+        /// In addition, once a component has deleted its children, it will remove itself from its parent.
+        ///
+        /// @note Child deletion happens in reverse order.
+        virtual ~Component()
+        {
+            // Delete children
+            while (!m_children.empty())
+            {
+                // When deleted, children will remove themselves from this list
+                auto* component{m_children.back()};
+                delete component;
+            }
+
+            // Delete from parent
+            if (m_parent)
+            {
+                m_parent->remove_child(this);
+            }
+        }
 
         /// @brief Obtain a reference to the underlying I/O context.
         ///
@@ -41,6 +73,30 @@ namespace kouta::base
             return m_parent->context();
         }
 
+        /// @brief Add a child component to the list.
+        ///
+        /// @details
+        /// This is used to keep track of objects to delete when the component has been allocated in the heap.
+        ///
+        /// @note Normally, this will only be called from the Constructor of the component.
+        ///
+        /// @param[in] component            Pointer to the component to add.
+        void add_child(Component* component)
+        {
+            m_children.emplace_back(component);
+        }
+
+        /// @brief Remove a child component from the list.
+        ///
+        /// @details
+        /// When a child is removed, the parent component will not attempt to delete it itself. Note that when a
+        /// component allocated in the stack is destroyed, it will remove itself from the list and prevent
+        /// double-free issues.
+        void remove_child(Component* component)
+        {
+            std::erase(m_children, component);
+        }
+
         /// @brief Post a method call to the event loop for deferred execution.
         ///
         /// @details
@@ -107,5 +163,6 @@ namespace kouta::base
 
     private:
         Component* m_parent;
+        std::vector<Component*> m_children;
     };
 }  // namespace kouta::base

include/kouta/base/root.hpp

@@ -12,8 +12,24 @@ namespace kouta::base
     class Root : public Component
     {
     public:
+        /// @brief Default constructor.
+        ///
+        /// @details
+        /// This constructor assumes that the Root object will not have a parent (e.g. it is the main object of the
+        /// tree), meaning that it will not attempt to register itself with the parent, nor remove itself from its list
+        /// when being destroyed.
         Root()
-            : Component{nullptr}
+            : Root{nullptr}
+        {
+        }
+
+        /// @brief Construct from a parent.
+        ///
+        /// @details
+        /// This constructor will register the Root object with the parent **only to manage the memory deallocation** in
+        /// case the object was allocated on the heap. Regardless of having a parent, the Root owns its event loop.
+        explicit Root(Component* parent)
+            : Component{parent}
             , m_context{}
         {
         }

tests/base/dummy-component.cpp

@@ -2,6 +2,10 @@
 
 namespace kouta::tests::base
 {
+    DummyComponent::DummyComponent(Component* parent)
+        : Component{parent}
+    {}
+
     DummyComponent::DummyComponent(
         Component* parent,
         const Callback<std::uint16_t>& callback_a,
@@ -28,6 +32,12 @@ namespace kouta::tests::base
     {
     }
 
+    DummyComponent::DummyComponent(Component* parent, const Callback<Component*> callback_on_delete)
+        : Component{parent}
+        , m_callback_on_delete{callback_on_delete}
+    {
+    }
+
     void DummyComponent::call_a(std::uint16_t value)
     {
         m_callback_a(value);

tests/base/dummy-component.hpp

@@ -17,6 +17,8 @@ namespace kouta::tests::base
     public:
         DummyComponent() = delete;
 
+        explicit DummyComponent(Component* parent);
+
         DummyComponent(
             Component* parent,
             const Callback<std::uint16_t>& callback_a,
@@ -30,6 +32,8 @@ namespace kouta::tests::base
             const Callback<const std::vector<std::uint8_t>&>& callback_c,
             const Callback<std::thread::id>& callback_d);
 
+        DummyComponent(Component* parent, const Callback<Component*> callback_on_delete);
+
         // Not copyable
         DummyComponent(const DummyComponent&) = delete;
         DummyComponent& operator=(const DummyComponent&) = delete;
@@ -38,7 +42,14 @@ namespace kouta::tests::base
         DummyComponent(DummyComponent&&) = delete;
         DummyComponent& operator=(DummyComponent&&) = delete;
 
-        ~DummyComponent() override = default;
+        ~DummyComponent() override
+        {
+            // Notify that the object was deleted
+            if (m_callback_on_delete)
+            {
+                m_callback_on_delete.value()(this);
+            }
+        }
 
         /// @brief Callback invokers.
         /// @{
@@ -53,5 +64,6 @@ namespace kouta::tests::base
         Callback<std::int32_t, const std::string&> m_callback_b;
         Callback<const std::vector<std::uint8_t>&> m_callback_c;
         Callback<std::thread::id> m_callback_d;
+        std::optional<Callback<Component*>> m_callback_on_delete;
     };
 }  // namespace kouta::tests::base

tests/base/test-base.cpp

@@ -17,6 +17,7 @@ namespace kouta::tests::base
         MOCK_METHOD(void, handler_b, (std::int32_t value_a, const std::string& value_b), ());
         MOCK_METHOD(void, handler_c, (const std::vector<std::uint8_t>& value), ());
         MOCK_METHOD(void, handler_d, (std::thread::id), ());
+        MOCK_METHOD(void, handler_delete, (Component*), ());
     };
 
     /// @brief Test the behaviour of the empty callback.
@@ -329,7 +330,7 @@ namespace kouta::tests::base
 
         RootMock root{};
         Branch<DummyComponent> worker{
-            &worker,
+            &root,
             callback::DirectCallback{&root, &RootMock::handler_a},
             callback::DeferredCallback{&root, &RootMock::handler_b},
             callback::DeferredCallback{&root, &RootMock::handler_c},
@@ -369,4 +370,81 @@ namespace kouta::tests::base
         root.run();
         alarm(0);
     }
+
+    /// @brief Test the behaviour of a component tree when allocated in the heap.
+    ///
+    /// @details
+    /// The test succeeds if all components are deallocated.
+    TEST(BaseTest, HeapAllocation)
+    {
+        RootMock root{};
+
+        auto* dummy_base = new DummyComponent{&root, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+
+        // Layer 1
+        auto* comp_a = new DummyComponent{dummy_base, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+        auto* comp_b = new DummyComponent{dummy_base, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+        auto* comp_c = new DummyComponent{dummy_base, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+
+        // Layer 2
+        auto* comp_a1 = new DummyComponent{comp_a, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+        auto* comp_a2 = new DummyComponent{comp_a, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+        auto* comp_c1 = new DummyComponent{comp_c, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+
+        // Layer 3
+        auto* comp_a1_1 = new DummyComponent{comp_a1, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+        auto* comp_a1_2 = new DummyComponent{comp_a1, callback::DeferredCallback{&root, &RootMock::handler_delete}};
+
+        // Deletion will be in reverse creation order, but we don't really care
+        EXPECT_CALL(root, handler_delete(dummy_base))
+            .WillOnce(
+                [&root]()
+                {
+                    root.post(&RootMock::stop);
+                });
+
+        EXPECT_CALL(root, handler_delete(comp_a)).Times(1);
+        EXPECT_CALL(root, handler_delete(comp_a2)).Times(1);
+        EXPECT_CALL(root, handler_delete(comp_a1)).Times(1);
+        EXPECT_CALL(root, handler_delete(comp_a1_2)).Times(1);
+        EXPECT_CALL(root, handler_delete(comp_a1_1)).Times(1);
+
+        EXPECT_CALL(root, handler_delete(comp_b)).Times(1);
+
+        EXPECT_CALL(root, handler_delete(comp_c)).Times(1);
+        EXPECT_CALL(root, handler_delete(comp_c1)).Times(1);
+
+        delete dummy_base;
+
+        root.run();
+    }
+
+    /// @brief Test the behaviour of a component tree when allocated in the stack.
+    ///
+    /// @details
+    /// The test succeeds if no exception is thrown due to free().
+    TEST(BaseTest, StackAllocation)
+    {
+        RootMock root{};
+
+        DummyComponent dummy_base{&root};
+
+        // Layer 1
+        DummyComponent comp_a{&dummy_base};
+        DummyComponent comp_b{&dummy_base};
+        DummyComponent comp_c{&dummy_base};
+
+        // Layer 2
+        DummyComponent comp_a1{&comp_a};
+        DummyComponent comp_a2{&comp_a};
+        DummyComponent comp_c1{&comp_c};
+
+        // Layer 3
+        DummyComponent comp_a1_1{&comp_a1};
+        DummyComponent comp_a1_2{&comp_a1};
+
+        auto* comp_c1_1{&comp_c1};
+
+        // Everything is deleted in reverse order, so there shouldn't be any exceptions at this point
+    }
 }  // namespace kouta::tests::base