Defensive code for alloc/dealloc during TLS teardown (#161)

* Defensive code for alloc/dealloc during TLS teardown

If an allocation or deallocation occurs during TLS teardown, then it is
possible for a new allocator to be created and then this is leaked. On
the mimalloc-bench mstressN benchmark this was observed leading to a
large memory leak.

This fix, detects if we are in the TLS teardown phase, and if so,
the calls to alloc or dealloc must return the allocator once they have
perform the specific operation.

Uses a separate variable to represent if a thread_local's destructor has
run already.  This is used to detect thread teardown to put the
allocator into a special slow path to avoid leaks.

* Added some printing first operation to track progress

* Improve error messages on posix

Flush errors, print assert details, and present stack traces.

* Detect incorrect use of pool.

* Clang format.

* Replace broken LL/SC implementation

LL/SC implementation was broken, this replaces it with
a locking implementation. Changes the API to support LL/SC
for future implementation on ARM.

* Improve TLS teardown.

* Make std::function fully inlined.

* Factor out PALLinux stack trace.

* Add checks for leaking allocators.

* Add release build of Windows Clang

src/mem/threadalloc.h

@@ -51,14 +51,25 @@ namespace snmalloc
   }
 
   /**
-   * Function passed as a tempalte parameter to `Allocator` to allow lazy
+   * Function passed as a template parameter to `Allocator` to allow lazy
    * replacement.  There is nothing to initialise in this case, so we expect
    * this to never be called.
    */
-  SNMALLOC_FAST_PATH void* init_thread_allocator()
+#  ifdef _MSC_VER
+// 32Bit Windows release MSVC is determining this as having unreachable code for
+// f(nullptr), which is true.  But other platforms don't. Disabling the warning
+// seems simplist.
+#    pragma warning(push)
+#    pragma warning(disable : 4702)
+#  endif
+  SNMALLOC_FAST_PATH void* init_thread_allocator(std::function<void*(void*)>& f)
   {
-    return nullptr;
+    error("Critical Error: This should never be called.");
+    return f(nullptr);
   }
+#  ifdef _MSC_VER
+#    pragma warning(pop)
+#  endif
 
   using ThreadAlloc = ThreadAllocUntypedWrapper;
 #else
@@ -90,15 +101,27 @@ namespace snmalloc
    */
   class ThreadAllocCommon
   {
-    friend void* init_thread_allocator();
+    friend void* init_thread_allocator(std::function<void*(void*)>&);
 
   protected:
+    /**
+     * Thread local variable that is set to true, once `inner_release`
+     * has been run.  If we try to reinitialise the allocator once
+     * `inner_release` has run, then we can stay on the slow path so we don't
+     * leak allocators.
+     *
+     * This is required to allow for the allocator to be called during
+     * destructors of other thread_local state.
+     */
+    inline static thread_local bool destructor_has_run = false;
+
     static inline void inner_release()
     {
       auto& per_thread = get_reference();
       if (per_thread != get_GlobalPlaceHolder())
       {
         current_alloc_pool()->release(per_thread);
+        destructor_has_run = true;
         per_thread = get_GlobalPlaceHolder();
       }
     }
@@ -106,11 +129,12 @@ namespace snmalloc
     /**
      * Default clean up does nothing except print statistics if enabled.
      */
-    static void register_cleanup()
+    static bool register_cleanup()
     {
 #  ifdef USE_SNMALLOC_STATS
       Singleton<int, atexit_print_stats>::get();
 #  endif
+      return false;
     }
 
 #  ifdef USE_SNMALLOC_STATS
@@ -134,7 +158,9 @@ namespace snmalloc
      */
     static inline Alloc*& get_reference()
     {
-      static thread_local Alloc* alloc = get_GlobalPlaceHolder();
+      // Inline casting as codegen doesn't create a lazy init like this.
+      static thread_local Alloc* alloc =
+        const_cast<Alloc*>(reinterpret_cast<const Alloc*>(&GlobalPlaceHolder));
       return alloc;
     }
 
@@ -158,19 +184,23 @@ namespace snmalloc
     /**
      * Public interface, returns the allocator for this thread, constructing
      * one if necessary.
-     *
-     * The returned Alloc* is guaranteed to be initialised.  This incurs a cost,
-     * so use `get_noncachable` if you can meet its criteria.
+     * This incurs a cost, so use `get_noncachable` if you can meet its
+     * criteria.
      */
     static SNMALLOC_FAST_PATH Alloc* get()
     {
 #  ifdef USE_MALLOC
       return get_reference();
 #  else
-      auto alloc = get_reference();
-      if (unlikely(needs_initialisation(alloc)))
+      auto*& alloc = get_reference();
+      if (unlikely(needs_initialisation(alloc)) && !destructor_has_run)
       {
-        alloc = reinterpret_cast<Alloc*>(init_thread_allocator());
+        std::function<void*(void*)> f = [](void*) { return nullptr; };
+        // Call `init_thread_allocator` to perform down call in case
+        // register_clean_up does more.
+        // During teardown for the destructor based ThreadAlloc this will set
+        // alloc to GlobalPlaceHolder;
+        init_thread_allocator(f);
       }
       return alloc;
 #  endif
@@ -214,11 +244,13 @@ namespace snmalloc
     friend class OnDestruct;
 
   public:
-    static void register_cleanup()
+    static bool register_cleanup()
     {
       static thread_local OnDestruct<ThreadAllocCommon::inner_release> tidier;
 
       ThreadAllocCommon::register_cleanup();
+
+      return destructor_has_run;
     }
   };
 
@@ -243,21 +275,24 @@ namespace snmalloc
    * The simple check that this is the global placeholder is inlined, the rest
    * of it is only hit in a very unusual case and so should go off the fast
    * path.
+   * The second component of the return indicates if this TLS is being torndown.
    */
-  SNMALLOC_SLOW_PATH inline void* init_thread_allocator()
+  SNMALLOC_FAST_PATH void* init_thread_allocator(std::function<void*(void*)>& f)
   {
     auto*& local_alloc = ThreadAlloc::get_reference();
-    if (local_alloc != get_GlobalPlaceHolder())
+    // If someone reuses a noncachable call, then we can end up here
+    // with an already initialised allocator. Could either error
+    // to say stop doing this, or just give them the initialised version.
+    if (local_alloc == get_GlobalPlaceHolder())
     {
-      // If someone reuses a noncachable call, then we can end up here.
-      // The allocator has already been initialised. Could either error
-      // to say stop doing this, or just give them the initialised version.
-      return local_alloc;
+      local_alloc = current_alloc_pool()->acquire();
     }
-    local_alloc = current_alloc_pool()->acquire();
-    SNMALLOC_ASSERT(local_alloc != get_GlobalPlaceHolder());
-    ThreadAlloc::register_cleanup();
-    return local_alloc;
+    auto result = f(local_alloc);
+    // Check if we have already run the destructor for the TLS.  If so,
+    // we need to deallocate the allocator.
+    if (ThreadAlloc::register_cleanup())
+      ThreadAlloc::inner_release();
+    return result;
   }
 
   /**