docs(job-array-single-wal-record): mark task 1.3's rtti leak note FIXED

Upstream librtti repin (see prior commit) resolves the SIGABRT this
note warned future tasks to expect and ignore -- updated so it no
longer reads as a live gotcha.
This commit is contained in:
Rongsong Shen 2026-07-17 09:28:52 +00:00
parent 8c7d26a9ce
commit 2cfb454119

View file

@ -218,7 +218,7 @@
- **Task 7.3**: `sched_handle_dataset()`'s `Data_data_jobArray` case now calls `free_JobSubmit_ptr((xce_job_submit_t**)&dataset->rows)` + `dataset->nrows = 0` after `sched_on_job_array()` returns (success and error paths alike), mirroring the sibling list-shaped cases' ownership-transfer convention. `free_JobSubmit_ptr()` is the existing capnp-generated teardown for this exact struct shape (frees `submit`'s 7 strings + `start_info`'s 10 strings + both nested structs + the top-level struct) — no new free logic was needed, just the missing call site. Fixing this exposed that `test_job_array_expand.c`'s white-box fixture (`init_job_array()`) built `dataset->rows` from stack-allocated structs with string-literal fields; freeing those crashes (SEGV in `free_JobSubmitData`). The fixture was rewritten to heap-allocate + `strdup` (matching the real `decode_JobSubmit_ptr()` wire-decode shape), and `test_job_array_redelivery_does_not_duplicate` now builds a fresh descriptor per delivery (the first is consumed+freed by the handler, so reusing one pointer across two calls is no longer valid — this also matches production, where a redelivery is always a freshly-decoded wire message, never a reused pointer). Verified via direct ASAN runs of all 4 scheduler job-array test binaries (leak detection is disabled by default in the meson test harness): zero leak growth traceable to the jobArray path in any of them (confirmed by grepping leak stack traces, not just byte counts, since fixture-init debt is a separate pre-existing, unrelated leak source in the same binaries).
- **Task 1.3**: `meson test --suite xce`'s `libxce test` target SIGABRTs under ASAN leak detection whenever any code path exercises a `job_init()` failure — root cause is the vendored `rtti` library's `makeInstance()` not freeing its allocation when a type's `.init()` returns negative (no `.c` source available, headers-only in the nix store). This is pre-existing and independently reproduced against an unmodified pre-task `jobj.c`, not introduced by this feature. It will fire again on any future task whose tests exercise a `create_job()`/`makeJob()` failure path (e.g. task 3.1/3.4's fault-injection tests for `restore_job_array()`) — expect the same SIGABRT-not-assertion-failure signature (`grep ":FAIL"` on the Unity log will be empty; the failure is LeakSanitizer-only) and do not treat it as a new regression without the same stash-and-reproduce check task 1.3's review used.
- **Task 1.3 (FIXED 2026-07-17, upstream)**: `meson test --suite xce`'s `libxce test` target used to SIGABRT under ASAN leak detection whenever any code path exercised a `job_init()` failure — root cause was the vendored `rtti` library's `makeInstance()` not freeing its allocation when a type's `.init()` returns negative (no `.c` source available, headers-only in the nix store). Pre-existing, independently reproduced against an unmodified pre-task `jobj.c`, not introduced by this feature. **Resolved by a `librtti` repin** (`src/flake.lock`: rev `7212ba170d8db040ac99d0fb47c7b1df70df6e11` → `53cf11e6df4e2f0bbcdf8e26feed74d011b0e9c1`, revCount 20→21) — verified via a genuinely fresh rebuild (`nix flake metadata` confirms the new rev resolved) plus a full green `libxce test` run and a standalone `test_jobj_hardening` run under `ASAN_OPTIONS=detect_leaks=1:abort_on_error=1`: `test_job_init_propagates_makeJobsub_failure` (the trigger test) now passes clean, zero LeakSanitizer output, no `makeInstance`/`jobj.c:768` leak stack. Any future task whose tests exercise a `create_job()`/`makeJob()` failure path should now see a normal PASS/FAIL Unity result, not a SIGABRT — if the SIGABRT reappears, treat it as a genuine regression (possibly a librtti downgrade), not this known-and-now-closed issue.
- **Task 1.3**: `job_obj_init()`/`job_obj_fini()` are now idempotent (atomic_flag-guarded) — safe to call more than once without an intervening `fini()`. Any future test construction can rely on this instead of coordinating a single call site.
- **Task 1.3**: `makeJobsub()`/`makeJobstartinfo()` now return `-ENODEV` (not a generic `-EINVAL`) when the interning table isn't initialized — if a later task's error handling branches on `job_init()`'s return value, `-ENODEV` is a distinct, checkable signal for "table not ready" vs. other construction failures.
- **Task 3.3**: `apply_wal_entry()` (persist.c:2574) is dead code in production — confirmed via exhaustive grep (zero call sites outside its own declaration/definition/comments) and via `ha.c:8024`'s `ha_standby_advance_committed_and_apply` (dated 2026-07-10), whose own doc comment explicitly says standby entity materialization now goes through the SAME `apply_committed_entry()`/`persist_apply_pump_kick` pump the primary uses, not `apply_wal_entry()`. The persist.c:3722 comment claiming otherwise predates that refactor and was never updated (design.md corrected accordingly). Requirement 3.1/3.2/4.1 are fully delivered by task 3.2 alone. Task 3.3's code was kept (not discarded) as defensive parity with `WAL_JOB_SUBMIT`'s existing, also-dead case in the same switch — it is tested and correct, just not currently reachable in production. If `apply_wal_entry()` is ever revived or removed entirely in a future spec, this task's case should move or be deleted with it.