From a056c20d671e5d001d9d232a7c6d9bb30288e9ef Mon Sep 17 00:00:00 2001 From: Carl Shapiro Date: Mon, 2 Mar 2026 17:15:35 -0800 Subject: [PATCH] Handle tcache init failures gracefully tsd_tcache_data_init() returns true on failure but its callers ignore this return value, leaving the per-thread tcache in an uninitialized state after a failure. This change disables the tcache on an initialization failure and logs an error message. If opt_abort is true, it will also abort. New unit tests have been added to test tcache initialization failures. --- Makefile.in | 1 + include/jemalloc/internal/tcache_externs.h | 5 +- src/tcache.c | 61 +++++++---- test/unit/tcache_init.c | 116 +++++++++++++++++++++ 4 files changed, 162 insertions(+), 21 deletions(-) create mode 100644 test/unit/tcache_init.c diff --git a/Makefile.in b/Makefile.in index 463693df..ec2215b3 100644 --- a/Makefile.in +++ b/Makefile.in @@ -292,6 +292,7 @@ TESTS_UNIT := \ $(srcroot)test/unit/stats.c \ $(srcroot)test/unit/stats_print.c \ $(srcroot)test/unit/sz.c \ + $(srcroot)test/unit/tcache_init.c \ $(srcroot)test/unit/tcache_max.c \ $(srcroot)test/unit/test_hooks.c \ $(srcroot)test/unit/thread_event.c \ diff --git a/include/jemalloc/internal/tcache_externs.h b/include/jemalloc/internal/tcache_externs.h index 73126db7..b7fdb5a4 100644 --- a/include/jemalloc/internal/tcache_externs.h +++ b/include/jemalloc/internal/tcache_externs.h @@ -64,7 +64,7 @@ bool tcache_bin_ncached_max_read( void tcache_arena_reassociate( tsdn_t *tsdn, tcache_slow_t *tcache_slow, tcache_t *tcache, arena_t *arena); tcache_t *tcache_create_explicit(tsd_t *tsd); -void thread_tcache_max_set(tsd_t *tsd, size_t tcache_max); +bool thread_tcache_max_set(tsd_t *tsd, size_t tcache_max); void tcache_cleanup(tsd_t *tsd); void tcache_stats_merge(tsdn_t *tsdn, tcache_t *tcache, arena_t *arena); bool tcaches_create(tsd_t *tsd, base_t *base, unsigned *r_ind); @@ -80,6 +80,9 @@ void tcache_flush(tsd_t *tsd); bool tsd_tcache_enabled_data_init(tsd_t *tsd); void tcache_enabled_set(tsd_t *tsd, bool enabled); +extern void *(*JET_MUTABLE tcache_stack_alloc)(tsdn_t *tsdn, size_t size, + size_t alignment); + void tcache_assert_initialized(tcache_t *tcache); extern te_base_cb_t tcache_gc_te_handler; diff --git a/src/tcache.c b/src/tcache.c index 172d9320..10fa7c21 100644 --- a/src/tcache.c +++ b/src/tcache.c @@ -949,6 +949,21 @@ tcache_bin_info_compute(cache_bin_info_t tcache_bin_info[TCACHE_NBINS_MAX]) { } } +static void * +tcache_stack_alloc_impl(tsdn_t *tsdn, size_t size, size_t alignment) { + if (cache_bin_stack_use_thp()) { + /* Alignment is ignored since it comes from THP. */ + assert(alignment == QUANTUM); + return b0_alloc_tcache_stack(tsdn, size); + } + size = sz_sa2u(size, alignment); + return ipallocztm(tsdn, size, alignment, true, NULL, + true, arena_get(TSDN_NULL, 0, true)); +} + +void *(*JET_MUTABLE tcache_stack_alloc)(tsdn_t *tsdn, size_t size, + size_t alignment) = tcache_stack_alloc_impl; + static bool tsd_tcache_data_init_impl( tsd_t *tsd, arena_t *arena, const cache_bin_info_t *tcache_bin_info) { @@ -961,16 +976,7 @@ tsd_tcache_data_init_impl( cache_bin_info_compute_alloc( tcache_bin_info, tcache_nbins, &size, &alignment); - void *mem; - if (cache_bin_stack_use_thp()) { - /* Alignment is ignored since it comes from THP. */ - assert(alignment == QUANTUM); - mem = b0_alloc_tcache_stack(tsd_tsdn(tsd), size); - } else { - size = sz_sa2u(size, alignment); - mem = ipallocztm(tsd_tsdn(tsd), size, alignment, true, NULL, - true, arena_get(TSDN_NULL, 0, true)); - } + void *mem = tcache_stack_alloc(tsd_tsdn(tsd), size, alignment); if (mem == NULL) { return true; } @@ -1010,7 +1016,20 @@ static bool tsd_tcache_data_init(tsd_t *tsd, arena_t *arena, const cache_bin_info_t tcache_bin_info[TCACHE_NBINS_MAX]) { assert(tcache_bin_info != NULL); - return tsd_tcache_data_init_impl(tsd, arena, tcache_bin_info); + bool err = tsd_tcache_data_init_impl(tsd, arena, tcache_bin_info); + if (unlikely(err)) { + /* + * Disable the tcache before calling malloc_write to + * avoid recursive allocations through libc hooks. + */ + tsd_tcache_enabled_set(tsd, false); + tsd_slow_update(tsd); + malloc_write(": Failed to allocate tcache data\n"); + if (opt_abort) { + abort(); + } + } + return err; } /* Created manual tcache for tcache.create mallctl. */ @@ -1062,8 +1081,8 @@ tsd_tcache_enabled_data_init(tsd_t *tsd) { if (opt_tcache) { /* Trigger tcache init. */ - tsd_tcache_data_init( - tsd, NULL, tcache_get_default_ncached_max()); + return tsd_tcache_data_init( + tsd, NULL, tcache_get_default_ncached_max()); } return false; @@ -1074,8 +1093,10 @@ tcache_enabled_set(tsd_t *tsd, bool enabled) { bool was_enabled = tsd_tcache_enabled_get(tsd); if (!was_enabled && enabled) { - tsd_tcache_data_init( - tsd, NULL, tcache_get_default_ncached_max()); + if (tsd_tcache_data_init( + tsd, NULL, tcache_get_default_ncached_max())) { + return; + } } else if (was_enabled && !enabled) { tcache_cleanup(tsd); } @@ -1084,13 +1105,14 @@ tcache_enabled_set(tsd_t *tsd, bool enabled) { tsd_slow_update(tsd); } -void +bool thread_tcache_max_set(tsd_t *tsd, size_t tcache_max) { assert(tcache_max <= TCACHE_MAXCLASS_LIMIT); assert(tcache_max == sz_s2u(tcache_max)); tcache_t *tcache = tsd_tcachep_get(tsd); tcache_slow_t *tcache_slow = tcache->tcache_slow; cache_bin_info_t tcache_bin_info[TCACHE_NBINS_MAX] = {{0}}; + bool ret = false; assert(tcache != NULL && tcache_slow != NULL); bool enabled = tcache_available(tsd); @@ -1111,10 +1133,11 @@ thread_tcache_max_set(tsd_t *tsd, size_t tcache_max) { tcache_max_set(tcache_slow, tcache_max); if (enabled) { - tsd_tcache_data_init(tsd, assigned_arena, tcache_bin_info); + ret = tsd_tcache_data_init(tsd, assigned_arena, tcache_bin_info); } assert(tcache_nbins_get(tcache_slow) == sz_size2index(tcache_max) + 1); + return ret; } static bool @@ -1177,9 +1200,7 @@ tcache_bins_ncached_max_write(tsd_t *tsd, char *settings, size_t len) { arena_t *assigned_arena = tcache->tcache_slow->arena; tcache_cleanup(tsd); - tsd_tcache_data_init(tsd, assigned_arena, tcache_bin_info); - - return false; + return tsd_tcache_data_init(tsd, assigned_arena, tcache_bin_info); } static void diff --git a/test/unit/tcache_init.c b/test/unit/tcache_init.c new file mode 100644 index 00000000..11d4b654 --- /dev/null +++ b/test/unit/tcache_init.c @@ -0,0 +1,116 @@ +#include "test/jemalloc_test.h" + +static void * +tcache_stack_alloc_fail(tsdn_t *tsdn, size_t size, size_t alignment) { + return NULL; +} + +TEST_BEGIN(test_tcache_data_init_oom) { + bool orig_opt_abort = opt_abort; + void *(*orig_tcache_stack_alloc)(tsdn_t *, size_t, size_t) = + tcache_stack_alloc; + + opt_abort = false; + tcache_stack_alloc = tcache_stack_alloc_fail; + + /* + * Trigger init through tcache_enabled_set by enabling and + * disabling the tcache. + */ + bool e0, e1; + size_t bool_sz = sizeof(bool); + + /* Disable the tcache. */ + e1 = false; + expect_d_eq(mallctl("thread.tcache.enabled", (void *)&e0, &bool_sz, + (void *)&e1, bool_sz), 0, "Unexpected mallctl failure"); + + /* Try to enable the tcache. Initialization should fail. */ + e1 = true; + expect_d_eq(mallctl("thread.tcache.enabled", (void *)&e0, &bool_sz, + (void *)&e1, bool_sz), 0, "Unexpected mallctl failure"); + + /* The tcache should be disabled. */ + tsd_t *tsd = tsd_fetch(); + expect_false(tsd_tcache_enabled_get(tsd), + "tcache should be disabled after init failure"); + + /* Allocations should go to the arena. */ + void *p = malloc(64); + expect_ptr_not_null(p, "malloc should succeed without tcache"); + free(p); + + /* Restore the original values */ + tcache_stack_alloc = orig_tcache_stack_alloc; + opt_abort = orig_opt_abort; + + /* + * Try to enable the tcache again. This time initialization + * should succeed. + */ + e1 = true; + expect_d_eq(mallctl("thread.tcache.enabled", (void *)&e0, &bool_sz, + (void *)&e1, bool_sz), 0, "Unexpected mallctl failure"); +} +TEST_END + +TEST_BEGIN(test_tcache_reinit_oom) { + bool orig_opt_abort = opt_abort; + void *(*orig_tcache_stack_alloc)(tsdn_t *, size_t, size_t) = + tcache_stack_alloc; + + /* Read current tcache max. */ + size_t old_tcache_max, sz; + sz = sizeof(old_tcache_max); + expect_d_eq(mallctl("thread.tcache.max", (void *)&old_tcache_max, &sz, + NULL, 0), 0, "Unexpected mallctl failure"); + + opt_abort = false; + tcache_stack_alloc = tcache_stack_alloc_fail; + + /* + * Setting thread.tcache.max causes a reinitialization. With + * the thread_stack_alloc override reinitialization should + * fail and disable tcache. + */ + size_t new_tcache_max = 1024; + new_tcache_max = sz_s2u(new_tcache_max); + expect_d_eq(mallctl("thread.tcache.max", NULL, NULL, + (void *)&new_tcache_max, sizeof(new_tcache_max)), 0, + "Unexpected mallctl failure"); + + /* Check that the tcache was disabled. */ + tsd_t *tsd = tsd_fetch(); + expect_false(tsd_tcache_enabled_get(tsd), + "tcache should be disabled after reinit failure"); + + /* Allocations should go to the arena. */ + void *p = malloc(64); + expect_ptr_not_null(p, "malloc should succeed without tcache"); + free(p); + + /* Restore the original values */ + tcache_stack_alloc = orig_tcache_stack_alloc; + opt_abort = orig_opt_abort; + + /* + * Try to enable the tcache again. This time initialization + * should succeed. + */ + bool e0, e1; + size_t bool_sz = sizeof(bool); + e1 = true; + expect_d_eq(mallctl("thread.tcache.enabled", (void *)&e0, &bool_sz, + (void *)&e1, bool_sz), 0, "Unexpected mallctl failure"); + + /* Restore the original tcache max. */ + expect_d_eq(mallctl("thread.tcache.max", NULL, NULL, + (void *)&old_tcache_max, sizeof(old_tcache_max)), 0, + "Unexpected mallctl failure"); +} +TEST_END + +int +main(void) { + return test(test_tcache_data_init_oom, test_tcache_reinit_oom); +}