# Resreq AST + CSE Implementation Plan > Generated from: `resreq-ast-cse-design.md` > Date: 2026-04-28 > Branch: merge_api_server --- ## Summary Priority Order ``` P0 (blocking): makeString() TOCTOU fix, section.peg boundary bug ✓ DONE Phase 1: TDD test skeletons (parallel with Phase 3) ✓ DONE Phase 2: Header definitions + ABI audit ✓ DONE Phase 3: PEG grammar (parallel with Phase 1) ✓ DONE 143 PASS / 3 FAIL (stubs) Phase 4a: Core interning + sel_optimize_level ✓ DONE 145 PASS / 1 FAIL (Phase 4b stub) Phase 4b: Type validation (gated on P0-1) ✓ DONE 203 PASS (2026-04-29) Phase 4c: AND/OR associativity flattening ✓ DONE 157 PASS (2026-04-29) Phase 4d: Const-fold transform ✓ DONE (2026-04-29) Phase 4e: Memoized eval ✓ DONE 73 PASS (2026-04-29) Phase 4f: GC + recompaction ✓ DONE 73 PASS (2026-04-29) Phase 5: Build wiring ✓ DONE Phase 6: Scheduler integration ✓ DONE 10 PASS (2026-04-30) ``` --- ## P0 — Critical Prerequisites (blocking gate) Must complete before any Phase 4 work begins. | # | Task | Section | Rationale | |---|------|---------|-----------| | P0-1 | ~~**Fix `makeString()` TOCTOU in `src/lib/xstring.c`**~~ **DONE** — extended lock region to cover `makeInstance` + `key_add`, eliminating the TOCTOU window. `key_add` can no longer return `-EEXIST` on a miss path. | §5.4 | Silent NULL return corrupts the interned AST in all §7 numeric-to-string coercions; blocks Phase 4b entirely | | P0-2 | ~~**Fix `section.peg` clause-boundary grammar**~~ **DONE** — Root cause: the original grammar used multiple ordered-choice alternatives in `select_statement` / `usage_statement` / `span_statement`, but PEG's `e*` always succeeds (matches empty), so alternatives 2+ were dead code (confirmed by packcc generating "defined but not used" for all `pcc_action_select_statement_N` functions). Fix: collapsed each statement rule into a single alternative with a compound negative lookahead covering all subsequent clause keywords. `select_statement` now stops at any of `rusage[`, `span[`, `same[`, `affinity[`, or `]`; `usage_statement` stops at `span[`, `same[`, `affinity[`, or `]`; `span_statement` stops at `same[`, `affinity[`, or `]`. Both implicit (`type==any span[ptile=1]`) and explicit (`select[type==any] span[ptile=1]`) select forms verified by TDD: `test_parse_section3` (implicit), `test_parse_section_explicit_select_with_span`, `test_parse_section_explicit_select_rusage_span`, `test_parse_section_span_not_in_select` — all PASS, no packcc dead-code warnings. | §3.1 | Silent span/rusage content bleed into selectString in all implicit-select + multi-clause inputs | --- ## Phase 1 — TDD Tests (write failing skeletons first) ✓ DONE **Priority: HIGH — must come before implementation (TDD contract)** Develop concurrently with Phase 3 (grammar is the test oracle). | # | Task | Notes | |---|------|-------| | 1-1 | ~~Write `lib/testsuite/test_select.c` skeletons: parse valid expressions, unknown resource names (error), type mismatches (error)~~ **DONE** | | | 1-2 | ~~Write tests: `SELECT_MAX_INPUT_LEN` (8192) overflow, `SELECT_MAX_DEPTH` (64) nesting overflow~~ **DONE** | | | 1-3 | ~~Write CSE tests: `A&&B` and `B&&A` produce same pointer; two `makeSelectNode` calls with same args return same pointer~~ **DONE** — FAIL until Phase 4a | | | 1-4 | ~~Write eval tests: numeric `>/<`/`==`, string `==`, bool standalone, `!`/`&&`/`||`, short-circuit~~ **DONE** | | | 1-5 | ~~Write GC tests: unlink job root → `gc_sweep_select_nodes` frees unreachable node; subsequent `makeSelectNode` re-interns fresh node~~ **DONE** — FAIL until Phase 4f | | | 1-6 | ~~Write GC predicate contract tests (`sched_session_active`, `sched_eval_in_progress`, `staging_merge_complete`) using `fake_scheduler_t` scaffold in `test_select.c`~~ **DONE** — FAIL until Phase 4f | Phase 6 wires real scheduler — signatures stable | | 1-7 | ~~Create shared resmgr fixture: `src/lib/testsuite/test_resmgr_fixture.h` + `.c` with `make_test_resmgr()`/`free_test_resmgr()`~~ **DONE** | | --- ## Phase 2 — Header Definitions ✓ DONE **Priority: HIGH — unblocks all implementation work** | # | Task | Notes | |---|------|-------| | 2-1 | ~~Add `res_type_t`, `sel_node_type_t`, `xce_select_node_t` typedef + struct to `include/resreq.h`~~ **DONE** | | | 2-2 | ~~Fill `struct xce_resreq_select_` with `xce_select_node_t* root`~~ **DONE** | | | 2-3 | ~~Declare all public APIs: `makeSelectNode`, `eval_select`, `eval_select_simple`, `select_eval_ctx_*`, `sel_node_table_*`, `gc_sweep_select_nodes`, `recompact_node_ids`~~ **DONE** | `gc_mark_tree` stays `static` in `.c` | | 2-4 | ~~Add `var resmgr` field to `resreq_context_t` (outer PEG auxil)~~ **DONE** | | | 2-5 | ~~Add `valtype_to_res_type` helper; omit `res_type_to_valtype` (no current consumer — YAGNI)~~ **DONE** | | | 2-6 | ~~Add `xce_str_match` as `static inline` wrapping `strcmp`~~ **DONE** | | | 2-7 | ~~**CGO audit**: `grep -rn 'parse_resreq\|C\.parse_resreq' src/frontend/`~~ **DONE** — no CGO callers found | | | 2-8 | ~~Update `parse_resreq_primary` and `parse_resreq` signatures to accept `var resmgr`~~ **DONE** — call sites updated: `schedobj.c:171`, `test_resreq.c:244/301/330` | | --- ## Phase 3 — PEG Grammar (`lib/xce_select.peg`) ✓ DONE **Priority: HIGH — develop concurrently with Phase 1** | # | Task | Notes | |---|------|-------| | 3-1 | ~~Implement `select_expr` grammar (§3.1): `\|\|`, `&&`, `!`, comparisons, resname, literals, `any`~~ **DONE** | | | 3-2 | ~~Implement `select_context_t` auxil with `resmgr`, `depth`, `errmsg[256]`, `has_error`~~ **DONE** | | | 3-3 | ~~Implement `resname` action: `key_find`, `res_idx` validity guard, `valtype_to_res_type` mapping~~ **DONE** | | | 3-4 | ~~**Clause-keyword conflict check** in `resname` action: check against `{select, rusage, span, same, affinity}`~~ **DONE** | | | 3-5 | ~~Implement `SELECT_MAX_INPUT_LEN` (8192) and `SELECT_MAX_DEPTH` (64) guards~~ **DONE** | | | 3-6 | ~~LSF-compatible disambiguation: bare word vs known resource name → `SEL_RESNAME` vs `SEL_STR`; `any` excluded by grammar; `true`/`false` produce migration diagnostic~~ **DONE** | | | 3-R1 | ~~**[Review fix — HIGH]** Switch `SEL_STR` from `strdup`/`free` to `makeString`/`dereference` in `xce_select.peg` and `xce_select.c`~~ **DONE** — xstring-interned strings now refcounted; `kill_select_node` calls `dereference()` | | | 3-R2 | ~~**[Review fix — MEDIUM]** Add `_Static_assert(sizeof(uintptr_t) >= sizeof(double))` before `makeSelectNode` in `xce_select.c`~~ **DONE** — guards the `SEL_NUM` bit-cast at compile time | | | 3-R3 | ~~**[Review fix — MEDIUM]** Add `!("any" ![a-zA-Z_0-9])` negative lookahead to `resname_node` and `resname_leaf` in `xce_select.peg`~~ **DONE** — prevents `any` keyword from being tried as a resource name | | --- ## Phase 4a — Core Node Allocation and Interning ✓ DONE **Priority: HIGH — foundational; unblocks 4b–4f** | # | Task | Notes | |---|------|-------| | 4a-1 | ~~Implement `sel_optimize_level` (`static _Atomic int`, sentinel -1) with `sel_optimize_set()` write-once CAS and `sel_get_optimize_level()` accessor~~ **DONE** | | | 4a-2 | ~~Implement `sel_make_key()` — zero padding bytes via struct literal init; SEL_RESNAME zeroes `op2` for key purposes (res_type stored in node, not in key)~~ **DONE** | | | 4a-3 | ~~Implement `sel_key_hash` over the 3 key fields~~ **DONE** | | | 4a-4 | ~~Implement `sel_node_table_init` / `sel_node_table_fini` — `makeInstance(&kvtab_type, ¶ms)` with **`datakiller=NULL`**, `zarray_init` for linear nodes array~~ **DONE** | | | 4a-5 | ~~Implement `makeSelectNode` — CSE-disabled fast path; CSE-enabled path with commutativity sort, key lookup, TOCTOU double-check under lock, node_id assignment via `atomic_fetch_add`, zarray rollback via `atomic_fetch_sub`~~ **DONE** | | | 4a-6 | ~~Implement `sel_node_to_key()` for GC key reconstruction (must apply same commutativity sort as `makeSelectNode`)~~ **DONE** | | | 4a-7 | ~~Add `sel_optimize_test_reset()` test-only helper (`#ifdef UNIT_TEST`): `atomic_store_explicit` reset to -1 plus `sel_node_table_fini`~~ **DONE** | | | 4a-R1 | ~~**[Review fix — HIGH]** Move `node_id` assignment inside lock after `key_add` succeeds so TOCTOU losers need no counter rollback~~ **DONE** | Counter divergence under concurrent load | | 4a-R2 | ~~**[Review fix — HIGH]** Add `dereference((char*)op1)` on first-lock cache hit for SEL_STR — caller transfers xstring ownership~~ **DONE** | Ref leak on every cache hit for string nodes | | 4a-R3 | ~~**[Review fix — HIGH]** Add `SEL_MAX_INTERNED_NODES` cap check in both lock sections (first miss + TOCTOU re-check)~~ **DONE** | Unbounded table growth without cap | | 4a-R4 | ~~**[Review fix — HIGH]** Fix `sel_node_table_fini` to iterate and free all interned nodes before `destroy(table)` — NULL datakiller does not free them~~ **DONE** | All nodes leaked at teardown; LSAN clean now | | 4a-R5 | ~~**[Review fix — MEDIUM]** Set `node->hashcode = sel_key_hash_fn(&key)` in CSE path — was left 0, breaking Phase 4e memoization~~ **DONE** | | | 4a-R6 | ~~**[Review fix — MEDIUM]** Add `dereference((char*)op1)` on `calloc` failure in level==0 path~~ **DONE** | xstring ref leak on OOM | | 4a-R7 | ~~**[Review fix — MEDIUM]** Add `-EINVAL` guard on `key_find` — corrupt table must not silently proceed~~ **DONE** | | | 4a-R8 | ~~**[Review fix — MEDIUM]** Add `-EEXIST` recovery in `key_add` failure path — re-find and return the racing winner~~ **DONE** | Rare concurrent insert race | | 4a-R9 | ~~**[Review fix — LOW]** Change `_Static_assert(sizeof(uintptr_t) >= ...)` to `==` per spec requirement~~ **DONE** | | --- ## Phase 4b — Parse-Time Type Validation ✓ DONE (2026-04-29) **Priority: HIGH — gate: P0-1 `makeString()` fix must be merged first** | # | Task | Notes | |---|------|-------| | 4b-1 | ~~Wire `parse_select()` into `parse_resreq_primary` **before** `kill_resreq_data`~~ **DONE** | Use-after-free if called after `kill_resreq_data` frees `data.selectString` | | 4b-2 | ~~Implement EQ/NE type validation: reject BOOL×STR; coerce SEL_NUM to SEL_STR when comparing with string resource (`snprintf "%.17g"`), NULL-check `makeString` result, symmetric coercion~~ **DONE** | Free orphaned SEL_NUM node in unoptimized mode before overwriting pointer | | 4b-3 | ~~Implement `>/<`/`>=`/`<=` type validation: reject non-numeric operands~~ **DONE** | `log_fatal` (not `assert`) for belt-and-suspenders at eval time too | | 4b-4 | ~~Implement `res_idx` validity guard and unknown-resource error; add migration diagnostic for `true`/`false` identifiers (parse as SEL_RESNAME with no match)~~ **DONE** | | | 4b-5 | ~~Update `kill_primary_resreq` for conditional teardown: CSE-enabled → null root only; CSE-disabled → `free_select_tree()` then `FREEUP`~~ **DONE** | NULL store before `FREEUP` to prevent GC mark dangling-pointer race | | 4b-6 | ~~**LC_NUMERIC enforcement**: wrap PEG parser call in `parse_select` with `newlocale(LC_NUMERIC_MASK, "C")` / `uselocale` / `freelocale`; `log_fatal` on failure in scheduler, `log_error`+return in API server~~ **DONE** | `_XOPEN_SOURCE 700` required before system headers | | 4b-7 | ~~**strtod locale coverage check**~~ **DONE — SAFE**: packcc emits single `strtod` in `pcc_action_value_node_1` (line 1593 of generated file), inside the `uselocale` scope — no locale vulnerability | | | 4b-R1 | ~~**[Review fix — HIGH]** Add `lock(auxil->resmgr, RTTI_LOCK_READ)` / `unlock(auxil->resmgr)` around `key_find` in `resolve_resname()` (`xce_select.peg:38`) — missing read lock caused data race in multi-threaded scheduler~~ **DONE** | | | 4b-R2 | **[Review check — F2]** STRING×NUM coercion error paths reviewed: `makeSelectNode` releases the xstring on OOM failure (level-0 path calls `dereference(op1)` before returning NULL); outer `makeSelectNode(op_type)` failure correctly calls `SEL_DISCARD(l)` + `SEL_DISCARD(r)` — **no leak, false positive** | | --- ## Phase 4c — Commutativity + Associativity Flattening ✓ DONE (2026-04-29) | # | Task | Notes | |---|------|-------| | 4c-1 | ~~Verify commutativity sort in `makeSelectNode` step 1 covers AND/OR/EQ/NE; non-commutative GT/LT/GE/LE/NOT unchanged~~ **DONE** — `test_cse_gt_not_commutative` confirms GT is not commutative; AND/OR/EQ/NE sort verified. | Already integrated in §5.4 design | | 4c-2 | ~~Implement post-parse **flatten pass** for `&&`/`||` chains: `do { changed = flatten_pass(root); } while (changed)`~~ **DONE** — `sel_flatten_pass` added to `xce_select.c`; do-while loop in `parse_select()` in `resreq.c`; 7 tests: noop, single-level, deep 3+, OR, mixed-ops, CSE mode. 157 PASS. | Repeat-until-stable needed for depth-3+ explicit parenthesization | | 4c-3 | ~~In unoptimized mode: `flatten_pass` must `free()` each orphaned node — not its children (re-parented children must not be freed)~~ **DONE** — level==0 path frees orphaned intermediate node (`free(new_right); free(node)`) after re-parenting A/B/C; SEL_NOT branch frees `node` only. | Children freed would cause double-free when `free_select_tree` walks from root | **Senior review fixes applied (2026-04-29):** - **[MEDIUM — OOM safety]** In AND/OR branch: update `node->u.binop.left/right` in-place (level==0) immediately after recursive calls, before any `makeSelectNode`. This ensures `return node` after OOM returns a valid tree with live (not dangling) children. - **[LOW — dead code]** Removed dead `if (node->type == SEL_STR ...)` / `if (new_right->type == SEL_STR ...)` guards in AND/OR and SEL_NOT branches — those nodes are never SEL_STR in those arms. --- ## Phase 4d — Const-Fold Transform ✓ DONE (2026-04-29) | # | Task | Notes | |---|------|-------| | 4d-1 | ~~`resname == any` → `SEL_TRUE`; `resname != any` → `SEL_FALSE` — check **both** child slots for SEL_ANY pointer (commutativity sort is ASLR-nondeterministic across runs)~~ **DONE** | | | 4d-2 | ~~Identity laws: `SEL_TRUE && X` → `X`; `SEL_FALSE \|\| X` → `X`~~ **DONE** | | | 4d-3 | ~~Annihilation laws: `SEL_FALSE && X` → `SEL_FALSE`; `SEL_TRUE \|\| X` → `SEL_TRUE`~~ **DONE** | | | 4d-4 | ~~Complement laws: `!SEL_TRUE` → `SEL_FALSE`; `!SEL_FALSE` → `SEL_TRUE`~~ **DONE** | Single-pass — const-fold shrinks trees, does not create new flattening opportunities | --- ## Phase 4e — Evaluation with Memoization ✓ DONE (2026-04-29) | # | Task | Notes | |---|------|-------| | 4e-1 | ~~Implement `eval_get_bool`/`eval_set_bool` 2-bit bitmap accessors with `node_id >= bool_bits_cap` guards using `log_fatal`~~ **DONE** | `_Static_assert(EVAL_NONE == 0)` for memset-reset correctness | | 4e-2 | ~~Implement `eval_select_inner` — recursive eval with **explicit** error propagation (`if (lv < 0) return -1`), NOT C `&&`/`||` (C's `-1` is truthy, would swallow errors)~~ **DONE** | SEL_ANY reaching eval → `log_fatal("SEL_ANY reached eval — const-fold not applied")` | | 4e-3 | ~~Leaf cache writes in `eval_select_inner`: SEL_RESNAME writes to `ctx->num_vals[node_id]` or `ctx->str_vals[node_id]`; comparison ops call `eval_select(child, ctx)` then read cached arrays~~ **DONE** | **Critical**: `get_leaf_value()` must NOT be called directly — bypasses cache writes, silently reads 0.0/NULL | | 4e-4 | ~~Implement memoized `eval_select` wrapper — bitmap cache check before calling `eval_select_inner`; cache EVAL_ERROR too (node-structural errors repeat per host)~~ **DONE** | | | 4e-5 | ~~Implement `eval_select_simple` for unoptimized path — plain recursive tree-walk, no `select_eval_ctx_t`, thread-safe (no shared state)~~ **DONE** | | | 4e-6 | ~~Add `eval_select` guard: `log_fatal` if `sel_get_optimize_level() <= 0`~~ **DONE** | | | 4e-7 | ~~Implement `select_eval_ctx_init` / `select_eval_ctx_reset` / `select_eval_ctx_fini`~~ **DONE** | `node_count` snapshot at init; `reset` uses snapshot, never re-reads `sel_node_next_id` | | 4e-8 | ~~Pre-session capacity check: `memset` extended bool_bits bytes after realloc (realloc does not zero extension); partial-OOM contract — all three reallocs (bool_bits, num_vals, str_vals) must succeed atomically or `log_fatal`~~ **DONE** | Mismatched capacities allow out-of-bounds write to num_vals with no inline bounds check | --- ## Phase 4f — Mark-and-Sweep GC + Recompaction ✓ DONE (2026-04-29) | # | Task | Notes | |---|------|-------| | 4f-1 | ~~Implement `gc_mark_tree` — recursive DFS, early return on `node == NULL \|\| node->gc_mark`; handles leaf / SEL_NOT / binary node types~~ **DONE** — `__attribute__((unused))` for Phase 6 wire-in | Recursion bounded by `SELECT_MAX_DEPTH` (64); any increase requires stack analysis | | 4f-2 | ~~Implement `gc_sweep_select_nodes` — Phase 6 guard placed **before** mark-clear (prevents two-call sweep-of-reachable-nodes bug); mark-clear iterates `caps`; sweep iterates `caps`, `zarray_del` before `free`~~ **DONE** — live_jobs traversal deferred to Phase 6 (sched/types.h not in libxce scope) | GC does NOT acquire `sel_node_table.lock` (idle-phase invariant) | | 4f-3 | Insert **`atomic_thread_fence(memory_order_seq_cst)`** after all `primary->select->root = NULL` unlinks and before `gc_sweep_select_nodes` call — **Phase 6 call-site task** | Without fence: CPU/compiler may reorder NULL store after GC mark pass | | 4f-4 | Runtime GC idle invariant guards at GC call site (`sched_session_active`, `sched_eval_in_progress`, `staging_merge_complete`) — `log_fatal` not `assert` — **Phase 6 call-site task** → see 6-3/6-4 | `assert` compiled out by NDEBUG; use-after-free would result | | 4f-5 | ~~Implement `recompact_node_ids` — iterate `nodes` zarray over `caps` slots (not `count`, gaps exist after `zarray_del`), assign sequential `node_id`, reset `sel_node_next_id` via `atomic_store_explicit(..., memory_order_release)`~~ **DONE** | `node_id` and zarray slot index are **decoupled** — do not conflate | | 4f-R1 | ~~**[Review fix — HIGH]** Move Phase 6 early-return guard to before mark-clear loop — prior ordering allowed two consecutive calls (njobs>0 then njobs==0) to sweep still-reachable nodes~~ **DONE** | Two-call scenario: first call clears all marks and returns; second call sweeps everything | | 4f-R2 | ~~**[Review fix — HIGH]** Swap `free(node)` / `zarray_del` order — `zarray_del` NULLs slot first, then `free`; eliminates latent use-after-free if killer is ever installed on nodes zarray~~ **DONE** | | | 4f-R3 | ~~**[Bug fix]** `sel_node_table_fini` iterated `nodes.count` instead of `nodes.caps` — after `zarray_del` creates gaps, new nodes slot into positions beyond `count` boundary and leaked~~ **DONE** | Manifested as 32-byte ASAN leak after GC sweep + reintern in same lifetime | --- ## Phase 5 — Build Wiring | # | Task | Notes | |---|------|-------| | 5-1 | ~~Add `custom_target` for `xce_select.peg` in `lib/meson.build` matching `section_parser`/`rusage_parser` pattern~~ **DONE** | Verify `-o 'lib/xce_select'` path format: `grep -A8 'custom_target' src/lib/meson.build \| grep '\-o'` | | 5-2 | ~~Add `xce_select_parser[0]` (generated `.c`) to `libxce_src`~~ **DONE** | | | 5-3 | ~~Add `test_resmgr_fixture.c` to testsuite `executable()` sources in `lib/meson.build`~~ **DONE** | | --- ## Phase 6 — Scheduler Integration **Status: ✓ DONE — 2026-04-30 — 10/10 scheduler tests pass** | # | Task | Notes | |---|------|-------| | 6-1 | ~~**Pre-Phase-6 audit**: confirm allocator-thread-only invariant~~ **DONE** `bb4a95c` | Single-threaded confirmed; stub at `resource.c:127` | | 6-2 | ~~**Blocking gate**: all `sched_meet_host_selection` call sites accounted for~~ **DONE** `bb4a95c` | No missed call sites | | 6-3 | ~~Add `_Atomic int session_active`, `eval_in_progress`, `staging_merge_complete` to `xce_scheduler_t`~~ **DONE** `823a66e` | Use `_Atomic int` not `volatile int` — ARM atomicity guarantee | | 6-4 | ~~Implement GC predicate wrappers referencing real `xce_scheduler_t` fields~~ **DONE** `acda9bb` | Signatures unchanged; only wrapper body changed | | 6-5 | ~~Implement `sched_meet_host_selection` via `eval_select_simple`~~ **DONE** `40261b7` | Full implementation replacing stub that always returned 1 | | 6-6 | ~~`sched_meet_host_selection` returns int (0/1); preason on fail~~ **DONE** `40261b7` | int return kept; preason wired | | 6-7 | ~~Implement `build_host_eval_ctx`~~ **DONE** `907d8d8` | Flattens `worker->resvals` zarray to `numvals`/`strvals` using `Value_u_fvalue/svalue/bvalue` | | 6-8 | ~~Materialize `live_jobs` flat array from `sched->jobs` kvmgr~~ **DONE** `5267d36` | Growth-loop heap alloc; OOM-safe (double-guarded: `calloc` fail + `realloc` null-sentinel `742d79e`, `fb5127a`) | | 6-9 | ~~Wire `sel_optimize_set(1)` at scheduler startup~~ **DONE** `acda9bb` | Called in `sched_init`; no `sel_optimize_set(0)` needed (API server has separate process) | | 6-10 | ~~Set `sel_alloc_thread_id`; add thread-ID checks in `eval_select` and `gc_sweep_select_nodes`~~ **DONE** `b722e5c` `1eb4eb1` | `_Atomic int` registered flag + `pthread_equal` guard; registered before `while(1)` loop | | 6-11 | ~~Write integration/parity tests against known-good resreq/host pairs~~ **DONE** `5fcf85c` | Numeric (`SEL_GE`) and string (`SEL_EQ`) pass/fail cases | | 6-12 | ~~`atomic_thread_fence(memory_order_seq_cst)` before `gc_sweep_select_nodes`~~ **DONE** `5267d36` | After all `gc_mark_select_tree` calls in `sched_end_session` | | R1 | ~~**[Review fix — CRITICAL]** `staging_merge_complete` bootstrap deadlock — GC never ran~~ **DONE** `d438742` | `sched_begin_session` never set it to 1; guard in `sched_end_session` always fired | | R2 | ~~**[Review fix — HIGH]** `recompact_node_ids()` missing from GC protocol~~ **DONE** `d438742` | Added after `gc_sweep_select_nodes` in `sched_end_session` | --- ## Key Implementation Traps 1. **`free_select_tree` vs GC mode confusion** — calling `free_select_tree` in CSE-enabled mode or forgetting it in disabled mode both produce memory corruption. The `sel_get_optimize_level()` guard inside `free_select_tree` catches this with `log_fatal`. 2. **`get_leaf_value()` bypass bug** — calling `get_leaf_value()` directly in comparison branches bypasses the `num_vals`/`str_vals` cache write. Comparisons silently read 0.0/NULL (calloc default) instead of actual host values. Every comparison operator must call `eval_select(child, ctx)` first, then read the typed cache array. 3. **TOCTOU in `makeSelectNode`** — the double-check under lock (step 5) prevents node leaks when two threads race to intern the same node. The re-find must happen inside the lock or the winner could be freed before the loser reads it. 4. **`atomic_thread_fence` before GC** — root-unlink writes (`primary->select->root = NULL`) must be visible to the GC mark pass. Without a seq_cst fence, CPU/compiler reordering causes GC to retain a node that should be swept (or vice versa). 5. **Partial-OOM in `select_eval_ctx_t` resize** — if one of the three reallocs (bool_bits, num_vals, str_vals) succeeds and another fails, the capacities diverge and `eval_select_inner` can write out-of-bounds to `num_vals` (no inline bounds guard at write site). Treat all three as atomic — `log_fatal` on partial failure.