diff --git a/src/scheduler/src/core_events.c b/src/scheduler/src/core_events.c index 9c388bb8..edc40e8f 100644 --- a/src/scheduler/src/core_events.c +++ b/src/scheduler/src/core_events.c @@ -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); } } diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index 9abf8857..187722e3 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -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); } diff --git a/src/scheduler/testsuite/test_core_events.c b/src/scheduler/testsuite/test_core_events.c index fde03b4a..a4a342b6 100644 --- a/src/scheduler/testsuite/test_core_events.c +++ b/src/scheduler/testsuite/test_core_events.c @@ -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); diff --git a/src/scheduler/testsuite/test_job_update.c b/src/scheduler/testsuite/test_job_update.c index 3c5c09be..c262c2d2 100644 --- a/src/scheduler/testsuite/test_job_update.c +++ b/src/scheduler/testsuite/test_job_update.c @@ -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);