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

fix(scheduler): release interned queue resreq with dereference, not free (bug-1858)

Browse source 
q->resreq holds an interned makeString() value (pointer past the xstring
object header). core_upsert_queue() and sched_update_existing_queue()
released the old value with FREEUP()/free() when a queue update carried a
new resreq, an ASAN bad-free. Seen as HA soak replay-parity errors=1: the
sim replay of a snapshot-restored queue followed by a config push aborted
in core_upsert_queue.

Both sites now dereference() the old value (NULL-guarded). Regression
tests cover each site; the existing queue-update test had skipped the
resreq branch.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-24 14:21:20 +00:00
parent 441008cd30
commit 9e9f2798a0
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
4 changed files with 65 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

5
src/scheduler/src/core_events.c
View file

@ -380,7 +380,10 @@ static void core_upsert_queue(xce_sched_core_t* core, xce_sched_plugin_t* picker
q->sched_delay = updated->sched_delay;
if (updated->resreq != NULL) {
FREEUP(q->resreq);
/* interned makeString() value: release, never free() (bug-1858) */
if (q->resreq != NULL) {
dereference(q->resreq);
}
q->resreq = makeString(updated->resreq);
}
}

5
src/scheduler/src/schedobj.c
View file

@ -600,7 +600,10 @@ int sched_update_existing_queue(xce_scheduler_t* sched, xce_queue_t* updated)
q->sched_delay = updated->sched_delay;
if (updated->resreq != NULL) {
free(q->resreq);
/* interned makeString() value: release, never free() (bug-1858) */
if (q->resreq != NULL) {
dereference(q->resreq);
}
q->resreq = makeString(updated->resreq);
}

35
src/scheduler/testsuite/test_core_events.c
View file

@ -457,6 +457,40 @@ test_push_events_fires_each_hook_once_per_event_with_picker_idle(void)
TEST_ASSERT_EQUAL_PTR(&rusages, sched_get_job(&core, 42)->rusages);
}
/* bug-1858: a QUEUE_UPDATE carrying a resreq for an existing queue whose
* resreq is already set must dereference() the old interned makeString()
* value, not free() it (ASAN bad-free in core_upsert_queue, first seen
* replaying an HA-soak recording). */
static void
test_queue_update_replaces_existing_interned_resreq(void)
{
xce_sched_core_t core;
xce_core_event_t ev;
xce_queue_t wire_queue;
xce_sched_queue_t* q;
fixture_init(&core, &g_full_plugin);
memset(&wire_queue, 0, sizeof(wire_queue));
wire_queue.queue = "normal";
wire_queue.status = QUEUE_STAT_OPEN;
memset(&ev, 0, sizeof(ev));
ev.type = XCE_CORE_EVENT_QUEUE_UPDATE;
ev.u.queue_update.queue = &wire_queue;
/* create, then set a resreq, then replace it */
TEST_ASSERT_EQUAL_INT(0, xce_core_push_events(&core, &ev, 1));
wire_queue.resreq = "type == any";
TEST_ASSERT_EQUAL_INT(0, xce_core_push_events(&core, &ev, 1));
wire_queue.resreq = "type == local";
TEST_ASSERT_EQUAL_INT(0, xce_core_push_events(&core, &ev, 1));
q = sched_get_queue(&core, "normal");
TEST_ASSERT_NOT_NULL(q);
TEST_ASSERT_EQUAL_STRING("type == local", q->resreq);
}
/* JOBS_PUSH with njobs > 1: on_job_push must fire once PER JOB, not once
* per event -- Req 5 AC 4's "on_job_push(picker, job) for each job in the
* batch". */
@ -1849,6 +1883,7 @@ int main(void)
UNITY_BEGIN();
RUN_TEST(test_push_events_fires_each_hook_once_per_event_with_picker_idle);
RUN_TEST(test_jobs_push_fires_on_job_push_once_per_job);
RUN_TEST(test_queue_update_replaces_existing_interned_resreq);
RUN_TEST(test_jobs_push_duplicate_jobid_does_not_call_on_job_push_or_leak);
RUN_TEST(test_jobs_push_duplicate_jobid_with_newer_state_updates_existing_job);
RUN_TEST(test_jobs_push_duplicate_jobid_run_state_transfers_allocation);

22
src/scheduler/testsuite/test_job_update.c
View file

@ -350,6 +350,27 @@ test_update_existing_queue_real(void)
TEST_ASSERT_EQUAL_INT(3, q->sched_delay);
}
/* bug-1858: q->resreq is an interned makeString() instance, so replacing
* it must dereference() the old value -- a plain free() is an ASAN
* bad-free (pointer is past the xstring object header). */
static void
test_update_existing_queue_replaces_interned_resreq(void)
{
xce_scheduler_t sched;
fixture_init(&sched);
xce_sched_queue_t* q = make_test_queue("q1", 1);
q->resreq = makeString("type == any");
xce_queue_t upd;
memset(&upd, 0, sizeof(upd));
upd.queue = makeString("q1");
upd.resreq = makeString("type == local");
TEST_ASSERT_EQUAL_INT(0, sched_update_existing_queue(&sched, &upd));
TEST_ASSERT_EQUAL_STRING("type == local", q->resreq);
}
static void
test_update_unknown_queue_returns_enoent(void)
{
@ -416,6 +437,7 @@ int main(int argc, char* argv[])
RUN_TEST(test_rusage_unknown_job_returns_enoent, 0);
RUN_TEST(test_rusage_null_returns_einval, 0);
RUN_TEST(test_update_existing_queue_real, 0);
RUN_TEST(test_update_existing_queue_replaces_interned_resreq, 0);
RUN_TEST(test_update_unknown_queue_returns_enoent, 0);
RUN_TEST(test_update_queue_null_returns_einval, 0);
RUN_TEST(test_ctx_init_defaults_alloc_mode_to_normal, 0);