The oldest unresolved bug in this project's tracker (first sighted
2026-06-20): controller_unit_test intermittently hung for the full
300s meson timeout with no crash/ASAN report, never previously caught
live under gdb.
Live-reproduced this session (first attempt): thread 1 stuck forever
in commit_engine_fini() -> pthread_cond_destroy() (test_commit_barrier_
rollout.c's jap_teardown), while two fsync_worker threads sat parked
in pthread_cond_timedwait on the same persist_t. Root cause: several
test teardown functions called commit_engine_fini() BEFORE
persist_stop() -- the exact opposite of commit_engine_fini()'s own
documented contract. This let the live background fsync_worker thread
call back into commit_on_durable_advance() while commit_engine_fini()
was mid-destroy of the engine's lock/condvar -- a genuine TOCTOU race
with no synchronization between the two threads.
This is the same race already root-caused once for a SEGV symptom in
bug-1452 (2026-07-19, test_persist_truncate.c) and partially re-fixed
in bug-1687 (2026-08-25, cbrp_teardown) -- but never swept across all
affected files. Bug-1452's own remediation note explicitly flagged
test_demotion_convergence.c as a known, unfixed instance; that flag
went unactioned until now.
Fix: insert persist_stop() immediately before commit_engine_fini() at
21 confirmed-vulnerable sites across 12 files (test_apply_pump.c,
test_apply_pump_integration.c, test_commit_barrier_rollout.c,
test_commit_durability.c, test_commit_engine.c,
test_convergence_verification.c, test_demotion_convergence.c,
test_egress_filter.c, test_ha_role_lifecycle.c, test_job_array_fence.c,
test_job_array_replay_fence.c, test_p2_integration.c) -- the same
pattern bug-1452/1687 already established. persist_stop() is
idempotent (guarded by an atomic "already running" check), so each
fixture's own pre-existing persist_stop() call becomes a safe no-op.
No production code changed.
Known remaining exposure, documented but not fixed this session:
test_config_repl.c (~23 sites) and test_term_safety_adapter.c
(~5 sites) have the same ordering bug in their own teardown helpers,
flagged as follow-up.
Verified: full controller suite run 4 times total (once as part of
the fix, three further independent sequential runs) -- Ok:6 Fail:0
every time, 38-42s wall clock each run, no hang, nowhere near the
300s timeout. 1220/1220 individual controller_unit_test cases pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
persist_flush()'s synchronous fdatasync path advanced durable_lsn on
success but never called on_durable_advance() -- only the async
fsync_worker did. This gap was previously masked by bug-1563's own
busy-spin (a fast accidental repoll covered for the missing
notification); fixing that spin unmasked it, causing
test_durable_majority_commit_end_to_end to fail deterministically
(3/3 runs).
Fixed by firing on_durable_advance() from persist_flush() too,
whenever it genuinely advances durable_lsn. Fixed via TDD (RED test
before the fix, GREEN after) and re-validated with a fresh 40-round
sequential + chaotic HA soak (both pass cleanly, zero fdatasync
errors, zero divergence/loss).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Only 21 of 86 test_*.c group functions called their own nested
UNITY_BEGIN()/UNITY_END(). Each nested call reset Unity's global
NumberOfTests/TestFailures counters, so controller_test.c's final
`return UNITY_END();` -- the value meson test actually gates on --
only ever reflected whichever small window of tests ran since the
LAST nested reset, silently discarding pass/fail evidence for
everything before it (measured: 16 of 972 total tests in one run).
The same nested-BEGIN calls also caused PASS/FAIL banners to show
the wrong source file for tests in files that don't reset it
themselves (Unity.TestFile just carries over from whichever group
last called UNITY_BEGIN()).
Fix: replace every nested UNITY_BEGIN() with UnitySetTestFile(__FILE__)
(sets file attribution only, no counter reset) and drop the matching
UNITY_END(); add the same UnitySetTestFile(__FILE__) as the first
statement to the other group functions that had neither. The 2
genuinely standalone binaries (test_job_array_sched_push.c,
test_job_array_apiserver_push.c -- each has its own int main(), not
registered via controller_test.c's TEST() macro) are untouched.
Verified: single consolidated summary now reads
"972 Tests 0 Failures 0 Ignored OK", exactly matching the
ground-truth RUN_TEST call-site count across all 83 registered
group files (976 total minus 4 belonging to the 2 standalone
binaries). Every PASS/FAIL banner now names its own true source
file. Full xce-build.sh all regression pass clean (Ok: 9, Fail: 0).
No production code touched -- test-harness plumbing only.
Threads !persist_apply_resource_fenced(&c->persist) as a required
conjunct into both existing promotion-eligibility call sites in ha.c:
the periodic fence-release check (promotion_fence_timeout_fn) and the
election-time check (raft_cb_become_leader). A node with the fence
engaged is refused promotion at both points; once it self-clears
(task 4.2), the node is eligible again with no other action.
Task 4.3 of job-array-single-wal-record (Requirements 9.4, 9.5).
Add explicit test coverage for the DIVERGENT fence's stay-alive and
restart-only-clear guarantees (task 5.3), which surfaced a critical,
pre-existing safety gap: the sticky recovery_unrecoverable flag is
trigger-agnostic, and two pre-existing live call sites in ha.c
(promotion_fence_timeout_fn's periodic timer, and
raft_cb_become_leader's promotion-eligibility gate) clear it based
purely on structural conditions, never checking the apply verdict.
A node whose local apply dispatch diverges from the committed log,
but whose WAL bytes are otherwise structurally healthy, would have
its fence silently released by the live timer, then be eligible for
promotion to primary on a later election -- serving genuinely
divergent state.
Fix: gate both clear-call-sites on persist_apply_verdict(&c->persist)
!= APPLY_DIVERGENT in addition to their existing structural checks.
An apply-divergence-triggered fence can now only clear via a fresh
process restart's clean persist_recover() replay.
New tests: test_promotion_fence_apply_divergent_survives_structural_catchup
(test_commit_durability.c) covers the timer site; and
test_promotion_refused_when_apply_divergent_even_if_eligible
(test_convergence_verification.c) covers the promotion-gate site,
structurally isolated so it cannot pass for the wrong reason.
Closes out task-group 5 (restart apply-correctness verification).
On the standby replicated-apply path, ha_apply_single_entry wrote the entry via
persist_wal_write_replicated (which correctly set the self-computed CRC chain
link p->last_crc), then called apply_wal_entry with a SYNTHETIC header whose
crc=0. apply_wal_entry's `p->last_crc = hdr->crc` clobbered the chain link with
0, so every subsequent replicated write stamped prev_crc=0 on disk. The on-disk
CRC chain broke at the second replicated entry — invisible while live (standbys
serve in-memory state) but fatal on COLD RESTART: replay hit the break, returned
RECOVER_TRUNCATED, recovered only the config prefix, and lost all committed jobs
(joblist=0). The sticky truncated verdict then drove a promotion-fence
availability wedge (bug-777) when every survivor was truncated.
Fix — single ownership of the WAL header + CRC chain:
- apply_wal_entry is now LOGICAL entity-restore only: signature
(p, type, lsn, payload, payload_len); no header. It advances the current_lsn
high-water (monotonic) but NEVER touches p->last_crc.
- The physical layer is the sole owner of the CRC chain: persist_wal_write_-
replicated on the live path, replay_wal/persist_recover on recovery.
- ha_apply_single_entry drops the fabricated header and passes entry fields
directly. The persist_recover replay caller is updated to the new signature.
Verified: new RED->GREEN test test_replicated_apply_chain_survives_cold_restart
(drives the real ha_apply_wal_catchup_entry path, cold-replays -> RECOVER_CLEAN,
all N recovered); controller suite 0 failures; 3-node rolling-SIGKILL soak on a
real ext2/3 fs now passes 5/5 (was wedge+loss at round 3); full cold restart
recovers all 6 committed jobs (was 0) with CLEAN verdicts and a normal election.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per design invariant (a controller exits only on crash or admin kill), replace the
self-abort recovery terminal (tasks 3.3/4.3/4.4) with a loud, permanent, non-promotable
fence that stays alive and keeps retrying catch-up (self-heals on peer return; no manual
admin recovery). Records the never-self-terminate invariant as project-wide guidance in
src/controller/CLAUDE.md + .kiro/steering/ha-async-io.md.
