lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ha): free liveness poll ctx on failed/skipped delaymgr schedule (bug-1483)

Browse source 
ha_arm_heartbeat_timeout_vtask published a calloc'd liveness_poll_ctx_t
into s_liveness_ctx unconditionally but only ever freed it via
liveness_poll_fn's cancelled-check path, which never runs if
delay_task_ms was never called (c==NULL, delaymgr not running) or itself
failed. Both no-schedule cases now free the ctx immediately, establishing
the invariant that s_liveness_ctx is non-NULL only when a delaymgr task
actually holds a live reference to it.

Regression test drives a real LOST PreVote round without stubbing
ha_prevote_rearm_liveness_hook (every other LOST-path test in the file
stubs it specifically to avoid this code path) and asserts the ctx isn't
left dangling.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXkenK8rqE8mdS1YKjsq78
This commit is contained in:
Rongsong Shen 2026-08-02 04:36:37 +00:00
parent b17d272f1d
commit f16733ee61
2 changed files with 73 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

29
src/controller/ha.c
View file

@ -8856,6 +8856,17 @@ int primary_heartbeat_fn_for_test(cluster_t *c)
{
return primary_heartbeat_fn(c);
}
/*
* liveness_ctx_get_for_test: expose the current s_liveness_ctx pointer (as an
* opaque void*, no type leaked to callers) so a test can assert whether a
* ctx is published as "active" without a live delaymgr task actually
* servicing it (bug-1483 regression lock). Only compiled when XCE_TESTING.
*/
void *liveness_ctx_get_for_test(void)
{
return (void *)atomic_load_explicit(&s_liveness_ctx, memory_order_acquire);
}
#endif
void ha_arm_heartbeat_timeout_vtask(ha_state_t *ha, cluster_t *c)
@ -8898,8 +8909,24 @@ void ha_arm_heartbeat_timeout_vtask(ha_state_t *ha, cluster_t *c)
if (c && c->delaymgr.running) {
int rc = delay_task_ms(&c->delaymgr, (int)poll_ms, 1,
(delay_task_entry_fn_t)liveness_poll_fn, ctx);
if (rc < 0)
if (rc < 0) {
WARN("HA: ha_arm_heartbeat_timeout_vtask: delay_task_ms failed: %d", rc);
/* Scheduling failed -- no delaymgr task holds ctx, so
* liveness_poll_fn's cancelled-check free will never run for it.
* Free it now instead of leaving it published as "active"
* (bug-1483). */
atomic_store_explicit(&s_liveness_ctx, NULL, memory_order_release);
free(ctx);
}
} else {
/* No live delaymgr to poll on (c==NULL, e.g. unit-test mode driving
* this path directly via ha_prevote_rearm_liveness with no live
* cluster_t; or delaymgr not running). liveness_poll_fn's own
* cancelled-check free is the ONLY place a scheduled ctx ever gets
* freed, so an unscheduled ctx would otherwise leak permanently
* (bug-1483). Free it here instead. */
atomic_store_explicit(&s_liveness_ctx, NULL, memory_order_release);
free(ctx);
}
}

45
src/controller/testsuite/test_ha_prevote.c
View file

@ -63,6 +63,12 @@ extern void ha_prevote_round_timeout_fire_for_test(ha_state_t *ha, cluster_t *c)
* LOST outcome without a live delaymgr. */
extern int (*ha_prevote_rearm_liveness_hook)(ha_state_t *ha, cluster_t *c);
/*
* liveness_ctx_get_for_test: opaque accessor for ha.c's static s_liveness_ctx
* (bug-1483 regression lock — see test below).
*/
extern void *liveness_ctx_get_for_test(void);
/* ── Task 5.1 transport-route seams ──────────────────────────────────────── */
/*
@ -280,6 +286,44 @@ static void test_prevote_round_timeout_is_loss(void)
printf("test_prevote_round_timeout_is_loss: PASS\n");
}
/* ── Test 4b: LOST round with no test hook must not leak the liveness ctx
* (bug-1483) ──────────────────────────────────────────────────────────── */
/*
* Every other LOST-path test in this file stubs ha_prevote_rearm_liveness_hook
* to avoid driving the real ha_arm_heartbeat_timeout_vtask(ha, c) with c==NULL
* — that unstubbed shape is exactly what bug-1483 found: the real function
* callocs a liveness_poll_ctx_t and publishes it to the static s_liveness_ctx,
* but with c==NULL there is no delaymgr to schedule liveness_poll_fn on, so
* nothing ever frees it — a permanent leak/dangling-publish on every such
* call. This test deliberately does NOT stub the hook, driving the real path,
* and asserts the ctx is never left published once arming without a
* scheduler is impossible.
*/
static void test_bug1483_lost_round_without_hook_does_not_leak_liveness_ctx(void)
{
ha_state_t ha;
TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &g_cfg, 0));
ha_do_demote(&ha);
make_three_node(&ha);
/* Deliberately leave ha_prevote_rearm_liveness_hook == NULL so the LOST
* branch calls the real ha_arm_heartbeat_timeout_vtask(&ha, NULL). */
TEST_ASSERT_NULL_MESSAGE(ha_prevote_rearm_liveness_hook,
"precondition: hook must be unstubbed to drive the real path");
ha_start_prevote_async(&ha, NULL);
ha_prevote_round_timeout_fire_for_test(&ha, NULL);
TEST_ASSERT_NULL_MESSAGE(liveness_ctx_get_for_test(),
"bug-1483: arming the liveness watchdog with c==NULL (no delaymgr "
"to poll on) must not leave an allocated ctx published as the "
"\"active\" s_liveness_ctx -- nothing would ever free it");
ha_fini(&ha);
printf("test_bug1483_lost_round_without_hook_does_not_leak_liveness_ctx: PASS\n");
}
/* ── Test 5: a subsequent genuine failure still triggers a new round ─────── */
/*
@ -876,6 +920,7 @@ void test_ha_prevote(void)
RUN_TEST(test_prevote_majority_grant_starts_real_election);
RUN_TEST(test_prevote_no_majority_stays_standby_and_rearms);
RUN_TEST(test_prevote_round_timeout_is_loss);
RUN_TEST(test_bug1483_lost_round_without_hook_does_not_leak_liveness_ctx);
RUN_TEST(test_prevote_reusable_after_loss);
RUN_TEST(test_prevote_pending_is_noop);
RUN_TEST(test_prevote_reply_over_real_transport_reaches_tally);