diff --git a/src/scheduler/src/driver.c b/src/scheduler/src/driver.c index a30b9f36..737be1bb 100644 --- a/src/scheduler/src/driver.c +++ b/src/scheduler/src/driver.c @@ -1070,6 +1070,11 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx) "op=%d", __func__, (int)decision->op); } + + /* bug-1885: the decision struct is the round's (make_*_decision()); + * its allocation or pend reason belongs to the job, and the + * callback and the recorder copy what they keep */ + free(decision); } plugin_end_session(&(sched->core)); diff --git a/src/scheduler/testsuite/test_driver_session_ordering.c b/src/scheduler/testsuite/test_driver_session_ordering.c index 97c8085b..20791675 100644 --- a/src/scheduler/testsuite/test_driver_session_ordering.c +++ b/src/scheduler/testsuite/test_driver_session_ordering.c @@ -141,6 +141,10 @@ #include +#if defined(__SANITIZE_ADDRESS__) +#include +#endif + #include "controller.capnp.h" #include "kvmgr.h" #include "preason.h" @@ -673,6 +677,10 @@ static void* picker_thread_fn(void* arg) /* test function itself can play this role sequentially). */ /* ---------------------------------------------------------------------- */ +/* bug-1885: when set, the stand-in emits one PEND decision carrying this + * (test-owned) reason before the end-of-session marker */ +static xce_pendreason_t* g_emit_pend_reason; + static void* stand_in_allocator_thread_fn(void* arg) { xce_sched_core_t* core = (xce_sched_core_t*)arg; @@ -683,6 +691,12 @@ static void* stand_in_allocator_thread_fn(void* arg) (void)jobid; /* always 0 here -- this fixture pushes no jobs */ log_event(EVT_ALLOCATOR_DRAINED_JBUF_SENTINEL); + if (g_emit_pend_reason != NULL) { + xce_decision_t* pend = make_pend_decision(g_emit_pend_reason); + + cbuf_put(&(core->decisions), &pend); + } + cbuf_put(&(core->decisions), &decision); log_event(EVT_ALLOCATOR_PUSHED_END_DECISION); @@ -695,15 +709,16 @@ static void* stand_in_allocator_thread_fn(void* arg) /* ---------------------------------------------------------------------- */ typedef struct { - int calls; + int calls; + xce_decision_t* last; } test_driver_ctx_t; static int test_on_decision(void* driver_ctx, xce_decision_t* decision) { test_driver_ctx_t* ctx = (test_driver_ctx_t*)driver_ctx; - (void)decision; ctx->calls++; + ctx->last = decision; return 0; } @@ -938,6 +953,70 @@ static void test_recorded_begin_round_equals_session_start(void) TEST_ASSERT_EQUAL_UINT64(sched.core.session_start, g_record_begin_ts); } +/* + * bug-1885: the round loop frees each decision struct once the decision + * callback has returned; the pend reason (or allocation) it points at is + * not the round's and stays valid. Checked through ASAN's shadow memory, + * so only meaningful in a sanitizer build. + */ +static void test_round_frees_decision_after_callback(void) +{ +#if defined(__SANITIZE_ADDRESS__) + xce_scheduler_t sched; + pthread_t picker_tid; + pthread_t allocator_tid; + test_driver_ctx_t driver_ctx; + xce_pendreason_t* reason; + int rc; + + memset(&g_events, 0, sizeof(g_events)); + memset(&sched, 0, sizeof(sched)); + memset(&driver_ctx, 0, sizeof(driver_ctx)); + + rc = xce_sched_core_init(&(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + lock(sched.core.plugins, RTTI_LOCK_WRITE); + rc = key_add(sched.core.plugins, (void*)g_stub_picker_plugin.name, + (void*)&g_stub_picker_plugin); + TEST_ASSERT_GREATER_OR_EQUAL(0, rc); + unlock(sched.core.plugins); + sched.core.job_pick_plugin = g_stub_picker_plugin.name; + + reason = make_pendreason(4242); + TEST_ASSERT_NOT_NULL(reason); + g_emit_pend_reason = reason; + + rc = xce_core_set_decision_cb(&(sched.core), test_on_decision, &driver_ctx); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = pthread_create(&picker_tid, NULL, picker_thread_fn, &(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + rc = pthread_create(&allocator_tid, NULL, stand_in_allocator_thread_fn, + &(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + + xce_sched_driver_run_normal_mode_round(&sched); + + TEST_ASSERT_EQUAL_INT(0, pthread_join(picker_tid, NULL)); + TEST_ASSERT_EQUAL_INT(0, pthread_join(allocator_tid, NULL)); + g_emit_pend_reason = NULL; + + TEST_ASSERT_EQUAL_INT(1, driver_ctx.calls); + TEST_ASSERT_NOT_NULL(driver_ctx.last); + TEST_ASSERT_TRUE_MESSAGE( + __asan_address_is_poisoned(driver_ctx.last), + "the round must free the decision struct after publishing it"); + TEST_ASSERT_FALSE_MESSAGE(__asan_address_is_poisoned(reason), + "the pend reason is not the round's to free"); + TEST_ASSERT_EQUAL_UINT64(4242, reason->jobid); + + free_PendReason_ptr(&reason); +#else + TEST_IGNORE_MESSAGE("needs an ASAN build"); +#endif +} + int main(void) { UNITY_BEGIN(); @@ -945,5 +1024,6 @@ int main(void) test_round_runs_plugin_begin_and_account_jobs_before_picker_signaled); RUN_TEST(test_null_driver_ctx_is_safe_noop); RUN_TEST(test_recorded_begin_round_equals_session_start); + RUN_TEST(test_round_frees_decision_after_callback); return UNITY_END(); }