pool: initial attempts at clear

pool.hpp

@@ -29,6 +29,7 @@ namespace cruft {
 
         union node {
             alignas(node*) std::atomic<node*> next;
+            alignas(node*) node* raw;
             alignas(T)     char data[sizeof(T)];
         };
 
@@ -77,16 +78,19 @@ namespace cruft {
             // allocate the memory and note the base address for deletion in destructor
             m_next = m_head = new node[m_capacity];
 
-            clear ();
+            relink ();
         }
 
         ~pool ()
         {
+            clear ();
+
             // don't check if everything's been returned as pools are often used
             // for PODs which don't need to be destructed via calling release.
             delete [] m_head;
         }
 
+
         // Data management
         [[nodiscard]] T*
         allocate [[gnu::malloc]] [[gnu::returns_nonnull]] (void)
@@ -158,18 +162,86 @@ namespace cruft {
         bool full     (void) const { return size () == capacity (); }
 
 
+        /// Destroys all objects that have been allocated, frees the
+        /// associated memory, and then rebuilds the free node list ready for
+        /// allocations again.
+        ///
+        /// NOTE: All bets are off if any object throws an exception out of
+        /// their destructor. We provide no exception guarantees.
+        ///
+        /// This call is NOT thread safe. No users should be accessing this
+        /// object for the duration of this call.
         void clear (void)
         {
+            // Create a fake root so that we can always point to the parent
+            // of every node in the system. Hopefully this isn't too large for
+            // the stack.
+            node container;
+            container.next.store (m_next.load ());
+
+            // Sort the node list. We walk the list, and at each step reparent
+            // the child at the lowest memory address to the cursor.
+            for (node* start = container.raw; start; start = start->raw) {
+                node* parent = start;
+
+                // Find the node whose child is the lowest pointer
+                int count = 0;
+                for (auto cursor = parent; cursor->raw; cursor = cursor->raw) {
+                    ++count;
+                    CHECK_NEQ (cursor->raw, start);
+                    if (cursor->raw < parent)
+                        parent = cursor;
+                }
+
+                // Parent the lowest child to the start of the sorted list
+                auto tmp = start->raw;
+                start->raw = parent->raw;
+
+                // Remove the lowest child from their old parent
+                auto parent_next = parent->raw;
+                parent->raw = parent_next ? parent_next->raw : nullptr;
+
+                // Parent the old successor of the start to the lowest child
+                start->raw = tmp;
+            }
+
+            // Now that we've ordered the nodes we can walk the list from
+            // start to finish and find nodes that aren't in the free list.
+            // Call the destructors on the data contained in these.
+            auto node_cursor = m_next.load (std::memory_order_relaxed);
+            auto data_cursor = m_head;
+
+            while (node_cursor) {
+                while (data_cursor < node_cursor) {
+                    cruft::cast::alignment<T*> (data_cursor->data)->~T ();
+                    ++data_cursor;
+                }
+
+                node_cursor = node_cursor->raw;
+                ++data_cursor;
+            }
+
+            while (data_cursor < m_head + m_capacity) {
+                cruft::cast::alignment<T*> (data_cursor->data)->~T ();
+                ++data_cursor;
+            }
+
+            relink ();
+        }
+
+    private:
+        void relink (void)
+        {
+            // Reset the allocation cursor to point to the start of the region
             m_next = m_head;
 
-            // build out a complete singly linked list from all the nodes.
+            // build out the linked list from all the nodes.
             for (size_t i = 0; i < m_capacity - 1; ++i)
                 m_next[i].next = m_next + i + 1;
-
             m_next[m_capacity - 1].next = nullptr;
         }
 
-
+    public:
         // Indexing
         size_t index (T const *ptr) const
         {

test/pool.cpp

@@ -1,6 +1,7 @@
 #include "../pool.hpp"
 
 #include "../tap.hpp"
+#include "../random.hpp"
 
 #include <set>
 #include <vector>
@@ -138,6 +139,84 @@ check_size_queries (cruft::TAP::logger &tap)
 }
 
 
+//-----------------------------------------------------------------------------
+static void
+check_destructors (cruft::TAP::logger &tap)
+{
+    struct counter {
+        counter (int *_target): target (_target) { ; }
+
+        counter (counter const &) = delete;
+        counter& operator= (counter const &) = delete;
+
+        counter (counter &&) = delete;
+        counter& operator= (counter &&) = delete;
+
+        ~counter () { if (target) ++*target; }
+
+        int *target;
+    };
+
+    int count = 0;
+    int expected = 0;
+
+    {
+        cruft::pool<counter> data (8);
+        auto *first = data.construct (&count);
+        auto const *second = data.construct (&count);
+        (void)second;
+
+        CHECK_EQ (count, 0);
+
+        data.destroy (first);
+        ++expected;
+        tap.expect_eq (count, expected, "destructors run on destroy");
+    }
+
+    expected++;
+    tap.expect_eq (count, expected, "single destructor run on pool destructor");
+
+    // Make an pool without allocations and destroy it. This isn't reported
+    // via TAP, but is used as a sanity check that the destructor doesn't run
+    // into infinite loops or other such problems.
+    {
+        cruft::pool<counter> data (8);
+    }
+
+    // Destroy a full pool
+    {
+        cruft::pool<counter> data (8);
+        for (int i = 0; i < 8; ++i) {
+            data.construct (&count);
+            ++expected;
+        }
+    }
+
+    tap.expect_eq (count, expected, "prestine full pool destructor triggers all data desctructors");
+
+    {
+        cruft::pool<counter> data (8);
+        counter* items[8];
+        for (int i = 0; i < 8; ++i) {
+            items[i] = data.construct (&count);
+            ++expected;
+        }
+
+        for (int round = 0; round < 128; ++round) {
+            auto idx = cruft::random::uniform (std::size (items) - 1);
+            data.destroy (items[idx]);
+            items[idx] = data.construct (&count);
+            ++expected;
+        }
+    }
+
+    tap.expect_eq (count, expected, "randomised full pool destructor triggers all data desctructors");
+
+    cruft::pool<int> data (100*1024);
+    (void)data;
+}
+
+
 //-----------------------------------------------------------------------------
 int
 main (int, char **)
@@ -148,5 +227,6 @@ main (int, char **)
         check_keep_value (tap);
         check_keep_variadic_value (tap);
         check_size_queries (tap);
+        check_destructors (tap);
     });
 }