From 7bfa11e83808398b58c18e9bb69092a02cb873ee Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 19 May 2026 01:57:06 +0200 Subject: [PATCH] =?UTF-8?q?fix(rc):=20GREEN=20=E2=80=94=20atomic=20global?= =?UTF-8?q?=20g=5Frc=5F*=20stats=20fallback=20counters=20(swarm-safe)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bugfix-rc-global-stats-race, GREEN stage. RED is the separate audit-trail commit 427b687. g_rc_alloc_count / g_rc_free_count (runtime/rc.c:90-91) were plain static uint64_t; the __ail_tls_ctx == NULL fallback at rc.c:161 (alloc) and rc.c:212 (dec-to-zero) did a non-atomic ++. A multi-threaded host using the global fallback path (no ailang_ctx_new) raced the read-modify-write and lost increments, so the AILANG_RC_STATS atexit Σ under-counted non-deterministically. Not a memory bug — Ctx:!Send keeps every box on one thread; only the global statistics Σ was wrong (the M5 iter-2 symbol-fan leak-proof finding, b724cd1; user-approved Option A bounce-back resolution). Fix (runtime/rc.c only, 37+/14-): the two globals are now _Atomic uint64_t; atomic_fetch_add_explicit(.., memory_order_relaxed) at the two null-ctx fallback ++ sites; atomic_load_explicit(.., relaxed) snapshot in ailang_rc_stats_atexit (its sole reader). Relaxed is correct and sufficient — pure statistics, no happens-before obligation, atexit reader runs after all worker threads joined. Per-ctx counters and the per-object refcount header stay non-atomic BY DESIGN (single-thread-per-ctx; boxes never cross threads) — out of scope, untouched. Frozen value layout / ABI offsets / host-free rule untouched. The genuinely-still- single-threaded drop-worklist doc note was correctly left unchanged (false correction refused); the two stale atomicity doc blocks corrected for honesty. Boss-verified independently: RED -> GREEN deterministically 3/3 (jitter gone, allocs==frees==16_000_000); full cargo test -p ail green (every binary 0 failed — per-ctx tsan harnesses + embed e2e unaffected; rc.c links into every ail binary, strong regression gate); scope = runtime/rc.c + journal + stats only; RED files unchanged. Unblocks resuming the M5 leak-proof. Includes the per-iter journal, stats, and the INDEX.md line. --- ...5-19-iter-bugfix-rc-global-stats-race.json | 12 +++ ...-05-19-iter-bugfix-rc-global-stats-race.md | 73 +++++++++++++++++++ docs/journals/INDEX.md | 1 + runtime/rc.c | 51 +++++++++---- 4 files changed, 123 insertions(+), 14 deletions(-) create mode 100644 bench/orchestrator-stats/2026-05-19-iter-bugfix-rc-global-stats-race.json create mode 100644 docs/journals/2026-05-19-iter-bugfix-rc-global-stats-race.md diff --git a/bench/orchestrator-stats/2026-05-19-iter-bugfix-rc-global-stats-race.json b/bench/orchestrator-stats/2026-05-19-iter-bugfix-rc-global-stats-race.json new file mode 100644 index 0000000..7ea5a98 --- /dev/null +++ b/bench/orchestrator-stats/2026-05-19-iter-bugfix-rc-global-stats-race.json @@ -0,0 +1,12 @@ +{ + "iter_id": "bugfix-rc-global-stats-race", + "date": "2026-05-19", + "mode": "mini", + "outcome": "DONE", + "tasks_total": 1, + "tasks_completed": 1, + "reloops_per_task": { "1": 0 }, + "review_loops_spec": 0, + "review_loops_quality": 0, + "blocked_reason": null +} diff --git a/docs/journals/2026-05-19-iter-bugfix-rc-global-stats-race.md b/docs/journals/2026-05-19-iter-bugfix-rc-global-stats-race.md new file mode 100644 index 0000000..3e901a3 --- /dev/null +++ b/docs/journals/2026-05-19-iter-bugfix-rc-global-stats-race.md @@ -0,0 +1,73 @@ +# iter bugfix-rc-global-stats-race — atomic global RC-stats fallback counters + +**Date:** 2026-05-19 +**Started from:** 427b687b9588ac8e1f96070ffc20f2644e9cd28c +**Status:** DONE +**Tasks completed:** 1 of 1 + +## Summary + +The two null-ctx fallback RC-stats counters `g_rc_alloc_count` / +`g_rc_free_count` (`runtime/rc.c`) were plain `static uint64_t` with a +non-atomic `++` at the `__ail_tls_ctx == NULL` arm of `ailang_rc_alloc` +and the to-zero branch of `ailang_rc_dec`. A multi-threaded host that +uses the global fallback path (no `ailang_ctx_new`) raced the +read-modify-write and lost increments, so the `AILANG_RC_STATS` atexit +Σ under-counted non-deterministically. Not a memory bug — no box +crosses a thread; only the global statistics Σ was wrong. Fix +(user-approved Option A, 2026-05-19): the two global counters are now +`_Atomic uint64_t`, incremented via +`atomic_fetch_add_explicit(.., memory_order_relaxed)` and read via +`atomic_load_explicit(.., memory_order_relaxed)` in the atexit printer +(its only reader). Relaxed is correct and sufficient: pure statistics +with no happens-before obligation, and the atexit reader runs after all +worker threads have joined (single-threaded at exit). The per-ctx +counters and the per-object refcount header stay non-atomic by design +(single-thread-per-ctx; boxes never cross threads) — explicitly out of +scope. Scope held to `runtime/rc.c` only. + +## Per-task notes + +- bugfix-rc-global-stats-race.1: made `g_rc_alloc_count` / + `g_rc_free_count` `_Atomic uint64_t`; added ``; + `atomic_fetch_add_explicit(.., relaxed)` at the two null-ctx + fallback `++` sites; `atomic_load_explicit(.., relaxed)` snapshot + in `ailang_rc_stats_atexit` (the sole reader). Corrected the two + now-stale doc blocks (file header comment + the 18g.0 stats block) + to state the global counters are atomic while per-ctx + header + stay non-atomic by design. Left the rc.c drop-worklist + "Single-threaded; non-atomic" note unchanged — it refers only to + the per-call drop-worklist, which is genuinely still + single-threaded, so correcting it would be a false correction the + constraint explicitly forbids. RED `embed_rc_global_stats_race` + went GREEN deterministically (3/3 runs, allocs == frees == + 16_000_000); full `cargo test -p ail` green (39/39 result lines + ok, all embed_* incl. the per-ctx tsan harnesses unaffected). + +## Concerns + +(none) + +## Known debt + +- `runtime/rc.c` comment near the 18g.0 block ("adds only two + unconditional `++` operations to the hot path") is now slightly + imprecise: the global path is a relaxed atomic add and is + conditional on `_ctx == NULL`. Left verbatim deliberately — it is + a performance aside (not an atomicity claim), it predates and was + already imprecise after the M2 per-ctx split, and the atomicity + statement is now correctly carried by the adjacent rewritten + block; editing it would exceed the named doc-honesty drift the + minimal-fix constraint scoped this iter to. + +## Blocked detail + +(n/a — DONE) + +## Files touched + +- runtime/rc.c + +## Stats + +bench/orchestrator-stats/2026-05-19-iter-bugfix-rc-global-stats-race.json diff --git a/docs/journals/INDEX.md b/docs/journals/INDEX.md index a637888..23cd9d9 100644 --- a/docs/journals/INDEX.md +++ b/docs/journals/INDEX.md @@ -108,3 +108,4 @@ - 2026-05-18 — iter bugfix-over-strict-mode-ctor-rebuild-consume (RED→GREEN, debug→implement mini, DONE 1/1): fixed a conservative `[over-strict-mode]` false-positive surfaced by the Tick-coverage fixtures. The lint's consume-detection (`any_sub_binder_consumed_for`/`pattern_has_consumed_heap_binder`, `crates/ailang-check/src/linearity.rs`) only recognised a consume of an `(own (con T))` param when a *heap-typed* pattern-binder was moved out of `match p`; when `p` was destructured into purely *primitive* fields fed into a `Term::Ctor` rebuilding `p`'s own ctor, that genuine dismantle+rebuild consume was invisible, so the lint spuriously advised `(borrow ...)`. Real harm: an LLM author "fixing" the spurious warning by flipping an export's declared mode `own`→`borrow` would silently invert the ABI ownership contract. RED-first: debugger disproved the carrier's initial nested-`match` hypothesis (the M3 `embed_backtest_step_record.ail` is silent only because its implicit-mode scalar `Float` param disables the lint via the activation gate, linearity.rs:327 — NOT because it handles the rebuild; the defect reproduces single-param, no nesting), wrote the synthetic RED unit `over_strict_mode_silent_when_ctor_rebuilt_from_primitive_fields` committed as its own audit-trail commit `a11cb7c`. GREEN (implement mini): added a 2nd recognition path to `any_sub_binder_consumed_for` — a `match p` arm that destructures binders out of `p`'s ctor and references any of them (primitive or not) inside a `Term::Ctor`'s args in the arm body genuinely consumes `p`; two pure helpers (`ctor_uses_any_binder` + deep `term_mentions_any_binder`, so `(+ acc px)`-mediated flow counts), conservative toward NOT suppressing (a ctor ignoring `p`'s payload still warns — negative-control proven), over-strict-only (by-name shadowing imprecision is extra-silence, never under-strict; recorded as Known debt). Both stale doc comments that mis-attributed the FP to nested `match` corrected for doc-honesty (debugger concern #2, same code region — in-scope, not opportunistic). +166/−20 in linearity.rs only; check-only, zero codegen/runtime/ABI/schema/DESIGN.md change. Boss-verified independently: RED→GREEN, full `cargo test -p ailang-check` 108/0 lib + every binary 0-failed with NO existing test modified, `ail check embed_backtest_step_tick.ail` no longer over-strict on `st`/`tick` (exit 0), `_tick_borrow.ail` + M3 `embed_backtest_step_record.ail` still clean, the already-green `embed_tick_e2e` + bench posture untouched. No audit/fieldtest gate (lint-precision bugfix; the RED test + the green check-suite ARE the regression coverage). RED `a11cb7c` → GREEN this commit. → 2026-05-18-iter-bugfix-over-strict-mode-ctor-rebuild-consume.md - 2026-05-19 — iter embedding-abi-m5.1 (DONE 3/3): M5 iter 1 — stood up the workspace-excluded `ail-embed` crate. A zero-dependency embedding core (Rust port of the audited `crates/ail/tests/embed/tick_roundtrip.c`: `extern "C"` to the M3-frozen ABI + frozen-layout `State`/`Tick` box helpers + a `Kernel` price fold, raw pointers never escaping the type) links the M3 staticlib via a new `build.rs` (no in-repo precedent — `AIL_BIN` env override else nested `cargo build -p ail` against the parent workspace, separate target dir so no cargo-lock deadlock). Plus a hermetic `data-server` smoke: a synthetic Pepperstone-format ZIP fixture written via the crate's own public `RawTickRecord` type (correct-by-construction, mirrors `data-server/src/loader.rs:110-124`) → real `DataServer` → `Kernel`, asserting bit-exact vs a same-order host reference fold and the closed-form `55.0`; runs with no `/mnt`. `ail-embed` is its own cargo workspace root (empty `[workspace]` table) so the AILang compiler workspace owes it nothing; `data-server` is a dev-dependency only ⇒ Invariant 1 holds in the dependency graph, not just on paper. Boss-verified independently: ail-embed suite 2/2 green (`kernel_run_sums_prices` unit RED-first + `hermetic_smoke_data_server_roundtrip` integration), full+no-deps `cargo metadata` on the AILang workspace shows `data-server` count 0, `git status` path filter empty (zero diff to `crates/ailang-*`/`crates/ail/`/`runtime/`/`examples/*.ail`), `src/lib.rs` zero code-level `data_server`, AILang `cargo build --workspace` still clean. Two toolchain-forced corrections to the plan's verbatim `ail-embed/Cargo.toml` (added the empty `[workspace]` table; sibling dev-dep path `../libs`→`../../libs` manifest-relative) — confined to the plan-created manifest, no acceptance gate altered, 0 review re-loops; planner-recon implication recorded in the journal Concerns. Adapter API + thread-swarm explicitly deferred to M5 iter 2+. → 2026-05-19-iter-embedding-abi-m5.1.md - 2026-05-19 — iter embedding-abi-m5.2 (PARTIAL 2/3; M5 bounce-back): data-server adapter + symbol-fan swarm; the leak-proof did its job and surfaced a real concurrency finding. Tasks 1+2 landed clean: `data-server` promoted dev-dep→real dep (Invariant-1 sanctioned — compiler workspace graph still data-server-count 0, zero compiler-surface diff), additive `adapter` module (`tick_to_px`/`MidPriceStream` lazy `Iterator`/`fold_symbol`, RED-first), and a `swarm_runner` `[[bin]]` whose clean compile *is* the compile-time per-thread-ctx proof (`Ctx: !Send` ⇒ a shared-ctx swarm is `E0277`, cannot build). The real-data symbol-fan swarm runs (EURUSD/GER40/XAUUSD, ~4s) and is **bit-exact vs an independent same-order host reference every run** (`symbol_fan_swarm_bit_exact`, GREEN, live). Task 3's `Σallocs==Σfrees` leak gate is BLOCKED: Boss independently read `runtime/rc.c` and confirms it is an **instrumentation race, not a memory bug** — no box crosses a thread (`!Send`), the real RC is correct (bit-exact green), only the *global* `g_rc_*` stat counters (rc.c:90-91,161,212) are non-atomic *by rc.c's own documented single-threaded design* and lose `++`s when 3 worker threads hit the null-`__ail_tls_ctx` host-side path. M5 is AILang's first concurrent consumer → the deferred "concurrency arrived: atomic-vs-non-atomic" decision rc.c:44-49 names is now due. The leak assertion was Boss-split into `symbol_fan_swarm_leak_free` (`#[ignore]`, **body verbatim** — quarantined not weakened; un-ignore = the runtime fix's acceptance) so main stays green and the finding is pinned. Prior claim that `embed_swarm_tsan.rs` covers this was corrected (it uses the scalar kernel — zero box allocs — never exercised this path). Escalated as a **bounce-back** (touches M3-frozen `runtime/`; re-frames M5's "zero runtime change"; multiple substantive options); Boss recommendation on the record = Option A (atomic *global-fallback* counters only, standalone RED-first runtime micro-iter, M5 framing amended) — user picks the direction. Time-shard + friction-harvest remain m5.3. → 2026-05-19-iter-embedding-abi-m5.2.md +- 2026-05-19 — iter bugfix-rc-global-stats-race (RED→GREEN, debug→implement mini, DONE 1/1): the M5 iter-2 bounce-back resolution (user-approved Option A). The two null-ctx fallback RC-stats counters `g_rc_alloc_count`/`g_rc_free_count` (`runtime/rc.c:90-91`) were plain `static uint64_t` with a non-atomic `++` on the `__ail_tls_ctx == NULL` arm of `ailang_rc_alloc` (rc.c:161) / the to-zero branch of `ailang_rc_dec` (rc.c:212); a multi-threaded host driving the global fallback (no `ailang_ctx_new`) raced the read-modify-write and lost increments, so the `AILANG_RC_STATS` atexit Σ under-counted non-deterministically — exactly the deferred "concurrency arrived: atomic-vs-non-atomic" decision rc.c's own header (rc.c:44-49) names, M5 being AILang's first concurrent consumer. NOT a memory bug (`Ctx: !Send` keeps every box on one thread; the real refcount/free is correct; programs bit-exact). RED (debugger, audit-trail commit `427b687`): a C host — 8 threads × 2M alloc-then-dec, no ctx so the global path is taken, no box crossing a thread — + integration test asserting the atexit Σ is exact; Boss-verified RED `allocs=2141382 expected 16000000, live=-131242` (deterministic-fail under that contention, not flaky). GREEN (implement mini, this commit): the two globals are now `_Atomic uint64_t`, `atomic_fetch_add_explicit(.., memory_order_relaxed)` at the two fallback `++` sites, `atomic_load_explicit(.., relaxed)` in `ailang_rc_stats_atexit` (its sole reader) — relaxed is correct (pure stats, no happens-before; atexit reader runs post-join). Per-ctx counters + the per-object refcount header left non-atomic BY DESIGN (single-thread-per-ctx; boxes never cross threads) — explicitly out of scope, untouched; frozen value layout / ABI offsets / host-free rule untouched; the genuinely-still-single-threaded drop-worklist doc note correctly left unchanged (a false correction was refused); the two genuinely-stale atomicity doc blocks corrected for honesty. Scope held to `runtime/rc.c` ONLY (37+/14−). Boss-verified independently: RED→GREEN deterministically 3/3 (jitter gone, `allocs==frees==16_000_000`), full `cargo test -p ail` green (every binary 0 failed — the per-ctx tsan harnesses embed_swarm_tsan/embed_rc_accounting_tsan + embed_tick_e2e/embed_record_e2e unaffected since the per-ctx path is untouched; rc.c links into every ail binary so this is the strong regression gate), git-status scope = rc.c + journal + stats only, the RED files unchanged. Unblocks resuming the M5 leak-proof (un-`#[ignore]` `ail-embed`'s `symbol_fan_swarm_leak_free`). RED `427b687` → GREEN this commit. → 2026-05-19-iter-bugfix-rc-global-stats-race.md diff --git a/runtime/rc.c b/runtime/rc.c index 0a7dd2e..e6f31a4 100644 --- a/runtime/rc.c +++ b/runtime/rc.c @@ -42,12 +42,20 @@ * ABI" > "Frozen value layout". A boundary-crossing single-ctor * scalar record's box offsets MUST NOT move. * - * Single-threaded: counter ops are non-atomic. AILang has no - * concurrency primitives yet; when it acquires them, atomic-vs-non- - * atomic becomes a separate decision per allocation kind (see - * Decision 10's "Does not commit to atomic refcounts" clause). + * Threading: the per-object refcount header ops and the per-ctx RC + * counters are non-atomic by design — a box never crosses a thread + * and each ailang_ctx_t is single-thread-per-ctx, so neither is + * contended. The two GLOBAL null-ctx fallback stats counters + * (g_rc_alloc_count / g_rc_free_count) ARE atomic (relaxed): a + * multi-threaded host using the null-ctx fallback path concurrently + * increments them, and the AILANG_RC_STATS atexit Σ must not lose + * updates. AILang itself still has no concurrency primitives; + * atomic-vs-non-atomic on the refcount header remains a separate + * future decision per Decision 10's "Does not commit to atomic + * refcounts" clause. */ +#include #include #include #include @@ -66,7 +74,7 @@ static inline ailang_rc_header_t *header_of(void *payload) { /* --------------------------------------------------------------------------- * Iter 18g.0: opt-in alloc/free stats counter. * - * Two non-atomic 64-bit counters incremented from `ailang_rc_alloc` and + * Two atomic 64-bit counters incremented from `ailang_rc_alloc` and * the to-zero branch of `ailang_rc_dec`. Their difference is the live * cell count at any point in execution; at program exit a non-zero * difference is a leak. @@ -82,13 +90,22 @@ static inline ailang_rc_header_t *header_of(void *payload) { * and the bench numbers in JOURNAL 18f.2 were taken with the counters * compiled in but disabled, so the figures are still valid. * - * Single-threaded; non-atomic. Same scope as the rest of `runtime/rc.c`. - * The counters are intentionally not exposed via FFI symbols — the + * Atomic (relaxed): a multi-threaded host can drive the null-ctx + * fallback path concurrently, so these two counters use + * `atomic_fetch_add_explicit(.., memory_order_relaxed)` on increment + * and `atomic_load_explicit(.., memory_order_relaxed)` on read. + * Relaxed is correct and sufficient: the counters are pure + * statistics with no happens-before obligation, and the atexit + * reader runs after all threads have joined (single-threaded at + * exit). The per-ctx counters and the refcount header stay + * non-atomic by design (single-thread-per-ctx; boxes never cross + * threads). The counters are intentionally not exposed via FFI + * symbols — the * env-var-gated atexit print is the supported readback path, and that * is sufficient for the e2e leak tests that consume the diagnostic. * --------------------------------------------------------------------------- */ -static uint64_t g_rc_alloc_count = 0; -static uint64_t g_rc_free_count = 0; +static _Atomic uint64_t g_rc_alloc_count = 0; +static _Atomic uint64_t g_rc_free_count = 0; /* M2: per-thread embedding context. A scalar kernel allocates nothing, * so this carries only the de-globalised RC accounting. Set by the @@ -122,11 +139,15 @@ void ailang_ctx_free(ailang_ctx_t *ctx) { } static void ailang_rc_stats_atexit(void) { + uint64_t allocs = + atomic_load_explicit(&g_rc_alloc_count, memory_order_relaxed); + uint64_t frees = + atomic_load_explicit(&g_rc_free_count, memory_order_relaxed); fprintf(stderr, "ailang_rc_stats: allocs=%llu frees=%llu live=%lld\n", - (unsigned long long)g_rc_alloc_count, - (unsigned long long)g_rc_free_count, - (long long)(g_rc_alloc_count - g_rc_free_count)); + (unsigned long long)allocs, + (unsigned long long)frees, + (long long)(allocs - frees)); } __attribute__((constructor)) @@ -158,7 +179,8 @@ void *ailang_rc_alloc(size_t size) { void *payload = (uint8_t *)block + HEADER_SIZE; memset(payload, 0, size); ailang_ctx_t *_ctx = __ail_tls_ctx; - if (_ctx != NULL) _ctx->alloc_count++; else g_rc_alloc_count++; + if (_ctx != NULL) _ctx->alloc_count++; + else atomic_fetch_add_explicit(&g_rc_alloc_count, 1, memory_order_relaxed); return payload; } @@ -209,7 +231,8 @@ void ailang_rc_dec(void *payload) { if (*hdr == 0) { free(hdr); ailang_ctx_t *_ctx = __ail_tls_ctx; - if (_ctx != NULL) _ctx->free_count++; else g_rc_free_count++; + if (_ctx != NULL) _ctx->free_count++; + else atomic_fetch_add_explicit(&g_rc_free_count, 1, memory_order_relaxed); } }