From fb5499aa9c13aa7c0e11041009ae894c584b64bd Mon Sep 17 00:00:00 2001 From: Slobodan Predolac Date: Thu, 11 Jun 2026 17:45:02 -0700 Subject: [PATCH] Handle jemalloc calls after TSD teardown This only changes behavior for the tsd_generic path, where pthread_getspecific() can report no TSD after the pthread key destructor has finished. Other TSD backends keep tsd_teardown_done() as a constant false, so the added checks compile out there. Avoid recreating TSD for late deallocations. Preserve existing reincarnation behavior for late allocations and nonzero reallocations. Add Linux CI coverage for force_tls=0, with and without --enable-debug, to exercise the Android-equivalent generic TSD path. --- .github/workflows/linux-ci.yml | 10 + include/jemalloc/internal/tsd.h | 3 +- include/jemalloc/internal/tsd_generic.h | 27 ++ .../internal/tsd_malloc_thread_cleanup.h | 5 + include/jemalloc/internal/tsd_tls.h | 5 + include/jemalloc/internal/tsd_win.h | 10 + scripts/README_GH_ACTIONS.md | 7 +- scripts/gen_gh_actions.py | 12 + src/jemalloc.c | 55 +++- src/tsd.c | 1 + test/unit/tsd.c | 305 +++++++++++++++++- 11 files changed, 432 insertions(+), 8 deletions(-) diff --git a/.github/workflows/linux-ci.yml b/.github/workflows/linux-ci.yml index f8a6842e..106f9600 100644 --- a/.github/workflows/linux-ci.yml +++ b/.github/workflows/linux-ci.yml @@ -508,6 +508,16 @@ jobs: CXX: g++ CONFIGURE_FLAGS: "--enable-debug --enable-experimental-smallocx --enable-stats --enable-prof" EXTRA_CFLAGS: "-Werror -Wno-array-bounds" + - env: + CC: gcc + CXX: g++ + CONFIGURE_FLAGS: force_tls=0 + EXTRA_CFLAGS: "-Werror -Wno-array-bounds" + - env: + CC: gcc + CXX: g++ + CONFIGURE_FLAGS: "force_tls=0 --enable-debug" + EXTRA_CFLAGS: "-Werror -Wno-array-bounds" - env: CC: gcc CXX: g++ diff --git a/include/jemalloc/internal/tsd.h b/include/jemalloc/internal/tsd.h index 91369dde..1d663c8d 100644 --- a/include/jemalloc/internal/tsd.h +++ b/include/jemalloc/internal/tsd.h @@ -4,7 +4,8 @@ /* * We put the platform-specific data declarations and inlines into their own * header files to avoid cluttering this file. They define tsd_boot0, - * tsd_boot1, tsd_boot, tsd_booted_get, tsd_get_allocates, tsd_get, and tsd_set. + * tsd_boot1, tsd_boot, tsd_booted_get, tsd_get_allocates, + * tsd_teardown_done, tsd_get, and tsd_set. */ #ifdef JEMALLOC_MALLOC_THREAD_CLEANUP # include "jemalloc/internal/jemalloc_preamble.h" diff --git a/include/jemalloc/internal/tsd_generic.h b/include/jemalloc/internal/tsd_generic.h index e049766f..0ab7f505 100644 --- a/include/jemalloc/internal/tsd_generic.h +++ b/include/jemalloc/internal/tsd_generic.h @@ -27,6 +27,7 @@ void *tsd_init_check_recursion(tsd_init_head_t *head, tsd_init_block_t *block); void tsd_init_finish(tsd_init_head_t *head, tsd_init_block_t *block); extern pthread_key_t tsd_tsd; +extern pthread_key_t tsd_thread_initialized_tsd; extern tsd_init_head_t tsd_init_head; extern tsd_wrapper_t tsd_boot_wrapper; extern bool tsd_booted; @@ -51,6 +52,12 @@ tsd_cleanup_wrapper(void *arg) { return; } } + /* + * Leave tsd_thread_initialized_tsd set. It has no destructor and + * intentionally outlives the TSD wrapper, so later jemalloc calls can + * distinguish this fully-torn-down thread from one that has never + * initialized TSD. + */ malloc_tsd_dalloc(wrapper); } @@ -63,6 +70,11 @@ tsd_wrapper_set(tsd_wrapper_t *wrapper) { malloc_write(": Error setting TSD\n"); abort(); } + if (pthread_setspecific(tsd_thread_initialized_tsd, + (void *)&tsd_thread_initialized_tsd) != 0) { + malloc_write(": Error setting TSD\n"); + abort(); + } } JEMALLOC_ALWAYS_INLINE tsd_wrapper_t * @@ -116,6 +128,15 @@ tsd_boot0(void) { if (pthread_key_create(&tsd_tsd, tsd_cleanup_wrapper) != 0) { return true; } + /* + * This key has no destructor. It records that the current thread once + * had jemalloc TSD, so a later NULL tsd_tsd value means teardown has + * finished rather than initialization not having happened yet. + */ + if (pthread_key_create(&tsd_thread_initialized_tsd, NULL) != 0) { + pthread_key_delete(tsd_tsd); + return true; + } tsd_booted = true; tsd_wrapper_set(&tsd_boot_wrapper); tsd_init_finish(&tsd_init_head, &block); @@ -160,6 +181,12 @@ tsd_get_allocates(void) { return true; } +JEMALLOC_ALWAYS_INLINE bool +tsd_teardown_done(void) { + return tsd_booted && pthread_getspecific(tsd_tsd) == NULL + && pthread_getspecific(tsd_thread_initialized_tsd) != NULL; +} + /* Get/set. */ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_get(bool init) { diff --git a/include/jemalloc/internal/tsd_malloc_thread_cleanup.h b/include/jemalloc/internal/tsd_malloc_thread_cleanup.h index 00756df1..e1d46de8 100644 --- a/include/jemalloc/internal/tsd_malloc_thread_cleanup.h +++ b/include/jemalloc/internal/tsd_malloc_thread_cleanup.h @@ -50,6 +50,11 @@ tsd_get_allocates(void) { return false; } +JEMALLOC_ALWAYS_INLINE bool +tsd_teardown_done(void) { + return false; +} + /* Get/set. */ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_get(bool init) { diff --git a/include/jemalloc/internal/tsd_tls.h b/include/jemalloc/internal/tsd_tls.h index 6536eb54..15b718d2 100644 --- a/include/jemalloc/internal/tsd_tls.h +++ b/include/jemalloc/internal/tsd_tls.h @@ -43,6 +43,11 @@ tsd_get_allocates(void) { return false; } +JEMALLOC_ALWAYS_INLINE bool +tsd_teardown_done(void) { + return false; +} + /* Get/set. */ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_get(bool init) { diff --git a/include/jemalloc/internal/tsd_win.h b/include/jemalloc/internal/tsd_win.h index 8b22bec1..837f0f89 100644 --- a/include/jemalloc/internal/tsd_win.h +++ b/include/jemalloc/internal/tsd_win.h @@ -146,6 +146,11 @@ tsd_get_allocates(void) { return true; } +JEMALLOC_ALWAYS_INLINE bool +tsd_teardown_done(void) { + return false; +} + /* Get/set. */ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_get(bool init) { @@ -220,6 +225,11 @@ tsd_get_allocates(void) { return false; } +JEMALLOC_ALWAYS_INLINE bool +tsd_teardown_done(void) { + return false; +} + /* Get/set. */ JEMALLOC_ALWAYS_INLINE tsd_t * tsd_get(bool init) { diff --git a/scripts/README_GH_ACTIONS.md b/scripts/README_GH_ACTIONS.md index 1cb236ad..650ad93f 100644 --- a/scripts/README_GH_ACTIONS.md +++ b/scripts/README_GH_ACTIONS.md @@ -27,12 +27,12 @@ The script can generate workflows for different platforms: ### Linux CI (`linux-ci.yml`) - **test-linux** (AMD64): `ubuntu-latest` (x86_64) - - ~96 configurations covering GCC, Clang, various flags + - ~98 configurations covering GCC, Clang, various flags - **test-linux-arm64** (ARM64): `ubuntu-24.04-arm` (aarch64) - ~14 configurations including large hugepage tests - **Note:** Free ARM64 runners (Public Preview) - may have longer queue times during peak hours -**Total:** 110 configurations +**Total:** 112 configurations ### macOS CI (`macos-ci.yml`) - **test-macos** (Intel): `macos-15-intel` (x86_64) @@ -161,7 +161,7 @@ The Windows workflow uses: ### Linux Build Process - Ubuntu Latest for AMD64, Ubuntu 24.04 for ARM64 - Installs 32-bit cross-compilation dependencies when needed -- Most comprehensive test matrix (110 configurations) +- Most comprehensive test matrix (112 configurations) ## Relationship to Travis CI @@ -178,4 +178,3 @@ To regenerate all workflows after modifying `gen_gh_actions.py`: ``` **Note**: The generated files should not be edited by hand. All changes should be made to `gen_gh_actions.py` and then regenerated. - diff --git a/scripts/gen_gh_actions.py b/scripts/gen_gh_actions.py index 5827424f..e35196d5 100755 --- a/scripts/gen_gh_actions.py +++ b/scripts/gen_gh_actions.py @@ -273,6 +273,18 @@ def generate_linux_job(arch): 'CXX': 'g++', 'CONFIGURE_FLAGS': '--enable-debug --enable-experimental-smallocx --enable-stats --enable-prof', 'EXTRA_CFLAGS': '-Werror -Wno-array-bounds' + }, + { + 'CC': 'gcc', + 'CXX': 'g++', + 'CONFIGURE_FLAGS': 'force_tls=0', + 'EXTRA_CFLAGS': '-Werror -Wno-array-bounds' + }, + { + 'CC': 'gcc', + 'CXX': 'g++', + 'CONFIGURE_FLAGS': 'force_tls=0 --enable-debug', + 'EXTRA_CFLAGS': '-Werror -Wno-array-bounds' } ] diff --git a/src/jemalloc.c b/src/jemalloc.c index 3644fe9a..9d5ee8ef 100644 --- a/src/jemalloc.c +++ b/src/jemalloc.c @@ -1025,12 +1025,50 @@ isfree(tsd_t *tsd, void *ptr, size_t usize, tcache_t *tcache, bool slow_path) { thread_dalloc_event(tsd, usize); } +JEMALLOC_ALWAYS_INLINE bool +dealloc_no_tsd(void *ptr) { + /* + * On the generic pthread_getspecific() TSD path, a deallocation can run + * after the thread's TSD has been torn down. Using tsd_fetch_min() here + * would allocate and publish a fresh TSD wrapper mid-teardown, + * reincarnating the state teardown just released (the crashes this change + * fixes). Instead we hand the object straight back to its arena via + * idalloctm() with a NULL tsdn. + * + * This deliberately bypasses the per-thread bookkeeping that the normal + * ifree()/isfree() slow path performs, none of which is available or + * meaningful without a live TSD. The trade-offs, all acceptable for + * these rare teardown-time frees (the memory itself is still correctly + * returned to the arena): + * - Profiling: prof_free() is skipped, so a sampled object is not + * unregistered from its prof context; its bytes stay counted as live + * in prof stats and in any final heap/leak dump. + * - Junk filling: opt_junk_free is not applied to the freed region. + * - Sized dealloc: for sdallocx() the caller-supplied size is ignored + * (szind is looked up from the extent map), so the sized-dealloc + * safety check does not run. + * - Thread events: thread_dalloc_event() does not fire, so this free + * does not advance decay / tcache GC or dalloc stats. + */ + if (!tsd_teardown_done()) { + return false; + } + + idalloctm(TSDN_NULL, ptr, /* tcache */ NULL, /* alloc_ctx */ NULL, + /* is_internal */ false, /* slow_path */ true); + return true; +} + JEMALLOC_NOINLINE void free_default(void *ptr) { UTRACE(ptr, 0, 0); if (likely(ptr != NULL)) { int saved_errno = get_errno(); + if (unlikely(dealloc_no_tsd(ptr))) { + set_errno(saved_errno); + return; + } /* * We avoid setting up tsd fully (e.g. tcache, arena binding) * based on only free() calls -- other activities trigger the @@ -1568,6 +1606,9 @@ do_realloc_nonnull_zero(void *ptr) { return do_rallocx(ptr, 1, MALLOCX_TCACHE_NONE, true); } else if (opt_zero_realloc_action == zero_realloc_action_free) { UTRACE(ptr, 0, 0); + if (unlikely(dealloc_no_tsd(ptr))) { + return NULL; + } tsd_t *tsd = tsd_fetch(); check_entry_exit_locking(tsd_tsdn(tsd)); @@ -1843,6 +1884,12 @@ je_dallocx(void *ptr, int flags) { assert(ptr != NULL); assert(malloc_initialized() || malloc_is_initializer()); + UTRACE(ptr, 0, 0); + if (unlikely(dealloc_no_tsd(ptr))) { + LOG("core.dallocx.exit", ""); + return; + } + tsd_t *tsd = tsd_fetch_min(); bool fast = tsd_fast(tsd); check_entry_exit_locking(tsd_tsdn(tsd)); @@ -1851,7 +1898,6 @@ je_dallocx(void *ptr, int flags) { tcache_t *tcache = tcache_get_from_ind(tsd, tcache_ind, !fast, /* is_alloc */ false); - UTRACE(ptr, 0, 0); if (likely(fast)) { tsd_assert_fast(tsd); ifree(tsd, ptr, tcache, false); @@ -1879,6 +1925,12 @@ sdallocx_default(void *ptr, size_t size, int flags) { assert(ptr != NULL); assert(malloc_initialized() || malloc_is_initializer()); + UTRACE(ptr, 0, 0); + if (unlikely(dealloc_no_tsd(ptr))) { + set_errno(saved_errno); + return; + } + tsd_t *tsd = tsd_fetch_min(); bool fast = tsd_fast(tsd); size_t usize = inallocx(tsd_tsdn(tsd), size, flags); @@ -1888,7 +1940,6 @@ sdallocx_default(void *ptr, size_t size, int flags) { tcache_t *tcache = tcache_get_from_ind(tsd, tcache_ind, !fast, /* is_alloc */ false); - UTRACE(ptr, 0, 0); if (likely(fast)) { tsd_assert_fast(tsd); isfree(tsd, ptr, usize, tcache, false); diff --git a/src/tsd.c b/src/tsd.c index 9c05e17f..849f2297 100644 --- a/src/tsd.c +++ b/src/tsd.c @@ -54,6 +54,7 @@ struct tsd_init_head_s { }; pthread_key_t tsd_tsd; +pthread_key_t tsd_thread_initialized_tsd; tsd_init_head_t tsd_init_head = { ql_head_initializer(blocks), MALLOC_MUTEX_INITIALIZER}; diff --git a/test/unit/tsd.c b/test/unit/tsd.c index 1e291133..ba57d098 100644 --- a/test/unit/tsd.c +++ b/test/unit/tsd.c @@ -250,6 +250,308 @@ TEST_BEGIN(test_tsd_sub_thread_dalloc_only) { } TEST_END +#if !defined(JEMALLOC_MALLOC_THREAD_CLEANUP) && !defined(JEMALLOC_TLS) \ + && !defined(_WIN32) +#define TEST_TSD_AFTER_TEARDOWN +static const bool skip_tsd_after_teardown = false; +#else +static const bool skip_tsd_after_teardown = true; +#endif + +static void * +tsd_raw_get(void) { +#ifdef TEST_TSD_AFTER_TEARDOWN + return pthread_getspecific(tsd_tsd); +#else + return NULL; +#endif +} + +static bool +tsd_raw_clear(void) { +#ifdef TEST_TSD_AFTER_TEARDOWN + return pthread_setspecific(tsd_tsd, NULL) == 0; +#else + return true; +#endif +} + +static void +tsd_teardown_for_test(void) { + tsd_t *tsd = tsd_fetch(); + void *tsd_wrapper = tsd_raw_get(); + expect_ptr_not_null(tsd_wrapper, "TSD wrapper should exist after malloc"); + + /* + * Emulate a jemalloc call after the pthread-key destructor has finished + * with the generic TSD wrapper. This is the state seen by cleanup code + * that runs late in thread teardown on pthread_getspecific()-based + * builds: the first cleanup call moves a nominal TSD to purgatory, where + * deallocation is handled by the existing reincarnation path; the second + * cleanup call emulates the final destructor round, after which the + * wrapper is released and no TSD remains to reincarnate. + */ + tsd_cleanup(tsd); + tsd_cleanup(tsd); + malloc_tsd_dalloc(tsd_wrapper); + expect_true(tsd_raw_clear(), "Unexpected TSD key clear failure"); + expect_ptr_null(tsd_raw_get(), + "TSD key should be clear before the late jemalloc call"); +} + +static void * +thd_start_free_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + void *free_victim = malloc(32); + expect_ptr_not_null(free_victim, "Unexpected malloc() failure"); + void *dallocx_victim = mallocx(32, 0); + expect_ptr_not_null(dallocx_victim, "Unexpected mallocx() failure"); + void *sdallocx_victim = mallocx(32, 0); + expect_ptr_not_null(sdallocx_victim, "Unexpected mallocx() failure"); + + tsd_teardown_for_test(); + + free(free_victim); + expect_ptr_null(tsd_raw_get(), + "free() after TSD teardown must not re-create TSD"); + dallocx(dallocx_victim, 0); + expect_ptr_null(tsd_raw_get(), + "dallocx() after TSD teardown must not re-create TSD"); + sdallocx(sdallocx_victim, 32, 0); + expect_ptr_null(tsd_raw_get(), + "sdallocx() after TSD teardown must not re-create TSD"); + + free(keep); + return NULL; +} + +TEST_BEGIN(test_tsd_free_after_teardown) { + test_skip_if(skip_tsd_after_teardown); + + thd_t thd; + thd_create(&thd, thd_start_free_after_teardown, NULL); + thd_join(thd, NULL); +} +TEST_END + +static void +expect_tsd_recreated(void *p, const char *func) { + expect_ptr_not_null(p, "%s() after TSD teardown should still allocate", + func); + expect_ptr_not_null(tsd_raw_get(), + "%s() after TSD teardown should preserve TSD reincarnation", func); +} + +static void * +thd_start_malloc_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *p = malloc(32); + expect_tsd_recreated(p, "malloc"); + if (p != NULL) { + free(p); + } + + free(keep); + return NULL; +} + +static void * +thd_start_mallocx_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *p = mallocx(32, 0); + expect_tsd_recreated(p, "mallocx"); + if (p != NULL) { + dallocx(p, 0); + } + + free(keep); + return NULL; +} + +static void * +thd_start_calloc_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *p = calloc(1, 32); + expect_tsd_recreated(p, "calloc"); + if (p != NULL) { + free(p); + } + + free(keep); + return NULL; +} + +static void * +thd_start_aligned_alloc_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *p = aligned_alloc(sizeof(void *), 32); + expect_tsd_recreated(p, "aligned_alloc"); + if (p != NULL) { + free(p); + } + + free(keep); + return NULL; +} + +static void * +thd_start_posix_memalign_after_teardown(void *arg) { + (void)arg; + + void *keep = malloc(32); + expect_ptr_not_null(keep, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *p = NULL; + expect_d_eq(posix_memalign(&p, sizeof(void *), 32), 0, + "posix_memalign() after TSD teardown should still allocate"); + expect_tsd_recreated(p, "posix_memalign"); + if (p != NULL) { + free(p); + } + + free(keep); + return NULL; +} + +static void * +thd_start_realloc_after_teardown(void *arg) { + (void)arg; + + void *p = malloc(32); + expect_ptr_not_null(p, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + void *ret = realloc(p, 64); + expect_tsd_recreated(ret, "realloc"); + if (ret != NULL) { + p = ret; + } + free(p); + return NULL; +} + +static void * +thd_start_rallocx_after_teardown(void *arg) { + (void)arg; + + void *p = mallocx(32, 0); + expect_ptr_not_null(p, "Unexpected mallocx() failure"); + + tsd_teardown_for_test(); + + void *ret = rallocx(p, 64, 0); + expect_tsd_recreated(ret, "rallocx"); + if (ret != NULL) { + p = ret; + } + dallocx(p, 0); + return NULL; +} + +static void * +thd_start_xallocx_after_teardown(void *arg) { + (void)arg; + + void *p = mallocx(32, 0); + expect_ptr_not_null(p, "Unexpected mallocx() failure"); + size_t old_usize = sallocx(p, 0); + + tsd_teardown_for_test(); + + expect_zu_ge(xallocx(p, 64, 0, 0), old_usize, + "xallocx() after TSD teardown should preserve behavior"); + expect_ptr_not_null(tsd_raw_get(), + "xallocx() after TSD teardown should preserve TSD reincarnation"); + dallocx(p, 0); + return NULL; +} + +static void * +thd_start_realloc_zero_after_teardown(void *arg) { + (void)arg; + + void *p = malloc(32); + expect_ptr_not_null(p, "Unexpected malloc() failure"); + + tsd_teardown_for_test(); + + if (opt_zero_realloc_action == zero_realloc_action_abort) { + free(p); + expect_ptr_null(tsd_raw_get(), + "free() after TSD teardown must not re-create TSD"); + return NULL; + } + + void *ret = realloc(p, 0); + if (opt_zero_realloc_action == zero_realloc_action_free) { + expect_ptr_null(ret, "realloc(ptr, 0) should return NULL"); + expect_ptr_null(tsd_raw_get(), + "realloc(ptr, 0) after TSD teardown must not re-create TSD"); + } else { + expect_tsd_recreated(ret, "realloc(ptr, 0)"); + if (ret != NULL) { + p = ret; + } + free(p); + } + return NULL; +} + +TEST_BEGIN(test_tsd_malloc_after_teardown) { + test_skip_if(skip_tsd_after_teardown); + + thd_t thd; + thd_create(&thd, thd_start_malloc_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_mallocx_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_calloc_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_aligned_alloc_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_posix_memalign_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_realloc_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_rallocx_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_xallocx_after_teardown, NULL); + thd_join(thd, NULL); + thd_create(&thd, thd_start_realloc_zero_after_teardown, NULL); + thd_join(thd, NULL); +} +TEST_END + int main(void) { /* Ensure tsd bootstrapped. */ @@ -260,5 +562,6 @@ main(void) { return test_no_reentrancy(test_tsd_main_thread, test_tsd_sub_thread, test_tsd_sub_thread_dalloc_only, test_tsd_reincarnation, - test_tsd_reentrant_bootstrap); + test_tsd_reentrant_bootstrap, + test_tsd_free_after_teardown, test_tsd_malloc_after_teardown); }