diff --git a/CMakeLists.txt b/CMakeLists.txt index bddb9058f..eee0d7d5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -877,7 +877,7 @@ if (MI_BUILD_TESTS) enable_testing() # static link tests - set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff free-before-init) + set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba heap-burst-destroy fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff free-before-init) if(NOT (MI_DEBUG_TSAN OR MI_TRACK_ASAN OR MI_DEBUG_UBSAN)) list(APPEND mi_static_tests thp-optout) # counts madvise calls by interposing it, which a sanitizer runtime does first endif() diff --git a/src/free.c b/src/free.c index a7bd24760..295ed7138 100644 --- a/src/free.c +++ b/src/free.c @@ -276,10 +276,29 @@ void _mi_free_subproc_safe_in_page(void* p, mi_page_t* page) mi_attr_noexcept { mi_free_ex(p, page, NULL, false); } -// Free a pointer that is potentially allocated in a different sub-process +// The sub-process of a page we do not own. On a multi-threaded free we cannot go through `page->heap` +// (see `mi_page_heap`): a concurrent `mi_heap_delete` moves the page to the main heap and frees the heap +// struct. The arena of a page is constant for the lifetime of the page instead; for the (rare) OS +// allocated pages we fall back to our own subproc. +static mi_subproc_t* mi_page_subproc_unowned(const mi_page_t* page) { + if mi_likely(page->memid.memkind == MI_MEM_ARENA) { + return page->memid.mem.arena.arena->subproc; + } + else { + return _mi_subproc(); + } +} + +// Free a pointer that is potentially allocated in a different sub-process. +// Collecting an abandoned page (free it, reclaim it, or re-map it) must not cross sub-processes, +// but inside our own it must still happen: a full page is abandoned and unmapped, and a block +// freed into it without collecting is never found again and pins the page (`mi_heap_free` frees +// every `mi_heap_t` and `mi_arena_pages_t` through here). void _mi_free_subproc_safe(void* p) mi_attr_noexcept { mi_page_t* const page = mi_validate_ptr_page(p,"_mi_free_subproc_safe"); - mi_free_ex(p, page, NULL, false); + if mi_unlikely(page==NULL) return; + const bool allow_collect = (mi_page_subproc_unowned(page) == _mi_subproc()); + mi_free_ex(p, page, NULL, allow_collect); } @@ -709,26 +728,14 @@ static void mi_check_padding(const mi_page_t* page, const mi_block_t* block) { // only maintain stats for smaller objects if requested #if (MI_STAT>0) -// The subproc to account a free on when we cannot use our own theap. On a multi-threaded free we do not -// own `page`, so we cannot go through `page->heap` (see `mi_page_heap`): a concurrent `mi_heap_delete` -// moves the page to the main heap and frees the heap struct. The arena of a page is constant for the -// lifetime of the page instead; for the (rare) OS allocated pages we fall back to our own subproc. -static mi_subproc_t* mi_stat_free_subproc(const mi_page_t* page) { - if mi_likely(page->memid.memkind == MI_MEM_ARENA) { - return page->memid.mem.arena.arena->subproc; - } - else { - return _mi_subproc(); - } -} - static void mi_stat_free(const mi_page_t* page, const mi_block_t* block) { MI_UNUSED(block); mi_theap_t* theap = _mi_theap_default(); mi_lock_t* lock = NULL; if mi_unlikely(!mi_theap_is_initialized(theap) || // can happen if free'd after thread_done was called (usually a thread cleanup call by the OS) mi_page_thread_id(page) == MI_THREADID_DETACHED) { // page->theap == subproc->theap_meta .. but we cannot read `theap` if we don't own the page (only the meta theap has the detached thread id) - mi_subproc_t* const subproc = mi_stat_free_subproc(page); + // account on the subproc of the page (we cannot use our own theap here) + mi_subproc_t* const subproc = mi_page_subproc_unowned(page); if (subproc->theap_meta == NULL) return; // give up (the subproc is being destroyed) theap = subproc->theap_meta; lock = &subproc->theap_meta_lock; diff --git a/test/test-heap-burst-destroy.c b/test/test-heap-burst-destroy.c new file mode 100644 index 000000000..e8f9b6817 --- /dev/null +++ b/test/test-heap-burst-destroy.c @@ -0,0 +1,133 @@ +/* ---------------------------------------------------------------------------- +Copyright (c) 2026, Microsoft Research, Daan Leijen +This is free software; you can redistribute it and/or modify it under the +terms of the MIT license. +-----------------------------------------------------------------------------*/ + +/* test-heap-burst-destroy.c + + Destroy many heaps at once, round after round, and check that the meta data + of a destroyed heap (`mi_heap_t`, `mi_arena_pages_t`) really goes away. + + mimalloc abandons a page once it is full. The only way such a page comes back + is a free into it that "collects" the page: frees it when it is empty, reclaims + it into the current theap, or re-maps it once enough blocks are free. + `mi_heap_free` frees the meta data through `_mi_free_subproc_safe`, which did + not collect (a collect must not cross sub-processes). Inside the same + sub-process that stranded the block: the page stayed abandoned and resident + for good. With enough heaps alive at the same time their meta data fills + whole pages of the main heap, so from then on every `mi_heap_destroy` leaked + the heap struct and, once the heap had allocated, its arena page bitmaps + (about 150 KiB for a 1 GiB arena). + + The test counts the bytes in use in the main heap with `mi_heap_visit_blocks` + (that walks every page of the heap, abandoned ones included, so a stranded + block is counted) before and after the rounds. Without the fix the count + grows by the meta data of every destroyed heap. + + > mimalloc-test-heap-burst-destroy [LIVE_HEAPS] [ROUNDS] +*/ + +#include +#include +#include +#include +#include + +#include "mimalloc.h" + +static int failed = 0; +#define EXPECT(what, cond) do { if (!(cond)) { fprintf(stderr, "\n FAILED %s: %s (%s:%d)\n", what, #cond, __FILE__, __LINE__); failed++; } } while(0) + +static bool count_used_bytes(const mi_heap_t* heap, const mi_heap_area_t* area, void* block, size_t block_size, void* arg) { + (void)heap; (void)block; (void)block_size; + if (area != NULL) { + *((size_t*)arg) += area->used * area->block_size; + } + return true; +} + +// Bytes in use in the main heap, over every page of it (also the abandoned ones). +static size_t main_heap_used_bytes(void) { + size_t used = 0; + mi_heap_visit_blocks(mi_heap_main(), false /* pages only */, &count_used_bytes, &used); + return used; +} + +static size_t current_rss(void) { + size_t elapsed, user, sys, current_rss, peak_rss, current_commit, peak_commit, page_faults; + mi_process_info(&elapsed, &user, &sys, ¤t_rss, &peak_rss, ¤t_commit, &peak_commit, &page_faults); + return current_rss; +} + +// One round: `live` heaps alive at the same time, each with one allocation (so +// that each heap also has its `mi_arena_pages_t`), then all of them destroyed. +static void round_of_heaps(mi_heap_t** heaps, int live, bool allocate) { + for (int i = 0; i < live; i++) { + heaps[i] = mi_heap_new(); + EXPECT("heap_new", heaps[i] != NULL); + if (allocate) { + void* p = mi_heap_malloc(heaps[i], 200); + EXPECT("heap_malloc", p != NULL); + memset(p, (int)(i & 0xFF), 200); + } + } + for (int i = 0; i < live; i++) { + mi_heap_destroy(heaps[i]); + heaps[i] = NULL; + } +} + +int main(int argc, char** argv) { + int live = (argc > 1 ? atoi(argv[1]) : 200); + int rounds = (argc > 2 ? atoi(argv[2]) : 20); + if (live <= 0) live = 200; + if (rounds <= 0) rounds = 20; + + mi_heap_t** heaps = (mi_heap_t**)mi_calloc((size_t)live, sizeof(mi_heap_t*)); + EXPECT("alloc heaps array", heaps != NULL); + if (heaps == NULL) return 1; + + // warm up: the first round pays for thread-local slots, theap meta data, and the like + round_of_heaps(heaps, live, true); + const size_t used_before = main_heap_used_bytes(); + const size_t rss_before = current_rss(); + + for (int r = 0; r < rounds; r++) { + round_of_heaps(heaps, live, true); + } + const size_t used_after = main_heap_used_bytes(); + const size_t rss_after = current_rss(); + + // heaps without any allocation strand only their `mi_heap_t` + for (int r = 0; r < rounds; r++) { + round_of_heaps(heaps, live, false); + } + const size_t used_after_empty = main_heap_used_bytes(); + + mi_free(heaps); + + const size_t destroyed = (size_t)live * (size_t)rounds; + printf("heap-burst-destroy: %d live heaps x %d rounds\n", live, rounds); + printf(" main heap in use: %zu KiB before, %zu KiB after (%zu B per destroyed heap)\n", + used_before / 1024, used_after / 1024, + (used_after > used_before ? (used_after - used_before) / destroyed : 0)); + printf(" main heap in use after %zu empty heaps: %zu KiB (%zu B per destroyed heap)\n", + destroyed, used_after_empty / 1024, + (used_after_empty > used_after ? (used_after_empty - used_after) / destroyed : 0)); + printf(" rss: %zu KiB before, %zu KiB after\n", rss_before / 1024, rss_after / 1024); + + // Without the fix every destroyed heap leaves about 7 KiB (`mi_heap_t`) plus about 150 KiB + // (`mi_arena_pages_t`) in use: `destroyed * 157 KiB`. With it the count stays where the warm-up + // round left it; allow a few pages of slack for meta data that legitimately grows. + const size_t slack = 4 * 64 * 1024; + EXPECT("meta data of destroyed heaps is freed", used_after <= used_before + slack); + EXPECT("meta data of destroyed empty heaps is freed", used_after_empty <= used_before + slack); + + if (failed > 0) { + fprintf(stderr, "test-heap-burst-destroy: %d failure(s)\n", failed); + return 1; + } + printf("test-heap-burst-destroy: ok\n"); + return 0; +}