From 92e7e6a91374e9f89c74ca45e01f4e525e331837 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 18 Sep 2026 23:26:05 +0200 Subject: [PATCH] fix: keep large thread-locals off the static TLS block Indexing broke on x86-64 Linux: the index worker refused to start and SIGKILLed its own process group, so every venue reported nothing but "index worker ended with killed (exit=-1, signal=9)". glibc places a thread's static TLS block inside the stack allocation it makes for that thread. This branch's new thread-local caches -- a 512-slot field cache (~24 KB), per-depth tree cursors (~4 KB) and two parked CBMArenas (~8 KB; an arena carries blocks[256] + block_sizes[256]) -- took static TLS from 47,905 to 93,513 bytes. The 64 KB parent-death watchdog thread then could not be created at all, and a worker without process-tree containment correctly refuses to index. Four layers, because there were four things wrong: - The big thread-locals move to the heap behind a thread-local POINTER, allocated once per thread and reused, each with a correct fallback if allocation fails (the field cache and cursor pool are caches; a miss is slower, never wrong). Static TLS is back to 52,233 bytes. - PARENT_WATCHDOG_STACK_SIZE 64 KB -> 256 KB. "The watchdog only polls, a tiny stack suffices" was true when written and false once TLS reached ~50 KB, because the TLS block comes out of that same allocation. - cbm_thread_create retries with the default stack on EINVAL. A stack size is a hint about what a thread needs; the platform refusing the hint is not a reason to fail to create the thread. - tests/test_thread_stack_tls_contract.sh (Step 5a of scripts/test.sh) keeps static TLS a quarter clear of the smallest requested stack, reading that smallest size from the sources so a new small stack tightens the gate automatically. Two diagnostics that made this findable at all, and that stay: smoke-test.sh prints the worker log its failure message names (CI deletes that sandbox with the job, so the one artifact naming the cause was unreadable), and worker_containment_unavailable() now reports WHICH of its conditions failed, with pid/pgrp/ppid. Verified in the amd64 container, the venue nothing on the local ladder covers: baseline main indexes 9039 nodes/15067 edges, this branch before the fix refuses to start the worker, and after it indexes 9039/15067. Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 144 +++++++++++++++++------- scripts/fuzz.sh | 0 scripts/memwaste.sh | 0 scripts/smoke-test.sh | 24 ++++ scripts/test.sh | 9 ++ src/foundation/compat_thread.c | 19 ++++ src/main.c | 45 ++++++-- src/pipeline/pass_lsp_cross.c | 48 +++++--- tests/test_thread_stack_tls_contract.sh | 82 ++++++++++++++ 9 files changed, 304 insertions(+), 67 deletions(-) mode change 100644 => 100755 scripts/fuzz.sh mode change 100644 => 100755 scripts/memwaste.sh create mode 100755 tests/test_thread_stack_tls_contract.sh diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index e88354680..b34540bbc 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -1,10 +1,10 @@ /* Full declaration set for the same CBMArena, and it must precede cbm.h: * internal/cbm/arena.h declares a subset and the two share the CBM_ARENA_H * guard, so whichever is included first is the one this file sees. */ -#include "foundation/arena.h" // cbm_arena_init_sized -#include "foundation/mem_core.h" // class accounting for the bound allocators +#include "foundation/arena.h" // cbm_arena_init_sized +#include "foundation/mem_core.h" // class accounting for the bound allocators #include "foundation/mem_events.h" // waste sanitizer: the bound allocators bypass every observer -#include "foundation/log.h" // cbm_log_warn -- extract.lsp.skipped +#include "foundation/log.h" // cbm_log_warn -- extract.lsp.skipped #include "cbm.h" #include "arena.h" // CBMArena, cbm_arena_init/alloc/strdup/destroy #include "helpers.h" @@ -349,19 +349,19 @@ static mi_heap_t *sqlite_heap(void) { } #if defined(CBM_MEMWASTE) && CBM_MEMWASTE -#define BOUND_ALLOC(block, n, flags) \ - do { \ - if ((block) && cbm_memev_enabled()) { \ - cbm_memev_alloc_ex((block), (n), mi_usable_size(block), \ - __builtin_return_address(0), (unsigned)(flags)); \ - } \ +#define BOUND_ALLOC(block, n, flags) \ + do { \ + if ((block) && cbm_memev_enabled()) { \ + cbm_memev_alloc_ex((block), (n), mi_usable_size(block), __builtin_return_address(0), \ + (unsigned)(flags)); \ + } \ } while (0) -#define BOUND_REALLOC(old_block, grown, n) \ - do { \ - if ((grown) && cbm_memev_enabled()) { \ - cbm_memev_realloc((old_block), (grown), (n), mi_usable_size(grown), \ - __builtin_return_address(0)); \ - } \ +#define BOUND_REALLOC(old_block, grown, n) \ + do { \ + if ((grown) && cbm_memev_enabled()) { \ + cbm_memev_realloc((old_block), (grown), (n), mi_usable_size(grown), \ + __builtin_return_address(0)); \ + } \ } while (0) #define BOUND_FREE(block) cbm_memev_free(block) #else @@ -518,19 +518,41 @@ typedef struct { TSFieldId id; char name[FIELD_NAME_CACHED_MAX + 1]; } field_id_slot_t; -static CBM_TLS field_id_slot_t tl_field_ids[FIELD_CACHE_SLOTS]; +/* On the HEAP behind a thread-local pointer, not a thread-local array: glibc + * takes the static TLS block out of each thread's stack allocation, so a 24 KB + * array here is 24 KB every thread must fit — and the threads that ask for a + * small stack (the 64 KB parent-death watchdog) then fail to start at all, + * with EINVAL surfacing nowhere near the growth that caused it. See + * cbm_thread_create's fallback for the other half of that lesson. */ +static CBM_TLS field_id_slot_t *tl_field_ids; + +static field_id_slot_t *field_cache(void) { + if (!tl_field_ids) { + tl_field_ids = cbm_calloc(CBM_MEM_CLASS_OTHER, FIELD_CACHE_SLOTS * sizeof(field_id_slot_t)); + } + return tl_field_ids; +} + +static void cbm_ts_field_cache_release_thread(void) { + cbm_free(CBM_MEM_CLASS_OTHER, tl_field_ids); + tl_field_ids = NULL; +} TSNode cbm_ts_child_by_field_name(TSNode node, const char *name, uint32_t name_length) { if (!name || name_length == 0 || name_length > FIELD_NAME_CACHED_MAX || ts_node_is_null(node)) { return (ts_node_child_by_field_name)(node, name, name_length); /* the real one */ } + field_id_slot_t *cache = field_cache(); + if (!cache) { + return (ts_node_child_by_field_name)(node, name, name_length); /* uncached, still correct */ + } const TSLanguage *lang = ts_node_language(node); uint64_t h = 0xcbf29ce484222325ULL ^ (uint64_t)(uintptr_t)lang; for (uint32_t i = 0; i < name_length; i++) { h ^= (uint8_t)name[i]; h *= 0x100000001b3ULL; } - field_id_slot_t *slot = &tl_field_ids[h & (FIELD_CACHE_SLOTS - 1)]; + field_id_slot_t *slot = &cache[h & (FIELD_CACHE_SLOTS - 1)]; if (slot->lang != lang || slot->len != name_length || memcmp(slot->name, name, name_length) != 0) { /* The content is compared, never just the pointer: callers also pass @@ -561,22 +583,33 @@ TSTreeCursor *cbm_thread_cursor(TSNode node) { * create and delete a cursor at EVERY node -- 49.8 M cursor stacks on the Go * corpus (waste sanitizer, 2026-09-17). Deeper than the pool: private cursors. */ enum { CURSOR_POOL_DEPTH = 128 }; -static CBM_TLS TSTreeCursor tl_cursor_pool[CURSOR_POOL_DEPTH]; -static CBM_TLS uint8_t tl_cursor_pool_state[CURSOR_POOL_DEPTH]; /* bit0 live, bit1 busy */ +/* Heap-backed for the same reason as the field cache: 4 KB of cursors plus + * their state in static TLS is 4 KB charged to every thread in the image, and + * it is the kind of growth that makes a small-stack thread refuse to start. */ +typedef struct { + TSTreeCursor cursors[CURSOR_POOL_DEPTH]; + uint8_t state[CURSOR_POOL_DEPTH]; /* bit0 live, bit1 busy */ +} cursor_pool_t; +static CBM_TLS cursor_pool_t *tl_cursor_pool; enum { CURSOR_LIVE = 1, CURSOR_BUSY = 2 }; TSTreeCursor *cbm_cursor_acquire(cbm_cursor_lease_t *lease, int depth, TSNode node) { - if (depth >= 0 && depth < CURSOR_POOL_DEPTH && !(tl_cursor_pool_state[depth] & CURSOR_BUSY)) { - if (tl_cursor_pool_state[depth] & CURSOR_LIVE) { - ts_tree_cursor_reset(&tl_cursor_pool[depth], node); + if (!tl_cursor_pool) { + tl_cursor_pool = cbm_calloc(CBM_MEM_CLASS_OTHER, sizeof(cursor_pool_t)); + } + cursor_pool_t *pool = tl_cursor_pool; + if (pool && depth >= 0 && depth < CURSOR_POOL_DEPTH && !(pool->state[depth] & CURSOR_BUSY)) { + if (pool->state[depth] & CURSOR_LIVE) { + ts_tree_cursor_reset(&pool->cursors[depth], node); } else { - tl_cursor_pool[depth] = ts_tree_cursor_new(node); + pool->cursors[depth] = ts_tree_cursor_new(node); } - tl_cursor_pool_state[depth] = CURSOR_LIVE | CURSOR_BUSY; + pool->state[depth] = CURSOR_LIVE | CURSOR_BUSY; lease->slot = depth; - lease->cursor = &tl_cursor_pool[depth]; + lease->cursor = &pool->cursors[depth]; return lease->cursor; } + /* No pool (or the slot is taken): a private cursor is always correct. */ lease->private_cursor = ts_tree_cursor_new(node); lease->slot = -1; lease->cursor = &lease->private_cursor; @@ -586,8 +619,8 @@ TSTreeCursor *cbm_cursor_acquire(cbm_cursor_lease_t *lease, int depth, TSNode no void cbm_cursor_release(cbm_cursor_lease_t *lease) { if (lease->slot < 0) { ts_tree_cursor_delete(&lease->private_cursor); - } else { - tl_cursor_pool_state[lease->slot] &= (uint8_t)~CURSOR_BUSY; + } else if (tl_cursor_pool) { + tl_cursor_pool->state[lease->slot] &= (uint8_t)~CURSOR_BUSY; } lease->cursor = NULL; } @@ -603,13 +636,25 @@ void cbm_destroy_thread_parser(void) { ts_tree_cursor_delete(&tl_cursor); tl_cursor_live = false; } - for (int d = 0; d < CURSOR_POOL_DEPTH; d++) { - /* a busy slot belongs to a walk still on this stack: leave it */ - if (tl_cursor_pool_state[d] == CURSOR_LIVE) { - ts_tree_cursor_delete(&tl_cursor_pool[d]); - tl_cursor_pool_state[d] = 0; + if (tl_cursor_pool) { + bool busy = false; + for (int d = 0; d < CURSOR_POOL_DEPTH; d++) { + /* a busy slot belongs to a walk still on this stack: leave it */ + if (tl_cursor_pool->state[d] == CURSOR_LIVE) { + ts_tree_cursor_delete(&tl_cursor_pool->cursors[d]); + tl_cursor_pool->state[d] = 0; + } else if (tl_cursor_pool->state[d] & CURSOR_BUSY) { + busy = true; + } + } + /* Outstanding leases point INTO this block, so it is only released once + * no walk still holds a slot. */ + if (!busy) { + cbm_free(CBM_MEM_CLASS_OTHER, tl_cursor_pool); + tl_cursor_pool = NULL; } } + cbm_ts_field_cache_release_thread(); } void cbm_shutdown(void) { @@ -2408,7 +2453,13 @@ static CBM_TLS bool tl_work_arena_live = false; * worth holding). Before this every file created and destroyed a * 512 KB block -- 28,145 of them on the Go corpus, 14 GB allocated, 99.9 % * never written and 7,136 never touched (waste sanitizer, 2026-09-17). */ -static CBM_TLS CBMArena tl_scratch_arena; +/* The parked arena lives on the HEAP, reached through a thread-local pointer, + * not inline in thread-local storage: a CBMArena is ~4 KB (256 block pointers + * and 256 sizes), and static TLS is charged to every thread AND taken out of + * each thread's stack allocation — enough of it and a small-stack thread stops + * being creatable at all (PR #2233). The holder is allocated once per thread + * and reused, so parking still costs no allocation per file. */ +static CBM_TLS CBMArena *tl_scratch_slot; static CBM_TLS bool tl_scratch_live = false; static CBM_TLS bool tl_scratch_keep = false; @@ -2427,10 +2478,12 @@ void cbm_work_arena_release(void) { cbm_arena_destroy(&tl_work_arena); tl_work_arena_live = false; } - if (tl_scratch_live) { - cbm_arena_destroy(&tl_scratch_arena); + if (tl_scratch_live && tl_scratch_slot) { + cbm_arena_destroy(tl_scratch_slot); tl_scratch_live = false; } + cbm_free(CBM_MEM_CLASS_OTHER, tl_scratch_slot); + tl_scratch_slot = NULL; cbm_result_compact_release_thread(); tl_scratch_keep = false; } @@ -2472,16 +2525,16 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua const char **include_paths, const CBMMacroTable *macro_table, const CBMReturnTypeTable *return_type_table) { CBMArena scratch; - if (tl_scratch_live) { - scratch = tl_scratch_arena; + if (tl_scratch_live && tl_scratch_slot) { + scratch = *tl_scratch_slot; tl_scratch_live = false; cbm_arena_rewind(&scratch); } else { cbm_arena_init_lazy(&scratch, CBM_EXTRACT_SCRATCH_BLOCK); } - CBMFileResult *result = - extract_file_ex_body(source, source_len, language, project, rel_path, timeout_micros, - extra_defines, include_paths, macro_table, return_type_table, &scratch); + CBMFileResult *result = extract_file_ex_body(source, source_len, language, project, rel_path, + timeout_micros, extra_defines, include_paths, + macro_table, return_type_table, &scratch); /* !tl_scratch_live: a nested extraction (an embedded language inside this * file) may already have parked its own; never overwrite it. */ /* Kept up to CBM_EXTRACT_SCRATCH_KEEP_BYTES, grown blocks included: the @@ -2490,8 +2543,15 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua * arena turned those files into fresh 512 KB blocks for the next file. */ if (tl_scratch_keep && !tl_scratch_live && scratch.nblocks > 0 && cbm_arena_capacity(&scratch) <= (size_t)CBM_EXTRACT_SCRATCH_KEEP_BYTES) { - tl_scratch_arena = scratch; - tl_scratch_live = true; + if (!tl_scratch_slot) { + tl_scratch_slot = cbm_alloc(CBM_MEM_CLASS_OTHER, sizeof(CBMArena)); + } + if (tl_scratch_slot) { + *tl_scratch_slot = scratch; + tl_scratch_live = true; + } else { + cbm_arena_destroy(&scratch); /* no holder: keep nothing, stay correct */ + } } else { cbm_arena_destroy(&scratch); } diff --git a/scripts/fuzz.sh b/scripts/fuzz.sh old mode 100644 new mode 100755 diff --git a/scripts/memwaste.sh b/scripts/memwaste.sh old mode 100644 new mode 100755 diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index ca27edcc6..3371fc424 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -174,6 +174,29 @@ cli() { return "$rc" } +# A worker failure says "inspect log: " — and in CI that path dies with the +# job's sandbox, so the one artifact naming the cause is the one nobody can open. +# A worker killed by a signal writes no summary of its own either, which is +# exactly the case that most needs the log (PR #2233: "index worker ended with +# killed (exit=-1, signal=9)" on ubuntu-latest, unreproducible on every local +# venue). Print it while it still exists. +smoke_dump_worker_log() { + local log + log=$(sed -n 's/.*inspect log: \([^ ]*\).*/\1/p' "$CLI_STDERR" 2>/dev/null | tail -1) + if [ -z "$log" ] || [ ! -f "$log" ]; then + # The path is only printed for some failures; fall back to the newest log + # the run produced. + log=$(ls -t "${CBM_CACHE_DIR:-$HOME/.cache/codebase-memory-mcp}"/logs/.worker-log-* 2>/dev/null | head -1) + fi + if [ -n "$log" ] && [ -f "$log" ]; then + echo "--- worker log: $log ---" + tail -80 "$log" + echo "--- end worker log ---" + else + echo "--- no worker log found (cache ${CBM_CACHE_DIR:-unset}) ---" + fi +} + echo "=== Phase 1: version ===" VERSION_STATUS=0 OUTPUT=$("$BINARY" --version 2>&1) || VERSION_STATUS=$? @@ -334,6 +357,7 @@ GENEOF if ! RESULT=$(cli index_repository --repo-path "$TMPDIR"); then echo "FAIL: index_repository (flag form) exited non-zero" cat "$CLI_STDERR" + smoke_dump_worker_log exit 1 fi echo "$RESULT" diff --git a/scripts/test.sh b/scripts/test.sh index ecfc26998..f9c832a63 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -318,6 +318,15 @@ make -j"$NPROC" -f Makefile.cbm cbm TEST_SEAMS=1 ${MAKE_ARGS[@]+"${MAKE_ARGS[@]} WATCHDOG_BINARY="$ROOT/$BUILD_DIR/codebase-memory-mcp" CBM_TEST_BINARY="$WATCHDOG_BINARY" bash "$ROOT/tests/test_parent_watchdog.sh" +# Step 5a: that watchdog is also the SMALLEST-stack thread in the image, which +# makes it the first casualty when static TLS grows — glibc takes the TLS block +# out of each thread's own stack allocation. Checked here, against the binary +# Step 5 just built, because the failure it prevents surfaces nowhere near its +# cause (PR #2233: a thread-local cache in an extraction file stopped the index +# worker from starting, on x86-64 only). +echo "=== Step 5a: static-TLS budget against the smallest thread stack ===" +bash "$ROOT/tests/test_thread_stack_tls_contract.sh" "$WATCHDOG_BINARY" + # Step 5b: worker-mode parent-death watchdog (#845). A supervised index worker # (`cli --index-worker …`) whose supervisor dies must self-exit instead of # indexing on as an orphan. Reuses the prod binary built in Step 5. diff --git a/src/foundation/compat_thread.c b/src/foundation/compat_thread.c index e10cec8f1..94af9f812 100644 --- a/src/foundation/compat_thread.c +++ b/src/foundation/compat_thread.c @@ -13,6 +13,7 @@ #include /* mi_thread_done at thread exit */ +#include /* EINVAL — a refused stack-size hint, see cbm_thread_create */ #include #include @@ -178,6 +179,24 @@ int cbm_thread_create(cbm_thread_t *t, size_t stack_size, void *(*fn)(void *), v pthread_attr_setstacksize(&attr, stack_size); int rc = pthread_create(&t->handle, &attr, fn, arg); pthread_attr_destroy(&attr); + if (rc == EINVAL) { + /* glibc carves the static TLS block out of the thread's own stack + * allocation, so a small REQUESTED stack stops being legal the moment + * the image's TLS grows — no warning where the growth happens, only + * EINVAL here, from then on. That is exactly how the 64 KB + * parent-death watchdog stopped starting once this image's TLS passed + * it (PR #2233): the worker then refused to index without containment + * and SIGKILLed its own group, so every venue reported nothing but + * "killed (signal 9)". + * A stack size is a hint about how much this thread needs; the platform + * refusing the hint is not a reason to fail to create the thread. Fall + * back to the default stack, which always has room for the TLS block. */ + pthread_attr_t fallback; + pthread_attr_init(&fallback); + pthread_attr_setstacksize(&fallback, cbm_thread_default_stack_size()); + rc = pthread_create(&t->handle, &fallback, fn, arg); + pthread_attr_destroy(&fallback); + } return rc; } diff --git a/src/main.c b/src/main.c index 52113bdcc..9b91f205c 100644 --- a/src/main.c +++ b/src/main.c @@ -60,7 +60,15 @@ enum { MAIN_HOOK_NOTICE_INTERVAL_SECONDS = 900, MAIN_CLOSE_TIMEOUT_MS = 5000, MAIN_COORDINATION_CLEANUP_MS = 500, - PARENT_WATCHDOG_STACK_SIZE = 64 * CBM_SZ_1K, /* watchdog only polls — tiny stack suffices */ + /* The watchdog only polls, so the STACK it needs is tiny — but on glibc the + * thread's static TLS block is carved out of this same allocation, and this + * image's TLS is ~50 KB. At the old 64 KB the request was almost entirely + * TLS, and when TLS grew past it the thread stopped being creatable at all: + * the worker then refused to index without containment and SIGKILLed its own + * group, reported everywhere as a bare "killed (signal 9)" (PR #2233). + * Sized to hold the TLS block AND a real stack; tests/ + * test_thread_stack_tls_contract.sh keeps the two from converging again. */ + PARENT_WATCHDOG_STACK_SIZE = 256 * CBM_SZ_1K, }; #define SLEN(s) (sizeof(s) - 1) #include "foundation/log.h" @@ -452,10 +460,22 @@ static bool worker_prepare_process_group(void) { * identically — write() and _exit() rather than fprintf/exit, because this runs * after fork-sensitive setup and must not touch stdio locks or atexit handlers. */ -static void worker_containment_unavailable(void) { - static const char message[] = - "CBM index worker could not start: process-tree containment unavailable\n"; - (void)write(STDERR_FILENO, message, sizeof(message) - 1); +static void worker_containment_unavailable(const char *why) { + /* WHICH of the containment conditions failed, and the pids behind it. The + * bare sentence cost a full CI cycle and two emulated builds to attribute + * (PR #2233): every venue reported the same line, and the supervisor only + * ever sees `killed (signal 9)` because the refusal SIGKILLs its own group. + * snprintf into a local buffer touches no stdio lock and no atexit handler, + * so the fork-sensitivity contract above still holds. */ + char message[256]; + int n = snprintf(message, sizeof(message), + "CBM index worker could not start: process-tree containment unavailable " + "(%s; pid=%ld pgrp=%ld ppid=%ld)\n", + why ? why : "unspecified", (long)getpid(), (long)getpgrp(), (long)getppid()); + if (n > 0) { + (void)write(STDERR_FILENO, message, + (size_t)n < sizeof(message) ? (size_t)n : sizeof(message) - 1); + } (void)kill(-getpid(), SIGKILL); _exit(EXIT_FAILURE); } @@ -3081,17 +3101,22 @@ int main(int argc, char **argv) { * unconditional in the source, so with the seam compiled out cppcheck * correctly reported `!probe()` as always false. Guarding the STEP, not * stubbing the function, means release builds simply do not have it. */ - if (!worker_prepare_process_group() || process_initial_ppid <= 1 || - getppid() != process_initial_ppid) { - worker_containment_unavailable(); + if (!worker_prepare_process_group()) { + worker_containment_unavailable("own process group not established"); + } + if (process_initial_ppid <= 1) { + worker_containment_unavailable("no supervising parent at startup"); + } + if (getppid() != process_initial_ppid) { + worker_containment_unavailable("parent changed since startup"); } #ifdef CBM_ENABLE_TEST_SEAMS if (!worker_start_watchdog_test_descendant()) { - worker_containment_unavailable(); + worker_containment_unavailable("watchdog test descendant refused"); } #endif if (!worker_start_parent_watchdog(process_initial_ppid)) { - worker_containment_unavailable(); + worker_containment_unavailable("parent-death watchdog would not start"); } #endif cbm_index_supervisor_mark_host(); diff --git a/src/pipeline/pass_lsp_cross.c b/src/pipeline/pass_lsp_cross.c index 69b0660a3..3b74cc1eb 100644 --- a/src/pipeline/pass_lsp_cross.c +++ b/src/pipeline/pass_lsp_cross.c @@ -968,13 +968,23 @@ bool cbm_pxc_has_cross_lsp(CBMLanguage lang) { enum { PXC_SCRATCH_DISPATCH = 0, PXC_SCRATCH_KEYS = 1, PXC_SCRATCH_COUNT = 2 }; enum { PXC_KEEP_BYTES = 4 * 1024 * 1024 }; static CBM_TLS bool tl_pxc_keep; -static CBM_TLS CBMArena tl_pxc_arena[PXC_SCRATCH_COUNT]; -static CBM_TLS bool tl_pxc_live[PXC_SCRATCH_COUNT]; +/* The parked arenas live on the HEAP behind one thread-local pointer. Inline in + * thread-local storage they were ~8 KB of static TLS charged to every thread in + * the image, and static TLS comes out of each thread's own stack allocation, so + * past a certain size a small-stack thread cannot be created at all — which is + * precisely how this branch broke the 64 KB parent-death watchdog and, with it, + * indexing on x86-64 Linux (PR #2233). The holder is allocated once per + * keeping thread, so parking still costs no allocation per file. */ +typedef struct { + CBMArena arena[PXC_SCRATCH_COUNT]; + bool live[PXC_SCRATCH_COUNT]; +} pxc_scratch_t; +static CBM_TLS pxc_scratch_t *tl_pxc; static void pxc_scratch_take(int slot, CBMArena *into) { - if (tl_pxc_live[slot]) { - *into = tl_pxc_arena[slot]; - tl_pxc_live[slot] = false; + if (tl_pxc && tl_pxc->live[slot]) { + *into = tl_pxc->arena[slot]; + tl_pxc->live[slot] = false; cbm_arena_rewind(into); return; } @@ -982,12 +992,16 @@ static void pxc_scratch_take(int slot, CBMArena *into) { } static void pxc_scratch_give(int slot, CBMArena *from) { - if (tl_pxc_keep && !tl_pxc_live[slot] && from->nblocks > 0 && - cbm_arena_capacity(from) <= (size_t)PXC_KEEP_BYTES) { - tl_pxc_arena[slot] = *from; - tl_pxc_live[slot] = true; - memset(from, 0, sizeof(*from)); - return; + if (tl_pxc_keep && from->nblocks > 0 && cbm_arena_capacity(from) <= (size_t)PXC_KEEP_BYTES) { + if (!tl_pxc) { + tl_pxc = cbm_calloc(CBM_MEM_CLASS_OTHER, sizeof(pxc_scratch_t)); + } + if (tl_pxc && !tl_pxc->live[slot]) { + tl_pxc->arena[slot] = *from; + tl_pxc->live[slot] = true; + memset(from, 0, sizeof(*from)); + return; + } } cbm_arena_destroy(from); } @@ -997,11 +1011,15 @@ void cbm_pxc_thread_scratch_begin(void) { } void cbm_pxc_thread_scratch_end(void) { - for (int slot = 0; slot < PXC_SCRATCH_COUNT; slot++) { - if (tl_pxc_live[slot]) { - cbm_arena_destroy(&tl_pxc_arena[slot]); - tl_pxc_live[slot] = false; + if (tl_pxc) { + for (int slot = 0; slot < PXC_SCRATCH_COUNT; slot++) { + if (tl_pxc->live[slot]) { + cbm_arena_destroy(&tl_pxc->arena[slot]); + tl_pxc->live[slot] = false; + } } + cbm_free(CBM_MEM_CLASS_OTHER, tl_pxc); + tl_pxc = NULL; } tl_pxc_keep = false; } diff --git a/tests/test_thread_stack_tls_contract.sh b/tests/test_thread_stack_tls_contract.sh new file mode 100755 index 000000000..9dd516db4 --- /dev/null +++ b/tests/test_thread_stack_tls_contract.sh @@ -0,0 +1,82 @@ +#!/usr/bin/env bash +# Static TLS must fit inside the SMALLEST stack any thread asks for. +# +# Why this exists: glibc places a thread's static TLS block inside the stack +# allocation it makes for that thread. So growing a thread-local — anywhere in +# the image, in a file that has nothing to do with threads — silently shrinks +# the usable stack of every thread, and once the block no longer fits the +# smallest requested stack, pthread_create returns EINVAL for that thread and +# ONLY that thread. +# +# What that looked like in practice (PR #2233): ~45 KB of new thread-local +# caches (a 512-slot field cache, per-depth tree cursors and two parked arenas — +# a CBMArena is ~4 KB because it carries blocks[256] + block_sizes[256]) pushed +# static TLS from 47.9 KB to 93.5 KB. The 64 KB parent-death watchdog thread +# then could not start, the index worker correctly refused to run without +# process-tree containment and SIGKILLed its own group, and every venue reported +# the same uninformative "index worker ended with killed (signal 9)". Local +# macOS and arm64 Linux legs were fully green throughout: nothing on the ladder +# built x86-64, and the arm64 TLS layout stayed just under the line. +# +# The gate: PT_TLS MemSiz of the built product binary must stay a safe margin +# below the smallest stack size the code requests for a thread. That is an ELF +# property, so the check is Linux-only; the macOS host leg skips with a reason +# (Mach-O has no PT_TLS, and its TLS is not carved from the thread stack). +# +# Usage: tests/test_thread_stack_tls_contract.sh [binary] + +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +BINARY="${1:-${CBM_TEST_BINARY:-}}" + +if [ -z "$BINARY" ] || [ ! -f "$BINARY" ]; then + echo "FAIL: no product binary to inspect (pass one, or set CBM_TEST_BINARY)" >&2 + exit 1 +fi + +case "$(uname -s)" in +Linux) ;; +*) + echo "SKIP: static-TLS budget is an ELF/glibc property (Mach-O keeps TLS off" \ + "the thread stack) — the Linux legs own this contract" + exit 0 + ;; +esac + +if ! command -v readelf >/dev/null 2>&1; then + echo "SKIP: readelf not available to read PT_TLS" + exit 0 +fi + +# The smallest stack the code asks for, read from the source rather than +# duplicated here: a future smaller stack must tighten this gate automatically. +smallest_kb=$(grep -ho '[A-Z_]*STACK_SIZE = [0-9]* \* CBM_SZ_1K' "$ROOT"/src/*.c "$ROOT"/src/*/*.c | + sed 's/.* = \([0-9]*\) \* CBM_SZ_1K/\1/' | sort -n | head -1) +if [ -z "$smallest_kb" ]; then + echo "FAIL: could not find any _STACK_SIZE in the sources — the gate lost its input" >&2 + exit 1 +fi +smallest=$((smallest_kb * 1024)) + +tls_hex=$(readelf -lW "$BINARY" | awk '$1 == "TLS" { print $6 }' | head -1) +if [ -z "$tls_hex" ]; then + echo "SKIP: binary has no PT_TLS segment (nothing to bound)" + exit 0 +fi +tls=$((tls_hex)) + +# The thread descriptor and guard share that allocation too, so the budget is +# not the whole stack. A quarter of it is reserve enough to notice growth long +# before a thread stops being creatable. +budget=$((smallest - smallest / 4)) + +echo "static TLS ${tls} bytes; smallest requested thread stack ${smallest} bytes; budget ${budget}" +if [ "$tls" -ge "$budget" ]; then + echo "FAIL: static TLS (${tls} B) is within a quarter of the smallest thread stack" \ + "(${smallest} B). glibc takes the TLS block out of that stack, so this is how" \ + "a thread silently stops being creatable — move large thread-locals to the" \ + "heap behind a thread-local POINTER (see cbm.c's field cache/cursor pool)." >&2 + exit 1 +fi +echo "Thread-stack TLS contract passed"