fix(scheduler): the normal-mode round frees each decision after publishing it (bug-1885)
xce_sched_driver_run_normal_mode_round() drained every decision, handed it to the recorder and the decision callback, and never freed the struct make_*_decision() allocates: 64 bytes per decision for the life of the scheduler. The struct is freed after the callback returns; its allocation or pend reason belongs to the job, and the callback and recorder copy what they keep. Test: test_round_frees_decision_after_callback (ASAN shadow check) RED before, GREEN after. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5b4d61180b
commit
d21b3d8f98
2 changed files with 87 additions and 2 deletions
|
|
@ -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));
|
||||
|
|
|
|||
|
|
@ -141,6 +141,10 @@
|
|||
|
||||
#include <unity/unity.h>
|
||||
|
||||
#if defined(__SANITIZE_ADDRESS__)
|
||||
#include <sanitizer/asan_interface.h>
|
||||
#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();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue