diff --git a/src/foundation/system_info.c b/src/foundation/system_info.c index ad182f150..84a91cdf1 100644 --- a/src/foundation/system_info.c +++ b/src/foundation/system_info.c @@ -17,8 +17,9 @@ enum { DEFAULT_CORES = 1, MIN_WORKERS = 1, CBM_WORKERS_MAX = 256 }; #include "foundation/mem_core.h" /* cbm_alloc: one accounted allocation path */ #include "foundation/platform.h" #include "foundation/system_info_internal.h" -#include // uint64_t -#include // strtol +#include // the system-info cache is published across worker threads +#include // uint64_t +#include // strtol #include #ifndef _WIN32 @@ -267,23 +268,51 @@ static cbm_system_info_t detect_system_windows(void) { /* ── Public API ──────────────────────────────────────────────────── */ -static int info_cached = 0; +/* The cache is published once, by whichever thread gets there first. + * + * This used to be a plain `if (!info_cached) { cached_info = detect(); }`, which + * was fine while only the main thread asked. It stopped being fine when the + * memory-relief path started calling cbm_mem_system_under_pressure() from the + * extract workers: several threads then raced on the flag AND on the struct, + * and a struct assignment is not atomic, so a reader could see half of one. + * TSan caught it on all three platforms (system_info.c:284 against :274, from + * extract_worker). + * + * Three states rather than a flag, so the winner of the compare-exchange is the + * only writer of cached_info: a loser returns the copy it detected itself, + * which is the same answer, and a later reader sees the published struct + * through the acquire/release pair. No mutex, so nothing has to be initialised + * before first use, and no reader can observe a half-written struct. */ +enum { INFO_EMPTY = 0, INFO_CLAIMED = 1, INFO_READY = 2 }; +static _Atomic int info_state = INFO_EMPTY; static cbm_system_info_t cached_info; -cbm_system_info_t cbm_system_info(void) { - if (!info_cached) { +static cbm_system_info_t detect_system_now(void) { #ifdef _WIN32 - cached_info = detect_system_windows(); + return detect_system_windows(); #elif defined(__APPLE__) - cached_info = detect_system_macos(); + return detect_system_macos(); #elif defined(__NetBSD__) || defined(__FreeBSD__) || defined(__OpenBSD__) - cached_info = detect_system_bsd(); + return detect_system_bsd(); #else - cached_info = detect_system_linux(); + return detect_system_linux(); #endif - info_cached = SKIP_ONE; +} + +cbm_system_info_t cbm_system_info(void) { + if (atomic_load_explicit(&info_state, memory_order_acquire) == INFO_READY) { + return cached_info; } - return cached_info; + /* Detection only reads the OS (sysconf/sysctl/GetSystemInfo), so racing + * threads doing it twice during startup costs a little and changes nothing. */ + cbm_system_info_t local = detect_system_now(); + int expected = INFO_EMPTY; + if (atomic_compare_exchange_strong_explicit(&info_state, &expected, INFO_CLAIMED, + memory_order_acq_rel, memory_order_relaxed)) { + cached_info = local; + atomic_store_explicit(&info_state, INFO_READY, memory_order_release); + } + return local; } int cbm_default_worker_count(bool initial) { diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index aba55fb20..dbf459814 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -832,8 +832,16 @@ static void pp_spill_enter(extract_ctx_t *ec, const char *reason) { } static bool pp_spill_active(const extract_ctx_t *ec) { - return ec->pctx && ec->pctx->spill && - atomic_load_explicit(&ec->pctx->spill_mode, memory_order_acquire) != 0; + /* The atomic gates the pointer, not the other way round. pp_spill_enter + * assigns ec->pctx->spill under spill_mu and only THEN release-stores + * spill_mode, so a reader that has acquired a non-zero spill_mode is + * guaranteed to see the finished pointer. Testing the pointer first read it + * with no synchronisation at all while another worker was publishing it — + * a genuine data race on an 8-byte write, which TSan caught at + * pass_parallel.c:817 against this line. Short-circuit order is load + * bearing here; do not reorder these terms. */ + return ec->pctx && atomic_load_explicit(&ec->pctx->spill_mode, memory_order_acquire) != 0 && + ec->pctx->spill; } CBMFileResult *cbm_pipeline_result_acquire(const cbm_pipeline_ctx_t *ctx, CBMFileResult **cache,