fix: guard-free recycling frame allocator, survives thread-exit teardown

Rework the recycling frame allocator so its per-thread pool is
trivially destructible and constant-initialized. This removes the
guard variable the compiler placed on the thread_local in local()
(checked on every allocate/free), making the hot path a bare TLS
load, and it fixes a thread-exit crash.

Previously the pool held a non-trivial destructor that drained its
cached blocks at thread exit. On MinGW toolchains that thread_local
teardown can run twice, the second time on the already freed pool
block, so the drain read a poisoned bucket count and indexed wild
(observed as an IOCP-thread heap corruption once multi-threaded
io_context tests began exiting worker threads, surfaced under gcov).

With no pool destructor, that teardown cannot double-run. Cached
blocks are reclaimed off the hot path instead: per thread by a
janitor armed once (from the slow paths, and behind a guard-free
flag in deallocate_fast) whose destructor drains via local() and is
naturally idempotent (a second run finds empty buckets) and never
touches its own storage; and the global pool by its holder's
destructor at process exit. Adopts the guard-free allocator design
from perf/frame-alloc-guard-free. Adds a thread-exit teardown
regression test.

Author: sgerbino

include/boost/capy/ex/recycling_memory_resource.hpp

@@ -33,15 +33,6 @@ namespace capy {
     This is the default allocator used by run_async when no allocator
     is specified.
 
-    @note This resource honors only the default new alignment
-    (`__STDCPP_DEFAULT_NEW_ALIGNMENT__`, typically
-    `alignof(std::max_align_t)`). The alignment argument passed to
-    `do_allocate`/`do_deallocate` (and to `allocate_fast`/`deallocate_fast`)
-    is ignored; backing storage comes from `::operator new`. Over-aligned
-    requests are therefore not satisfied. This is sufficient for coroutine
-    frame allocation but means the resource cannot be used where
-    over-aligned memory is required.
-
     @par Thread Safety
     Thread-safe. The thread-local pool requires no synchronization.
     The global pool uses a mutex for cross-thread access.
@@ -80,7 +71,7 @@ class BOOST_CAPY_DECL recycling_memory_resource : public std::pmr::memory_resour
     struct bucket
     {
         std::size_t count = 0;
-        void* ptrs[bucket_capacity];
+        void* ptrs[bucket_capacity] = {};
 
         void* pop() noexcept
         {
@@ -114,12 +105,12 @@ class BOOST_CAPY_DECL recycling_memory_resource : public std::pmr::memory_resour
     {
         bucket buckets[num_classes];
 
-        ~pool()
-        {
-            for(auto& b : buckets)
-                while(b.count > 0)
-                    ::operator delete(b.pop());
-        }
+        // No destructor: a non-trivial dtor forces a guard variable on the
+        // thread_local in local(), checked on every alloc/free. Constant
+        // initialization plus a trivial dtor makes that access a bare TLS
+        // load. Cached blocks are instead reclaimed explicitly: per-thread
+        // by arm_thread_cleanup() at thread exit, and the global pool by
+        // global()'s holder destructor at process exit.
     };
 
     static pool& local() noexcept
@@ -134,31 +125,23 @@ class BOOST_CAPY_DECL recycling_memory_resource : public std::pmr::memory_resour
     void* allocate_slow(std::size_t rounded, std::size_t idx);
     void deallocate_slow(void* p, std::size_t idx);
 
-public:
-    /** Destructor.
+    // Register a thread-exit callback that drains this thread's local
+    // pool back to the OS. Called only off the hot path: unconditionally
+    // from the slow paths, and once per thread from deallocate_fast
+    // behind a guard-free flag.
+    static void arm_thread_cleanup() noexcept;
 
-        Releases any blocks still held in this resource's thread-local
-        pool for the calling thread. Blocks held in the process-wide
-        global pool, and in other threads' thread-local pools, are
-        released when those pools are destroyed.
-    */
+public:
     ~recycling_memory_resource();
 
     /** Allocate without virtual dispatch.
 
         Handles the fast path inline (thread-local bucket pop)
         and falls through to the slow path for global pool or
         heap allocation.
-
-        @param bytes The number of bytes to allocate.
-
-        @return A pointer to the allocated storage.
-
-        @note The second (alignment) argument is ignored; only the
-        default new alignment is honored. See the class-level note.
     */
     void*
-    allocate_fast(std::size_t bytes, std::size_t /*alignment*/)
+    allocate_fast(std::size_t bytes, std::size_t)
     {
         std::size_t rounded = round_up_pow2(bytes);
         std::size_t idx = get_class_index(rounded);
@@ -175,17 +158,9 @@ class BOOST_CAPY_DECL recycling_memory_resource : public std::pmr::memory_resour
         Handles the fast path inline (thread-local bucket push)
         and falls through to the slow path for global pool or
         heap deallocation.
-
-        @param p Pointer previously returned by `allocate_fast`
-        (or `do_allocate`) on a resource that compares equal to this one.
-
-        @param bytes The size, in bytes, originally requested for `p`.
-
-        @note The third (alignment) argument is ignored; only the
-        default new alignment is honored. See the class-level note.
     */
     void
-    deallocate_fast(void* p, std::size_t bytes, std::size_t /*alignment*/)
+    deallocate_fast(void* p, std::size_t bytes, std::size_t)
     {
         std::size_t rounded = round_up_pow2(bytes);
         std::size_t idx = get_class_index(rounded);
@@ -194,36 +169,27 @@ class BOOST_CAPY_DECL recycling_memory_resource : public std::pmr::memory_resour
             ::operator delete(p);
             return;
         }
+        // Guard-free flag (constinit bool, trivial dtor): arms thread-exit
+        // cleanup exactly once for any thread that caches via deallocate,
+        // including consumer threads that never hit a slow path.
+        static thread_local bool armed = false;
+        if(!armed)
+        {
+            armed = true;
+            arm_thread_cleanup();
+        }
         auto& lp = local();
         if(lp.buckets[idx].push(p))
             return;
         deallocate_slow(p, idx);
     }
 
 protected:
-    /** Allocate storage (`std::pmr::memory_resource` interface).
-
-        Forwards to `allocate_fast`. The alignment argument is ignored;
-        see the class-level note.
-
-        @param bytes The number of bytes to allocate.
-
-        @return A pointer to the allocated storage.
-    */
     void*
-    do_allocate(std::size_t bytes, std::size_t /*alignment*/) override;
-
-    /** Deallocate storage (`std::pmr::memory_resource` interface).
-
-        Forwards to `deallocate_fast`. The alignment argument is ignored;
-        see the class-level note.
+    do_allocate(std::size_t bytes, std::size_t) override;
 
-        @param p Pointer previously returned by `do_allocate`.
-
-        @param bytes The size, in bytes, originally requested for `p`.
-    */
     void
-    do_deallocate(void* p, std::size_t bytes, std::size_t /*alignment*/) override;
+    do_deallocate(void* p, std::size_t bytes, std::size_t) override;
 
     bool
     do_is_equal(const memory_resource& other) const noexcept override

src/ex/recycling_memory_resource.cpp

@@ -9,29 +9,79 @@
 
 #include <boost/capy/ex/recycling_memory_resource.hpp>
 
+#include <new>
+
 namespace boost {
 namespace capy {
 
+// Instance destruction does nothing: the resource is stateless (all pools
+// are static). Global-pool cleanup is owned by global()'s holder, exactly
+// as in the original where the global pool's own destructor drained it,
+// independent of any instance lifetime.
 recycling_memory_resource::~recycling_memory_resource() = default;
 
+void
+recycling_memory_resource::arm_thread_cleanup() noexcept
+{
+    struct janitor
+    {
+        ~janitor()
+        {
+            // Return this thread's cached blocks to the OS so nothing
+            // leaks at thread exit. The pool has a trivial dtor, so its
+            // thread-local storage is still valid here.
+            auto& lp = local();
+            for(auto& b : lp.buckets)
+                while(b.count > 0)
+                    ::operator delete(b.pop());
+        }
+    };
+    static thread_local janitor j;
+    (void)j;
+}
+
 recycling_memory_resource::pool&
 recycling_memory_resource::global() noexcept
 {
-    static pool p;
-    return p;
+    // Holder gives the global pool a destructor (the trivial-dtor pool type
+    // itself must stay guard-free for local()). Runs unconditionally at
+    // process exit, mirroring the original global pool destructor, and is
+    // locked because a worker thread may still be in a slow path. This path
+    // is cold (slow paths only), so the holder's guard costs nothing hot.
+    struct holder
+    {
+        pool p;
+
+        ~holder()
+        {
+            std::lock_guard<std::mutex> lock(global_mutex());
+            for(auto& b : p.buckets)
+                while(b.count > 0)
+                    ::operator delete(b.pop());
+        }
+    };
+    static holder h;
+    return h.p;
 }
 
 std::mutex&
 recycling_memory_resource::global_mutex() noexcept
 {
-    static std::mutex mtx;
-    return mtx;
+    // Never destroyed: it is locked during process-exit teardown (in
+    // global()'s holder destructor), after a function-local `static
+    // std::mutex` could itself have been destroyed. Placement-new into
+    // static storage owns no heap allocation, so there is nothing to leak.
+    alignas(std::mutex) static unsigned char storage[sizeof(std::mutex)];
+    static std::mutex* const mtx =
+        ::new(static_cast<void*>(storage)) std::mutex();
+    return *mtx;
 }
 
 void*
 recycling_memory_resource::allocate_slow(
     std::size_t rounded, std::size_t idx)
 {
+    arm_thread_cleanup();
     {
         std::lock_guard<std::mutex> lock(global_mutex());
         if(auto* p = global().buckets[idx].pop(local().buckets[idx]))

test/unit/ex/recycling_memory_resource.cpp

@@ -11,6 +11,7 @@
 #include <boost/capy/ex/recycling_memory_resource.hpp>
 
 #include <cstddef>
+#include <thread>
 #include <vector>
 
 #include "test_suite.hpp"
@@ -69,11 +70,50 @@ class recycling_memory_resource_test
         drain();
     }
 
+    void
+    testThreadExitTeardown()
+    {
+        // Each worker fills its thread-local pool with cached blocks and
+        // then exits, running the pool's thread-exit drain. This is the
+        // path that must survive the pool teardown being reached once
+        // per exiting thread (and, on MinGW, potentially reached twice).
+        // Repeat across many short-lived threads so a teardown fault
+        // has ample opportunity to surface.
+        constexpr int threads = 32;
+        constexpr int rounds = 8;
+        constexpr std::size_t bytes = 128; // size class 1
+        constexpr std::size_t align = 8;
+
+        for(int r = 0; r < rounds; ++r)
+        {
+            std::vector<std::thread> ts;
+            ts.reserve(threads);
+            for(int t = 0; t < threads; ++t)
+            {
+                ts.emplace_back([] {
+                    recycling_memory_resource mr;
+                    std::vector<void*> ptrs;
+                    for(int i = 0; i < 40; ++i)
+                        ptrs.push_back(mr.allocate_fast(bytes, align));
+                    // Return them so the thread-local pool caches blocks
+                    // that its exit-time drain must release.
+                    for(void* p : ptrs)
+                        mr.deallocate_fast(p, bytes, align);
+                });
+            }
+            for(auto& t : ts)
+                t.join();
+        }
+
+        BOOST_TEST_PASS();
+    }
+
     void
     run()
     {
         testIsEqual();
         testSlowPaths();
+        testThreadExitTeardown();
     }
 };