From 74657d9dbc776a54f78b77896a9955ebce5c8076 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Tue, 7 Apr 2020 15:37:26 +0100 Subject: [PATCH] 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 --- CMakeLists.txt | 4 + azure-pipelines.yml | 22 +-- src/ds/aba.h | 159 ++++++++++++------ src/ds/defines.h | 6 +- src/ds/mpmcstack.h | 12 +- src/mem/alloc.h | 56 +++--- src/mem/globalalloc.h | 67 ++++---- src/mem/pool.h | 15 ++ src/mem/pooled.h | 21 +++ src/mem/threadalloc.h | 81 ++++++--- src/pal/pal.h | 2 +- src/pal/pal_posix.h | 15 ++ .../func/first_operation/first_operation.cc | 46 ++++- 13 files changed, 359 insertions(+), 147 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 045c996..13887a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -147,6 +147,10 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY) set(CMAKE_EXE_LINKER_FLAGS_RELEASE "${CMAKE_EXE_LINKER_FLAGS_RELEASE} /DEBUG") else() add_compile_options(-fno-exceptions -fno-rtti -g -ftls-model=initial-exec -fomit-frame-pointer) + if(SNMALLOC_CI_BUILD OR (${CMAKE_BUILD_TYPE} MATCHES "Debug")) + # Get better stack traces in CI and Debug. + target_link_libraries(snmalloc_lib INTERFACE "-rdynamic") + endif() if((${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86_64") OR (${CMAKE_SYSTEM_PROCESSOR} STREQUAL "x86") OR diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 5b825d1..f65fe87 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -224,7 +224,11 @@ jobs: matrix: 64-bit Debug Clang: BuildType: Debug - CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang++.exe"' + CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_RC_COMPILER="C:/Program Files/LLVM/bin/llvm-rc"' + JFlag: -j 4 + 64-bit Release Clang: + BuildType: Release + CMakeArgs: '-GNinja -DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" -DCMAKE_RC_COMPILER="C:/Program Files/LLVM/bin/llvm-rc"' JFlag: -j 4 steps: @@ -237,15 +241,13 @@ jobs: choco install --accept-license -y Ninja llvm displayName: 'Dependencies' - - task: CMake@1 - displayName: 'CMake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On' - inputs: - cmakeArgs: '.. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On' - - - task: CMake@1 - displayName: 'CMake --build . --config ${BuildType}' - inputs: - cmakeArgs: '--build . --config ${BuildType}' + - script: | + call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat" + mkdir build + cd build + cmake .. $(CMakeArgs) -DCMAKE_BUILD_TYPE=$(BuildType) -DSNMALLOC_CI_BUILD=On -DSNMALLOC_RUST_SUPPORT=On + cmake --build . --config ${BuildType} + displayName: 'build' - script: | set PATH=%PATH%;"C:\Program Files\LLVM\bin\" diff --git a/src/ds/aba.h b/src/ds/aba.h index 55a8e02..c7b3f6c 100644 --- a/src/ds/aba.h +++ b/src/ds/aba.h @@ -2,13 +2,25 @@ #include "bits.h" +/** + * This file contains an abstraction of ABA protection. This API should be + * implementable with double-word compare and exchange or with load-link + * store conditional. + * + * We provide a lock based implementation. + */ namespace snmalloc { +#ifndef NDEBUG + // LL/SC typically can only perform one operation at a time + // check this on other platforms using a thread_local. + inline thread_local bool operation_in_flight = false; +#endif +#ifdef PLATFORM_IS_X86 template class ABA { public: -#ifdef PLATFORM_IS_X86 struct alignas(2 * sizeof(std::size_t)) Linked { T* ptr; @@ -28,21 +40,12 @@ namespace snmalloc sizeof(Linked) == (2 * sizeof(std::size_t)), "Expecting ABA to be the size of two pointers"); - using Cmp = Linked; -#else - using Cmp = T*; -#endif - private: -#ifdef PLATFORM_IS_X86 union { alignas(2 * sizeof(std::size_t)) std::atomic linked; Independent independent; }; -#else - std::atomic raw; -#endif public: ABA() @@ -53,66 +56,116 @@ namespace snmalloc void init(T* x) { -#ifdef PLATFORM_IS_X86 independent.ptr.store(x, std::memory_order_relaxed); independent.aba.store(0, std::memory_order_relaxed); -#else - raw.store(x, std::memory_order_relaxed); -#endif } - T* peek() - { - return -#ifdef PLATFORM_IS_X86 - independent.ptr.load(std::memory_order_relaxed); -#else - raw.load(std::memory_order_relaxed); -#endif - } + struct Cmp; Cmp read() { - return -#ifdef PLATFORM_IS_X86 - Cmp{independent.ptr.load(std::memory_order_relaxed), - independent.aba.load(std::memory_order_relaxed)}; -#else - raw.load(std::memory_order_relaxed); -#endif +# ifndef NDEBUG + if (operation_in_flight) + error("Only one inflight ABA operation at a time is allowed."); + operation_in_flight = true; +# endif + return Cmp{{independent.ptr.load(std::memory_order_relaxed), + independent.aba.load(std::memory_order_relaxed)}, + this}; } - static T* ptr(Cmp& from) + struct Cmp { -#ifdef PLATFORM_IS_X86 - return from.ptr; -#else - return from; -#endif - } + Linked old; + ABA* parent; - bool compare_exchange(Cmp& expect, T* value) - { -#ifdef PLATFORM_IS_X86 + T* ptr() + { + return old.ptr; + } + + bool store_conditional(T* value) + { # if defined(_MSC_VER) && defined(SNMALLOC_VA_BITS_64) - return _InterlockedCompareExchange128( - (volatile __int64*)&linked, - (__int64)(expect.aba + (uintptr_t)1), - (__int64)value, - (__int64*)&expect); + auto result = _InterlockedCompareExchange128( + (volatile __int64*)parent, + (__int64)(old.aba + (uintptr_t)1), + (__int64)value, + (__int64*)&old); # else # if defined(__GNUC__) && !defined(__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16) #error You must compile with -mcx16 to enable 16-byte atomic compare and swap. # endif - Cmp xchg{value, expect.aba + 1}; + Linked xchg{value, old.aba + 1}; + std::atomic& addr = parent->linked; - return linked.compare_exchange_weak( - expect, xchg, std::memory_order_relaxed, std::memory_order_relaxed); + auto result = addr.compare_exchange_weak( + old, xchg, std::memory_order_relaxed, std::memory_order_relaxed); # endif -#else - return raw.compare_exchange_weak( - expect, value, std::memory_order_relaxed, std::memory_order_relaxed); -#endif - } + return result; + } + + ~Cmp() + { +# ifndef NDEBUG + operation_in_flight = false; +# endif + } + + Cmp(const Cmp&) = delete; + }; }; +#else + /** + * Naive implementation of ABA protection using a spin lock. + */ + template + class ABA + { + std::atomic ptr = nullptr; + std::atomic_flag lock = ATOMIC_FLAG_INIT; + + public: + struct Cmp; + + Cmp read() + { + while (lock.test_and_set(std::memory_order_acquire)) + Aal::pause(); + +# ifndef NDEBUG + if (operation_in_flight) + error("Only one inflight ABA operation at a time is allowed."); + operation_in_flight = true; +# endif + + return Cmp{this}; + } + + struct Cmp + { + ABA* parent; + + public: + T* ptr() + { + return parent->ptr; + } + + bool store_conditional(T* t) + { + parent->ptr = t; + return true; + } + + ~Cmp() + { + parent->lock.clear(std::memory_order_release); +# ifndef NDEBUG + operation_in_flight = false; +# endif + } + }; + }; +#endif } // namespace snmalloc diff --git a/src/ds/defines.h b/src/ds/defines.h index a89ed67..74a8e38 100644 --- a/src/ds/defines.h +++ b/src/ds/defines.h @@ -36,6 +36,9 @@ namespace snmalloc void error(const char* const str); } // namespace snmalloc +#define TOSTRING(expr) TOSTRING2(expr) +#define TOSTRING2(expr) #expr + #ifdef NDEBUG # define SNMALLOC_ASSERT(expr) \ {} @@ -44,7 +47,8 @@ namespace snmalloc { \ if (!(expr)) \ { \ - snmalloc::error("assert fail"); \ + snmalloc::error("assert fail: " #expr " in " __FILE__ \ + " on " TOSTRING(__LINE__)); \ } \ } #endif diff --git a/src/ds/mpmcstack.h b/src/ds/mpmcstack.h index 73e5fa7..153bb1e 100644 --- a/src/ds/mpmcstack.h +++ b/src/ds/mpmcstack.h @@ -29,9 +29,9 @@ namespace snmalloc do { - T* top = ABAT::ptr(cmp); + T* top = cmp.ptr(); last->next.store(top, std::memory_order_release); - } while (!stack.compare_exchange(cmp, first)); + } while (!cmp.store_conditional(first)); } T* pop() @@ -44,13 +44,13 @@ namespace snmalloc do { - top = ABAT::ptr(cmp); + top = cmp.ptr(); if (top == nullptr) break; next = top->next.load(std::memory_order_acquire); - } while (!stack.compare_exchange(cmp, next)); + } while (!cmp.store_conditional(next)); return top; } @@ -63,11 +63,11 @@ namespace snmalloc do { - top = ABAT::ptr(cmp); + top = cmp.ptr(); if (top == nullptr) break; - } while (!stack.compare_exchange(cmp, nullptr)); + } while (!cmp.store_conditional(nullptr)); return top; } diff --git a/src/mem/alloc.h b/src/mem/alloc.h index 59f36bb..f5dc2e5 100644 --- a/src/mem/alloc.h +++ b/src/mem/alloc.h @@ -17,6 +17,8 @@ #include "sizeclasstable.h" #include "slab.h" +#include + namespace snmalloc { enum Boundary @@ -60,7 +62,10 @@ namespace snmalloc * passed the global allocator. The second initialises the thread-local * allocator if it is has been been initialised already. Splitting into two * functions allows for the code to be structured into tail calls to improve - * codegen. + * codegen. The second template takes a function that takes the allocator + * that is initialised, and the value returned, is returned by + * `InitThreadAllocator`. This is used incase we are running during teardown + * and the thread local allocator cannot be kept alive. * * The `MemoryProvider` defines the source of memory for this allocator. * Allocators try to reuse address space by allocating from existing slabs or @@ -78,7 +83,7 @@ namespace snmalloc */ template< bool (*NeedsInitialisation)(void*), - void* (*InitThreadAllocator)(), + void* (*InitThreadAllocator)(std::function&), class MemoryProvider = GlobalVirtual, class ChunkMap = SNMALLOC_DEFAULT_CHUNKMAP, bool IsQueueInline = true> @@ -106,7 +111,7 @@ namespace snmalloc return large_allocator.stats; } - template + template friend class AllocPool; /** @@ -1126,7 +1131,7 @@ namespace snmalloc stats().sizeclass_alloc(sizeclass); return small_alloc_new_free_list(sizeclass); } - return small_alloc_first_alloc(sizeclass, size); + return small_alloc_first_alloc(size); } /** @@ -1134,12 +1139,12 @@ namespace snmalloc * then directs the allocation request to the newly created allocator. */ template - SNMALLOC_SLOW_PATH void* - small_alloc_first_alloc(sizeclass_t sizeclass, size_t size) + SNMALLOC_SLOW_PATH void* small_alloc_first_alloc(size_t size) { - auto replacement = InitThreadAllocator(); - return reinterpret_cast(replacement) - ->template small_alloc_inner(sizeclass, size); + std::function f = [size](void* alloc) { + return reinterpret_cast(alloc)->alloc(size); + }; + return InitThreadAllocator(f); } /** @@ -1316,10 +1321,10 @@ namespace snmalloc { if (NeedsInitialisation(this)) { - void* replacement = InitThreadAllocator(); - return reinterpret_cast(replacement) - ->template medium_alloc( - sizeclass, rsize, size); + std::function f = [size](void* alloc) { + return reinterpret_cast(alloc)->alloc(size); + }; + return InitThreadAllocator(f); } slab = reinterpret_cast( large_allocator.template alloc( @@ -1390,9 +1395,10 @@ namespace snmalloc if (NeedsInitialisation(this)) { - void* replacement = InitThreadAllocator(); - return reinterpret_cast(replacement) - ->template large_alloc(size); + std::function f = [size](void* alloc) { + return reinterpret_cast(alloc)->alloc(size); + }; + return InitThreadAllocator(f); } size_t size_bits = bits::next_pow2_bits(size); @@ -1417,9 +1423,12 @@ namespace snmalloc if (NeedsInitialisation(this)) { - void* replacement = InitThreadAllocator(); - return reinterpret_cast(replacement) - ->large_dealloc(p, size); + std::function f = [p](void* alloc) { + reinterpret_cast(alloc)->dealloc(p); + return nullptr; + }; + InitThreadAllocator(f); + return; } size_t size_bits = bits::next_pow2_bits(size); @@ -1470,10 +1479,11 @@ namespace snmalloc // a real allocator and construct one if we aren't. if (NeedsInitialisation(this)) { - void* replacement = InitThreadAllocator(); - // We have to do a dealloc, not a remote_dealloc here because this may - // have been allocated with the allocator that we've just had returned. - reinterpret_cast(replacement)->dealloc(p); + std::function f = [p](void* alloc) { + reinterpret_cast(alloc)->dealloc(p); + return nullptr; + }; + InitThreadAllocator(f); return; } diff --git a/src/mem/globalalloc.h b/src/mem/globalalloc.h index 7c33ed7..37c75dc 100644 --- a/src/mem/globalalloc.h +++ b/src/mem/globalalloc.h @@ -7,31 +7,11 @@ namespace snmalloc { inline bool needs_initialisation(void*); - void* init_thread_allocator(); + void* init_thread_allocator(std::function&); - using Alloc = Allocator< - needs_initialisation, - init_thread_allocator, - GlobalVirtual, - SNMALLOC_DEFAULT_CHUNKMAP, - true>; - - template - class AllocPool : Pool< - Allocator< - needs_initialisation, - init_thread_allocator, - MemoryProvider, - SNMALLOC_DEFAULT_CHUNKMAP, - true>, - MemoryProvider> + template + class AllocPool : Pool { - using Alloc = Allocator< - needs_initialisation, - init_thread_allocator, - MemoryProvider, - SNMALLOC_DEFAULT_CHUNKMAP, - true>; using Parent = Pool; public: @@ -173,19 +153,48 @@ namespace snmalloc UNUSED(result); #endif } + + void debug_in_use(size_t count) + { + auto alloc = Parent::iterate(); + while (alloc != nullptr) + { + if (alloc->debug_is_in_use()) + { + if (count == 0) + { + error("ERROR: allocator in use."); + } + count--; + } + alloc = Parent::iterate(alloc); + + if (count != 0) + { + error("Error: two few allocators in use."); + } + } + } }; - inline AllocPool*& current_alloc_pool() + using Alloc = Allocator< + needs_initialisation, + init_thread_allocator, + GlobalVirtual, + SNMALLOC_DEFAULT_CHUNKMAP, + true>; + + inline AllocPool*& current_alloc_pool() { return Singleton< - AllocPool*, - AllocPool::make>::get(); + AllocPool*, + AllocPool::make>::get(); } - template - inline AllocPool* make_alloc_pool(MemoryProvider& mp) + template + inline AllocPool* make_alloc_pool(MemoryProvider& mp) { - return AllocPool::make(mp); + return AllocPool::make(mp); } } // namespace snmalloc diff --git a/src/mem/pool.h b/src/mem/pool.h index 366e687..c460567 100644 --- a/src/mem/pool.h +++ b/src/mem/pool.h @@ -50,7 +50,10 @@ namespace snmalloc T* p = stack.pop(); if (p != nullptr) + { + p->set_in_use(); return p; + } p = memory_provider .template alloc_chunk( @@ -60,14 +63,21 @@ namespace snmalloc p->list_next = list; list = p; + p->set_in_use(); return p; } + /** + * Return to the pool an object previously retrieved by `acquire` + * + * Do not return objects from `extract`. + */ void release(T* p) { // The object's destructor is not run. If the object is "reallocated", it // is returned without the constructor being run, so the object is reused // without re-initialisation. + p->reset_in_use(); stack.push(p); } @@ -80,6 +90,11 @@ namespace snmalloc return p->next; } + /** + * Return to the pool a list of object previously retrieved by `extract` + * + * Do not return objects from `acquire`. + */ void restore(T* first, T* last) { // Pushes a linked list of objects onto the stack. Use to put a linked diff --git a/src/mem/pooled.h b/src/mem/pooled.h index e9a10af..71f8d91 100644 --- a/src/mem/pooled.h +++ b/src/mem/pooled.h @@ -17,5 +17,26 @@ namespace snmalloc std::atomic next = nullptr; /// Used by the pool to keep the list of all entries ever created. T* list_next; + std::atomic_flag in_use = ATOMIC_FLAG_INIT; + + public: + void set_in_use() + { + if (in_use.test_and_set()) + error("Critical error: double use of Pooled Type!"); + } + + void reset_in_use() + { + in_use.clear(); + } + + bool debug_is_in_use() + { + bool result = in_use.test_and_set(); + if (!result) + in_use.clear(); + return result; + } }; } // namespace snmalloc \ No newline at end of file diff --git a/src/mem/threadalloc.h b/src/mem/threadalloc.h index 969d1dc..47653f0 100644 --- a/src/mem/threadalloc.h +++ b/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& 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&); 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::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(reinterpret_cast(&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(init_thread_allocator()); + std::function 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 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& 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; } /** diff --git a/src/pal/pal.h b/src/pal/pal.h index 42e1b13..53faecb 100644 --- a/src/pal/pal.h +++ b/src/pal/pal.h @@ -49,7 +49,7 @@ namespace snmalloc DefaultPal; #endif - inline void error(const char* const str) + SNMALLOC_SLOW_PATH inline void error(const char* const str) { Pal::error(str); } diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index e15a63f..0289e00 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -3,9 +3,12 @@ #include "../ds/address.h" #include "../mem/allocconfig.h" +#include +#include #include #include #include +#include extern "C" int puts(const char* str); @@ -35,12 +38,24 @@ namespace snmalloc */ static constexpr uint64_t pal_features = LazyCommit; + static void print_stack_trace() + { + constexpr int SIZE = 1024; + void* buffer[SIZE]; + auto nptrs = backtrace(buffer, SIZE); + fflush(stdout); + backtrace_symbols_fd(buffer, nptrs, STDOUT_FILENO); + puts(""); + fflush(stdout); + } + /** * Report a fatal error an exit. */ static void error(const char* const str) noexcept { puts(str); + print_stack_trace(); abort(); } diff --git a/src/test/func/first_operation/first_operation.cc b/src/test/func/first_operation/first_operation.cc index 6af9954..239ac35 100644 --- a/src/test/func/first_operation/first_operation.cc +++ b/src/test/func/first_operation/first_operation.cc @@ -10,6 +10,39 @@ #include #include +/** + * This test is checking lazy init is correctly done with `get`. + * + * The test is written so platforms that do not do lazy init can satify the + * test. + */ +void get_test() +{ + // This should get the GlobalPlaceHolder if using lazy init + auto a1 = snmalloc::ThreadAlloc::get_noncachable(); + + // This should get a real allocator + auto a2 = snmalloc::ThreadAlloc::get(); + + // Trigger potential lazy_init if `get` didn't (shouldn't happen). + a2->dealloc(a2->alloc(5)); + + // Get an allocated allocator. + auto a3 = snmalloc::ThreadAlloc::get_noncachable(); + + if (a1 != a3) + { + printf("Lazy test!\n"); + // If the allocators are different then lazy_init has occurred. + // This should have been caused by the call to `get` rather than + // the allocations. + if (a2 != a3) + { + abort(); + } + } +} + void alloc1(size_t size) { void* r = snmalloc::ThreadAlloc::get_noncachable()->alloc(size); @@ -64,7 +97,7 @@ void f(size_t size) auto t3 = std::thread(alloc3, size); auto t4 = std::thread(alloc4, size); - auto a = snmalloc::ThreadAlloc::get(); + auto a = snmalloc::current_alloc_pool()->acquire(); auto p1 = a->alloc(size); auto p2 = a->alloc(size); auto p3 = a->alloc(size); @@ -83,17 +116,27 @@ void f(size_t size) t6.join(); t7.join(); t8.join(); + snmalloc::current_alloc_pool()->release(a); + snmalloc::current_alloc_pool()->debug_in_use(0); + printf("."); + fflush(stdout); } int main(int, char**) { setup(); + printf("."); + fflush(stdout); + + std::thread t(get_test); + t.join(); f(0); f(1); f(3); f(5); f(7); + printf("\n"); for (size_t exp = 1; exp < snmalloc::SUPERSLAB_BITS; exp++) { f(1ULL << exp); @@ -108,5 +151,6 @@ int main(int, char**) f((3ULL << exp) - 1); f((5ULL << exp) - 1); f((7ULL << exp) - 1); + printf("\n"); } }