diff --git a/.wolf/anatomy-index.json b/.wolf/anatomy-index.json index 0c851dc4..a5ca4a85 100644 --- a/.wolf/anatomy-index.json +++ b/.wolf/anatomy-index.json @@ -1,13 +1,13 @@ { "version": 1, "meta": { - "lastScanned": "2026-09-21T11:41:46.809Z", - "fileCount": 670, + "lastScanned": "2026-09-22T03:01:53.188Z", + "fileCount": 671, "hits": 0, "misses": 0, - "renderedHash": "9704b1ab4907098decf513d0818bd0fb1c4f301de23b9de49ce6882672c6da25", - "storeUpdatedAt": "2026-09-21T11:41:46.817Z", - "rootHash": "e3297d48be0b28ab" + "renderedHash": "02614f3b797e44059a670011f293e3047a15c20c2c31f6bcec3e18336f1f4f2d", + "storeUpdatedAt": "2026-09-22T03:01:53.196Z", + "rootHash": "e7f2dc076a00fe7b" }, "files": { ".agent-shell/transcripts/2026-08-11-11-39-59.md": { @@ -10457,11 +10457,11 @@ }, "src/controller/ha.c": { "description": "define _GNU_SOURCE", - "tokens": 127669, - "hash": "257355b271b99422", - "size": 483416, - "mtimeMs": 1789537740512.282, - "updatedAt": "2026-09-16T05:49:00.563Z", + "tokens": 127991, + "hash": "9278ac07e8071f45", + "size": 484628, + "mtimeMs": 1790046113136.1223, + "updatedAt": "2026-09-22T03:01:53.188Z", "source": "hook" }, "src/include/request.h": { @@ -10502,11 +10502,11 @@ }, "src/controller/testsuite/test_ha_liveness.c": { "description": "Declares directly", - "tokens": 25885, - "hash": "183986d90eef9326", - "size": 99484, - "mtimeMs": 1789534901696.1792, - "updatedAt": "2026-09-16T05:01:41.736Z", + "tokens": 26012, + "hash": "c020980138115d54", + "size": 99964, + "mtimeMs": 1790045648548.2148, + "updatedAt": "2026-09-22T02:54:08.589Z", "source": "hook" }, "src/controller/testsuite/test_ha_heartbeat_committed.c": { @@ -12252,6 +12252,15 @@ "mtimeMs": 1789989591378.6973, "updatedAt": "2026-09-21T11:19:51.419Z", "source": "hook" + }, + "src/controller/testsuite/test_ha_vep_teardown.c": { + "description": "", + "tokens": 8206, + "hash": "371d908c0b62aafa", + "size": 31661, + "mtimeMs": 1790044881003.8472, + "updatedAt": "2026-09-22T02:41:21.052Z", + "source": "hook" } } } \ No newline at end of file diff --git a/.wolf/anatomy.md b/.wolf/anatomy.md index 451c08d4..0e057b9c 100644 --- a/.wolf/anatomy.md +++ b/.wolf/anatomy.md @@ -1,7 +1,7 @@ # anatomy.md -> Auto-maintained by OpenWolf. Last scanned: 2026-09-21T11:41:46.809Z -> Files: 670 tracked | Anatomy hits: 0 | Misses: 0 +> Auto-maintained by OpenWolf. Last scanned: 2026-09-22T03:01:53.188Z +> Files: 671 tracked | Anatomy hits: 0 | Misses: 0 ## ./ @@ -884,7 +884,7 @@ ## src/controller/ - `ctrlstatus.c` — include "config.h" (~4588 tok) -- `ha.c` — define _GNU_SOURCE (~127669 tok) +- `ha.c` — define _GNU_SOURCE (~127991 tok) - `ha.h` — ifndef HA_H (~34444 tok) - `meson.build` (~3405 tok) @@ -895,9 +895,10 @@ - `test_ctrlstatus.c` — include (~12594 tok) - `test_e2e_capnp.c` — include (~20187 tok) - `test_ha_heartbeat_committed.c` — include (~5775 tok) -- `test_ha_liveness.c` — Declares directly (~25885 tok) +- `test_ha_liveness.c` — Declares directly (~26012 tok) - `test_ha_partition.c` (~5500 tok) - `test_ha_role.c` — include (~13998 tok) +- `test_ha_vep_teardown.c` (~8206 tok) - `test_rcfile_integration.sh` — test_rcfile_integration.sh — xce-test-rc-file integration test, shell-driven (~4117 tok) ## src/harness/ diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 3ca4402d..32f782af 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -24872,6 +24872,62 @@ ], "occurrences": 1, "last_seen": "2026-09-21T02:40:00Z" + }, + { + "id": "bug-1839", + "timestamp": "2026-09-22T02:30:00Z", + "error_message": "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).", + "file": "src/controller/ha.c", + "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.", + "tags": [ + "controller", + "ha", + "standby-endpoint", + "err-handler", + "null-deref", + "vtask-pool", + "peer-ctrl-dispatch", + "vote-ep-teardown", + "related-to-bug-1197", + "related-to-bug-1298", + "live-crash", + "user-reported", + "fixed", + "tdd" + ], + "related_bugs": [ + "bug-1197", + "bug-1298" + ], + "occurrences": 1, + "last_seen": "2026-09-22T02:30:00Z" + }, + { + "id": "bug-1840", + "timestamp": "2026-09-22T02:35:00Z", + "error_message": "Live 3-node HA cluster crash (same soak run as bug-1839): xce_scheduler SIGABRT, glibc `malloc_consolidate(): unaligned fastbin chunk detected`, surfacing during an OpenSSL TLS handshake's MAC-buffer growth: abort -> malloc_printerr -> malloc_consolidate -> _int_malloc -> __libc_malloc2 -> CRYPTO_malloc -> CRYPTO_clear_realloc -> BUF_MEM_grow_clean -> mem_write -> bwrite_conv -> BIO_write -> ssl3_finish_mac -> tls_get_message_body -> state_machine -> handle_ssl_handshake (netio.c:548) -> io_handler (netio.c:588) -> io_worker_dispatch_batch (netio.c:648) -> io_worker (netio.c:689).", + "file": "unknown -- heap corruption detected far from its cause", + "root_cause": "NOT YET DIAGNOSED -- heap corruption detected far from its cause; a corrupted fastbin chunk was only found during an unrelated later malloc_consolidate() call, inside an OpenSSL TLS handshake on the scheduler's io_worker thread. The actual out-of-bounds write that corrupted the allocator's freelist could have happened in any thread, any amount of time earlier, and is not reconstructable from a non-ASAN build.dbg core dump -- there is no shadow memory, no poisoning, nothing to localize it to a specific allocation or write site. Confirmed directly via gdb against the real core file (/tmp/core.xce_scheduler, build cd9ecb41): the crashing frames' locals (malloc_consolidate's av/fb/p) are all optimized out / no debug symbols available for glibc or libcrypto/libssl in this environment, so there is nothing further to extract from the core itself beyond the symbol-only backtrace already captured. Needs an ASAN reproduction of the same 3-node HA deploy soak scenario to catch the actual out-of-bounds write with a real stack trace.", + "fix": "NOT FIXED -- awaiting an ASAN repro. Recommended next step: `/xce-build scheduler asan` (or `bash src/tools/xce-build.sh scheduler asan`), then rerun the same HA deploy scenario that produced /tmp/core.xce_scheduler until ASAN aborts at the real corruption site, which will pinpoint the actual overflow/write instead of just its downstream symptom.", + "tags": [ + "scheduler", + "heap-corruption", + "malloc-consolidate", + "netio", + "ssl-handshake", + "live-crash", + "user-reported", + "needs-asan-repro", + "not-fixed", + "unfixed", + "needs-diagnosis" + ], + "related_bugs": [ + "bug-1839" + ], + "occurrences": 1, + "last_seen": "2026-09-22T02:35:00Z" } ] } diff --git a/.wolf/memory.md b/.wolf/memory.md index 7134da06..b6c01887 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -5626,3 +5626,11 @@ | 04:26 | Edited src/scheduler/src/sim_loop.c | added 1 condition(s) | ~128 | | 04:33 | Edited src/scheduler/src/sim_loop.c | modified sim_reconcile_invoke_on_dispatch() | ~90 | | 04:41 | Edited .kiro/specs/scheduler-sim-eager-reconciliation/design.md | modified Correction() | ~557 | +| 19:40 | Edited src/controller/testsuite/test_ha_vep_teardown.c | 4→9 lines | ~159 | +| 19:41 | Edited src/controller/testsuite/test_ha_vep_teardown.c | added 1 condition(s) | ~2024 | +| 19:41 | Edited src/controller/testsuite/test_ha_vep_teardown.c | poll_until_ge() → poll_until() | ~41 | +| 19:53 | Edited src/controller/ha.c | added 1 condition(s) | ~558 | +| 19:53 | Edited src/controller/testsuite/test_ha_liveness.c | 8→11 lines | ~178 | +| 19:53 | Edited src/controller/testsuite/test_ha_liveness.c | 4→4 lines | ~60 | +| 19:54 | Edited src/controller/testsuite/test_ha_liveness.c | 6→4 lines | ~69 | +| 20:01 | Edited src/controller/ha.c | modified XCE_TESTING() | ~235 | diff --git a/src/controller/ha.c b/src/controller/ha.c index 5a2aeb13..27556654 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -9462,14 +9462,14 @@ static void on_standby_connected(endpoint_t *ep) WARN("HA: peer[%d] reconnected — evicting stale duplicate outbound " "channel to keep exactly one live connection (Req 2.6)", sctx->peer_idx); - if (stale_same_peer->iomgr) - iomgr_unmonitor((iomgr_t *)stale_same_peer->iomgr, stale_same_peer); + /* bug-1839: standby_ep_err_handler_primary now owns the FULL + * teardown itself (mark tearing_down, iomgr_unmonitor if needed, + * defer the owner-ref drop via vote_ep_teardown_async so a same-ep + * peer-control vtask already queued/in-flight is not read after + * free) — it used to only free ep->data/null it, leaving the caller + * to iomgr_unmonitor + endpoint_unref manually. Do NOT also + * unmonitor/unref here: doing so double-drops the owner reference. */ (void)standby_ep_err_handler_primary(stale_same_peer); - /* err-handler set ep->data = NULL and freed sctx; bug-1298: drop the - * owner ref — endpoint_unref reclaims outq/ssl/packets and frees the - * calloc'd struct when the last reference goes (deferred if a pool vtask - * still references this just-unmonitored ep). */ - endpoint_unref(stale_same_peer); #ifdef XCE_TESTING atomic_fetch_add(&ha_standby_evicted_count_for_test, 1); #endif @@ -9542,7 +9542,18 @@ static void on_standby_connected(endpoint_t *ep) */ static int standby_ep_err_handler_primary(void *ep_raw) { - endpoint_t *ep = (endpoint_t *)ep_raw; + endpoint_t *ep = (endpoint_t *)ep_raw; + + /* bug-1839: idempotency guard. Once this handler stops nulling ep->data + * synchronously (below), a second handle_error() callback for the same + * ep (e.g. a read error and a write error both observed before + * iomgr_unmonitor takes effect) must not re-enter vote_ep_teardown_async + * and double-drop the endpoint's owner reference. endpoint_mark_tearing_ + * down (inside vote_ep_teardown_async) sets this flag synchronously + * before returning, so a repeated call deterministically observes it. */ + if (endpoint_is_tearing_down(ep)) + return 0; + standby_ep_ctx_t *sctx = (standby_ep_ctx_t *)ep->data; if (!sctx) return 0; @@ -9566,8 +9577,15 @@ static int standby_ep_err_handler_primary(void *ep_raw) if (c) repl_remove_standby(&c->persist.repl, ep); - free(sctx); - ep->data = NULL; + /* bug-1839: do not free sctx / null ep->data synchronously here — a + * peer-control vtask for this same ep may already be queued or in + * flight (peer_ctrl_msg_handler_inline reads ep->data), same hazard + * vote_ep_err_handler's own comment documents for vote_ep_ctx_t just + * above. Defer to the same safe, per-ep-FIFO-serialized teardown + * vote_ep_teardown_async already uses for promoted vote-endpoint + * demotion (raft_cb_become_follower) — endpoint_fini's own + * FREEUP(ep->data) frees sctx once the last reference releases. */ + vote_ep_teardown_async(ha, ep); return 0; } diff --git a/src/controller/testsuite/test_ha_liveness.c b/src/controller/testsuite/test_ha_liveness.c index de046424..c86518be 100644 --- a/src/controller/testsuite/test_ha_liveness.c +++ b/src/controller/testsuite/test_ha_liveness.c @@ -1456,10 +1456,13 @@ static void test_reconnect_resolves_dual_channel_same_peer(void) "the stale same-peer channel must be evicted exactly once"); /* fresh is owned by standby_endpoints; tear it down the production way so - * LSan sees the heap ep + ctx reclaimed (the err handler frees both). */ + * LSan sees the heap ep + ctx reclaimed. bug-1839: the err handler now + * defers to vote_ep_teardown_async, which (ha.pool == NULL here, no live + * iomgr in this unit fixture) takes the documented no-pool inline + * fallback — endpoint_unref drops the sole owner ref straight to 0 and + * fini's + frees fresh itself, so no separate endpoint_fini/free call + * is needed (or safe) here anymore. */ standby_ep_err_handler_primary_for_test(fresh); - endpoint_fini(fresh); - free(fresh); fini_ha_primary_with_standby_eps(&c.ha); printf("test_reconnect_resolves_dual_channel_same_peer: PASS\n"); @@ -1515,9 +1518,9 @@ static void test_reconnect_resolves_dual_channel_promoted_vote_ep(void) atomic_load(&ha_standby_evicted_count_for_test), "the stale promoted-vote-ep channel must be evicted exactly once"); + /* bug-1839: see test_reconnect_resolves_dual_channel_same_peer's comment + * above — the err handler now owns fresh's full teardown itself. */ standby_ep_err_handler_primary_for_test(fresh); - endpoint_fini(fresh); - free(fresh); free(dummy_ectx); fini_ha_primary_with_standby_eps(&c.ha); @@ -1551,12 +1554,10 @@ static void test_reconnect_preserves_other_peer_channel(void) atomic_load(&ha_standby_evicted_count_for_test), "no eviction when the peer_idx differs"); + /* bug-1839: see test_reconnect_resolves_dual_channel_same_peer's comment + * above — the err handler now owns each ep's full teardown itself. */ standby_ep_err_handler_primary_for_test(peer1); - endpoint_fini(peer1); - free(peer1); standby_ep_err_handler_primary_for_test(peer2); - endpoint_fini(peer2); - free(peer2); fini_ha_primary_with_standby_eps(&c.ha); printf("test_reconnect_preserves_other_peer_channel: PASS\n"); diff --git a/src/controller/testsuite/test_ha_vep_teardown.c b/src/controller/testsuite/test_ha_vep_teardown.c index f78d83fc..b7c8d3fd 100644 --- a/src/controller/testsuite/test_ha_vep_teardown.c +++ b/src/controller/testsuite/test_ha_vep_teardown.c @@ -40,6 +40,11 @@ extern int peer_ctrl_msg_handler_for_test(endpoint_t *ep, io_packet_t *pkt); extern io_packet_t *repl_catchup_done_pkt_for_test(void); extern void raft_cb_become_follower_for_test(ha_state_t *ha, uint64_t term); +/* bug-1839: standby-endpoint error-handler deferred-teardown seams. */ +extern endpoint_t *standby_ep_ctx_build_for_test(endpoint_t *ep, ha_state_t *ha, + cluster_t *c, int peer_idx); +extern int standby_ep_err_handler_primary_for_test(endpoint_t *ep); + /* ── Fixtures ────────────────────────────────────────────────────────────── */ static ha_config_t g_cfg = { @@ -623,6 +628,167 @@ static void test_vep_self_teardown_does_not_deadlock(void) printf("test_vep_self_teardown_does_not_deadlock: PASS\n"); } +/* ── Test (e): bug-1839 — standby_ep_err_handler_primary defers teardown ── + * + * standby_ep_err_handler_primary (the err_handler for an outbound + * standby/heartbeat channel, ha_connect_prep_peer) used to free ep->data and + * null it out SYNCHRONOUSLY, on whatever thread handle_error() runs on, with + * zero coordination with the vtask pool — unlike vote_ep_err_handler / + * pre_vote_ep_err_handler (both of which explicitly leave vctx alone with a + * "peer_ctrl_msg_handler may still reference ep->data" comment). A live + * 3-node HA crash (bug-1839) hit exactly this: a peer-control vtask already + * queued for an ep read ep->data == NULL after this handler had raced ahead + * of it. The fix routes it through the SAME deferred, per-ep-FIFO-serialized + * teardown (vote_ep_teardown_async) that vote-ep demotion already uses — + * this test proves the ordering guarantee holds for THIS call site too, + * using a real standby_ep_ctx_t (not the generic vote_ep_ctx_t stand-in + * test (a) above uses) and the real handler entry point. + */ +static void test_standby_err_handler_defers_teardown_until_inflight_completes(void) +{ + reset_oracles(); + gate_init(); + + ha_state_t ha; + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_cfg, 1)); + + vtaskmgr_t pool; + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_init(&pool, 2)); + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_start(&pool)); + ha.pool = &pool; + + iomgr_t iomgr; + TEST_ASSERT_EQUAL_INT(0, iomgr_init(&iomgr)); + + endpoint_t *ep = endpoint_get(); + TEST_ASSERT_NOT_NULL(ep); + ep->iomgr = &iomgr; + ep->fd = -1; + TEST_ASSERT_NOT_NULL(standby_ep_ctx_build_for_test(ep, &ha, NULL, 0)); + + ha_set_peerctrl_dispatch_for_test(1); + ha_vep_teardown_probe = teardown_probe; + + /* Park an in-flight peer-control vtask on this ep (busy=1 while gated) — + * standing in for the real "peer_ctrl_dispatch_to_pool already enqueued + * a vtask for this ep" race the live crash hit. */ + enqueue_io_vtask(&pool, ep, (vtask_func_t)inflight_vtask_body); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, gate_wait_entered(2000), + "in-flight vtask must run and park (busy=1)"); + + /* The connection now hits a socket/SSL error — standby_ep_err_handler_ + * primary fires on this (test) thread, exactly as handle_error() would + * call it on the io_worker thread in production. */ + TEST_ASSERT_EQUAL_INT(0, standby_ep_err_handler_primary_for_test(ep)); + + /* While the in-flight vtask is still parked, the deferred teardown must + * NOT have run yet — ep->data must still be alive for anything else + * still queued behind it. */ + TEST_ASSERT_TRUE_MESSAGE(endpoint_is_tearing_down(ep), + "ep must be marked tearing_down once the err handler defers teardown"); + struct timespec ts = { .tv_sec = 0, .tv_nsec = 80 * 1000000L }; + nanosleep(&ts, NULL); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, atomic_load(&g_teardown_done), + "bug-1839: teardown must NOT run while an in-flight same-ep vtask " + "is parked — ep->data must not be freed out from under it"); + + /* Release the in-flight vtask; teardown becomes eligible and runs. */ + gate_open(); + TEST_ASSERT_EQUAL_INT_MESSAGE(0, poll_until(&g_teardown_done, 1, 2000), + "teardown must run once the in-flight vtask completes"); + + TEST_ASSERT_GREATER_THAN_INT_MESSAGE(atomic_load(&g_inflight_seq), + atomic_load(&g_teardown_seq), + "teardown_seq must be strictly after inflight_seq"); + + vtaskmgr_fini(&pool); + iomgr_fini(&iomgr); + ha_vep_teardown_probe = NULL; + ha_set_peerctrl_dispatch_for_test(0); + ha.pool = NULL; + ha_fini(&ha); + gate_destroy(); + printf("test_standby_err_handler_defers_teardown_until_inflight_completes: PASS\n"); +} + +/* ── Test (f): bug-1839 — idempotent against a repeated error callback ── + * + * handle_error() can plausibly fire standby_ep_err_handler_primary more than + * once for the same ep (e.g. a read error and a write error both observed + * before iomgr_unmonitor takes effect). Once the handler stops nulling + * ep->data synchronously, the old "if (!sctx) return 0" reentry guard no + * longer protects against this — a second call would otherwise re-enter + * vote_ep_teardown_async and double-drop the endpoint's owner reference. + * endpoint_mark_tearing_down sets ep->tearing_down synchronously, inside the + * FIRST call, before it returns — so by construction a second call made + * strictly after the first returns can deterministically observe the flag + * and bail, no gating/timing needed. + */ +static _Atomic int g_teardown_calls_1839 = 0; + +static void teardown_probe_count_1839(endpoint_t *ep) +{ + (void)ep; + atomic_fetch_add(&g_teardown_calls_1839, 1); +} + +static void test_standby_err_handler_idempotent_against_double_invocation(void) +{ + reset_oracles(); + atomic_store(&g_teardown_calls_1839, 0); + + ha_state_t ha; + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_cfg, 1)); + + vtaskmgr_t pool; + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_init(&pool, 2)); + TEST_ASSERT_EQUAL_INT(0, vtaskmgr_start(&pool)); + ha.pool = &pool; + + iomgr_t iomgr; + TEST_ASSERT_EQUAL_INT(0, iomgr_init(&iomgr)); + + endpoint_t *ep = endpoint_get(); + TEST_ASSERT_NOT_NULL(ep); + ep->iomgr = &iomgr; + ep->fd = -1; + TEST_ASSERT_NOT_NULL(standby_ep_ctx_build_for_test(ep, &ha, NULL, 0)); + + ha_set_peerctrl_dispatch_for_test(1); + ha_vep_teardown_probe = teardown_probe_count_1839; + + TEST_ASSERT_EQUAL_INT(0, standby_ep_err_handler_primary_for_test(ep)); + TEST_ASSERT_TRUE_MESSAGE(endpoint_is_tearing_down(ep), + "first call must mark tearing_down"); + + /* Second call, same ep, strictly after the first returns: ep is still + * alive (the teardown vtask has not necessarily run yet), but must be a + * pure no-op — no second teardown vtask, no double endpoint_unref. */ + TEST_ASSERT_EQUAL_INT_MESSAGE(0, standby_ep_err_handler_primary_for_test(ep), + "bug-1839: a repeated error callback on an already-tearing-down ep " + "must be a safe no-op, not a second teardown"); + + TEST_ASSERT_EQUAL_INT_MESSAGE(0, poll_until(&g_teardown_calls_1839, 1, 2000), + "the (single) deferred teardown must still run to completion"); + + /* Give any wrongly-enqueued second teardown vtask a chance to also run + * before asserting the count never exceeds 1 (ASAN would abort on the + * underlying double endpoint_unref well before this if the guard were + * missing, but assert the count explicitly too). */ + struct timespec ts = { .tv_sec = 0, .tv_nsec = 80 * 1000000L }; + nanosleep(&ts, NULL); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, atomic_load(&g_teardown_calls_1839), + "exactly one teardown vtask must ever run for this ep"); + + vtaskmgr_fini(&pool); + iomgr_fini(&iomgr); + ha_vep_teardown_probe = NULL; + ha_set_peerctrl_dispatch_for_test(0); + ha.pool = NULL; + ha_fini(&ha); + printf("test_standby_err_handler_idempotent_against_double_invocation: PASS\n"); +} + /* ── Entry point ─────────────────────────────────────────────────────────── */ void test_ha_vep_teardown(void) @@ -634,4 +800,6 @@ void test_ha_vep_teardown(void) RUN_TEST(test_vep_off_path_uses_plain_fini); RUN_TEST(test_vep_mark_drains_inflight_dispatch); RUN_TEST(test_vep_self_teardown_does_not_deadlock); + RUN_TEST(test_standby_err_handler_defers_teardown_until_inflight_completes); + RUN_TEST(test_standby_err_handler_idempotent_against_double_invocation); }