Rongsong Shen
d26ff8870d
The buglog was one 27K-line JSON file whose fields covered only symptom,
root cause and fix, as free text: 2% of entries named the code version a
bug was found on, 12% the build profile, 36% a way to reproduce, and 58% of
the file was OpenWolf hook auto-detections ("Missing guard clause",
"Function not marked async" on .md files).
- .claude/skills/buglog/bug-record.md: the record standard. Eight
sections (Summary as the title, Description, Environment, Reproduction;
Root cause, Fix, Verification, References once fixed), each with the
content it must contain -- e.g. Environment needs the OS, the libc the
failing binary loads (from its ELF interpreter: glibc 2.42 in the nix
shell vs 2.40 on the host), toolchain, build profile and commit.
- .wolf/bugs/<id>.md: 500 records migrated from buglog.json; every field
verified to round-trip. They carry legacy: true and an explicit "not
recorded" where the old log had no such field; component derived from
file paths, severity from keywords (else unknown), fix_commits from
hashes in status_note/fix that git resolves (91 of 463 fixed).
- .wolf/buglog-archive.json: the 710 hook auto-detections, verbatim.
- .wolf/buglog.json is now an index generated from the records, keeping
the fields OpenWolf's hooks and `openwolf bug search` read.
- scripts/buglog.py rewritten: new (template + Environment collected from
the machine and --binary), check, status (refuses a status the record
doesn't support; fixed needs Root cause, Fix, Verification and
--commit hashes git resolves), note, recur, set, show, list, triage,
env, index. 29 unittest cases in scripts/tests/test_buglog.py.
- scripts/git-hooks/pre-commit runs `check --staged`: incomplete staged
records, a legacy record hand-changed to fixed without the fix-time
sections, a new record claiming legacy: true, or a stale index refuse
the commit.
- scripts/gitea-sync.py reads the records: issue body = the record's
sections plus linked fix commits (origin remote, GITEA_COMMIT_REPO to
override), and a body hash so an issue's text is refreshed when the
record changes.
- bug-1866 rewritten to the full standard as the worked example.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
7.6 KiB
| id | status | status_updated | status_note | component | severity | timestamp | last_seen | occurrences | files | tags | related | legacy | gitea_issue | gitea_synced_status | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| bug-1839 | fixed | 2026-09-24 | controller | crash | 2026-09-22T02:30:00Z | 2026-09-22T02:30:00Z | 1 |
|
|
|
true | 71 | fixed |
bug-1839: Live 3-node HA cluster crash: xce_controller SIGSEGV in peer_ctrl_msg_handler_inline (ha.c:7035…
Description
Live 3-node HA cluster crash: xce_controller SIGSEGV in peer_ctrl_msg_handler_inline (ha.c:7035, ha_state_t *ha = vctx->ha;). gdb analysis of the actual core dump (build cd9ecb41) confirmed vctx == NULL (i.e. ep->data == NULL) while the endpoint itself was fully alive and in active use: ep->refcnt=191964, ep->tearing_down=0, ep->heap_alloc=1, ep->msg_handler=peer_ctrl_msg_handler, ep->err_handler=standby_ep_err_handler_primary, ep->fsm_out.state=FSM_STATE_ERROR. The crash fired on the vtask-pool path (vcpu_entry -> run_task -> peer_ctrl_vtask_entry -> peer_ctrl_msg_handler_inline); g_drive_async_peerctrl_dispatch_enabled defaults to 1 (ON, the production path).
Environment
Not recorded (migrated from buglog.json).
Reproduction
Not recorded (migrated from buglog.json).
Root cause
standby_ep_err_handler_primary (the err_handler for an outbound standby/heartbeat replication channel, registered in ha_connect_prep_peer) synchronously did free(sctx); ep->data = NULL; on whichever thread handle_error() called it from (the io_worker thread), with zero coordination with the vtask pool. If a peer-control vtask for the SAME ep was already queued or in flight (entirely plausible -- the endpoint is actively receiving replication ACKs/heartbeats via peer_ctrl_msg_handler -> peer_ctrl_dispatch_to_pool -> the vtask pool, and can independently hit a write/SSL error on that same connection), that vtask later ran peer_ctrl_msg_handler_inline, read ep->data as NULL, and crashed with a NULL dereference (not a use-after-free -- ep->data was explicitly nulled, not freed-and-reused memory). This is exactly the hazard this codebase already identified and fixed for the sibling vote_ep_ctx_t case: vote_ep_err_handler (ha.c:7787) and pre_vote_ep_err_handler (ha.c:7825) both explicitly do NOT free their context, with an inline comment ('Do NOT free vctx here -- peer_ctrl_msg_handler may still reference ep->data.'). standby_ep_err_handler_primary was never updated to the same discipline when the vtask-pool dispatch machinery (raft-drive-async, bug-1197/bug-1298) was introduced and retrofitted onto the other err handlers -- it slipped through because it's a distinct, older handler for the ongoing standby-heartbeat connection rather than the vote-time connection. Audited the other ~6 ep->data = NULL sites in ha.c (ha_connect_prep_peer/ha_connect_finish_peer/ha_connect_to_peer) and confirmed none of them share this bug class -- each frees an endpoint that either was never registered with iomgr at all, or whose iomgr_monitor() call itself just failed, so no vtask could ever have been dispatched against it.
Fix
FIXED (2026-09-22, TDD, user-reported live crash). Reused the established, already-tested vote_ep_teardown_async(ha, ep) mechanism (ha.c:7232) instead of inventing a new one: it calls endpoint_mark_tearing_down(ep) (sets the flag + drains dispatch_inflight, closing the TOCTOU window against peer_ctrl_msg_handler's late-event gate), iomgr_unmonitor() if monitored, then enqueues a VTASK_IO teardown task bound to this ep via vtask_enq_classified -- the per-endpoint FIFO guarantees this teardown vtask runs strictly after any earlier-queued or in-flight same-ep peer-control vtask. Its body (vote_ep_teardown_vtask_entry) is fully generic over what ep->data holds -- it just endpoint_unref()s the endpoint, and endpoint_fini()'s own FREEUP(ep->data) frees whatever ep->data currently points to once the last reference truly releases. standby_ep_err_handler_primary's free(sctx); ep->data = NULL; tail was replaced with a single vote_ep_teardown_async(ha, ep); call (its ha->standby_endpoints removal and repl_remove_standby bookkeeping stayed unchanged, since those only read sctx's still-valid fields before the deferred free). Added an if (endpoint_is_tearing_down(ep)) return 0; idempotency guard at the top of the handler: the old if (!sctx) return 0 accidentally doubled as a reentry guard (a second call after the first had already nulled ep->data would bail), but that protection disappears once ep->data is no longer nulled synchronously, and handle_error() firing this handler more than once for the same ep (e.g. a read error and a write error both observed before iomgr_unmonitor takes effect) would otherwise double-drop the endpoint's owner reference. Also found and fixed a second-order regression this exposed: on_standby_connected's stale-duplicate-channel eviction path (ha.c ~9461-9476) called standby_ep_err_handler_primary(stale_same_peer) and THEN did its own iomgr_unmonitor + endpoint_unref(stale_same_peer) afterward -- correct under the OLD contract (the handler used to only free ep->data, leaving ref-dropping to the caller) but now a double-unref/use-after-free once the handler owns the full teardown itself. Removed the caller's now-redundant iomgr_unmonitor/endpoint_unref calls; the err handler's own vote_ep_teardown_async call handles everything. Caught by ASAN (heap-use-after-free in endpoint_unref via on_standby_connected, immediately on the first full-suite rebuild after the primary fix) rather than by design review -- confirms the value of running the full suite, not just the new tests, after this kind of shared-contract change. Also updated 3 pre-existing tests in test_ha_liveness.c (test_reconnect_resolves_dual_channel_same_peer, test_reconnect_resolves_dual_channel_promoted_vote_ep, test_reconnect_preserves_other_peer_channel) that called standby_ep_err_handler_primary_for_test(ep) followed by a manual endpoint_fini(ep)/free(ep) -- with ha.pool == NULL in these unit fixtures, vote_ep_teardown_async now takes its documented no-pool inline fallback (WARN + endpoint_unref, refcnt 1->0, fini+free) inside the handler itself, so the tests' own manual cleanup became a double-free; removed it. TDD: two new tests in test_ha_vep_teardown.c using a real standby_ep_ctx_t (standby_ep_ctx_build_for_test) and the real standby_ep_err_handler_primary_for_test entry point (not the generic vote_ep_ctx_t stand-in test (a) in that file uses): test_standby_err_handler_defers_teardown_until_inflight_completes proves the deferred-teardown ordering (mirrors test_vep_teardown_runs_after_inflight's gate/sequence-ticket technique -- parks an in-flight same-ep vtask, calls the err handler, asserts teardown does NOT run while the vtask is parked, then releases it and asserts teardown runs strictly after); test_standby_err_handler_idempotent_against_double_invocation proves calling the handler twice in a row on the same (still-alive, tearing-down) ep is a safe no-op with exactly one teardown vtask ever running. RED confirmed against the pre-fix handler: both new tests failed with 'ep must be marked tearing_down...' (endpoint_is_tearing_down never became true, since the old code never called vote_ep_teardown_async at all) -- 1219 tests, 2 failures, zero other regressions. GREEN after the fix plus the on_standby_connected correction: full controller suite 1219/1219 Ok, zero failures.
Verification
Not recorded (migrated from buglog.json).
References
- Related: bug-1197, bug-1298
- Gitea: #71