From ebfe4794f0c204412a8e325627512cfe64bd270d Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 29 Sep 2026 09:09:12 +0000 Subject: [PATCH] fix(scheduler): free jobs, schedobjs, sim decisions and the fcfs plugin at teardown (bug-1884, part 4) - kill_sched_job() (the job table's datakiller) frees the whole job: schedobj, parsed resreq, allocation, accounts and rusages; new kill_sched_obj() frees the schedobj's names, limit lists, per-primary wrappers and their pend reasons, and its last allocation/reason. - The simulator frees each decision struct after draining it (live and reconciled); the alloc/pend reason it points at stays the job's. - The builtin_fcfs heap plugin gets a fini_fn that also frees the plugin; builtin_fcfs_fini() still frees only the picker state (tests run it on stack plugins). - xce_sched_core_fini() frees the two pend_updates queues both drivers calloc(). Exit-time leak on the reproduction replay 11,426 -> 7,469 bytes. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/scheduler/include/sched/types.h | 1 + src/scheduler/src/core.c | 5 ++ src/scheduler/src/plugins/builtin_fcfs.c | 11 +++- src/scheduler/src/schedobj.c | 73 +++++++++++++++++++++++- src/scheduler/src/sim_loop.c | 4 ++ 5 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/scheduler/include/sched/types.h b/src/scheduler/include/sched/types.h index 25d3f880..a9ad3124 100644 --- a/src/scheduler/include/sched/types.h +++ b/src/scheduler/include/sched/types.h @@ -221,6 +221,7 @@ extern int sched_ctx_get_uncomit_shared(xvec_t**, xce_sched_ctx_t*); extern int sched_ctx_get_uncomit_host(xvec_t**, xce_sched_ctx_t*, int); extern int sched_ctx_uncommit_host(xce_sched_ctx_t*, int, xvec_idval_t*); extern void reset_sched_obj(xce_sched_obj_t*); +extern void kill_sched_obj(xce_sched_obj_t*); /* item killer for xce_sched_obj_t.hlimits: frees one per-worker xlist_t * (never its vres entries, which the limit plugin's instance table owns). */ extern void kill_hlimits_list(void*); diff --git a/src/scheduler/src/core.c b/src/scheduler/src/core.c index b86b3017..ec562ac5 100644 --- a/src/scheduler/src/core.c +++ b/src/scheduler/src/core.c @@ -215,6 +215,11 @@ void xce_sched_core_fini(xce_sched_core_t* core) dataq_fini(core->pend_updates[0]); dataq_fini(core->pend_updates[1]); + /* bug-1884: both drivers calloc() these (see xce_sched_core_init()) */ + free(core->pend_updates[0]); + free(core->pend_updates[1]); + core->pend_updates[0] = NULL; + core->pend_updates[1] = NULL; /* bug-1884: every registry xce_sched_core_init() makes. Plugins go * first -- their fini hooks may still read the entity tables. */ diff --git a/src/scheduler/src/plugins/builtin_fcfs.c b/src/scheduler/src/plugins/builtin_fcfs.c index 82fc58c0..1ca4a543 100644 --- a/src/scheduler/src/plugins/builtin_fcfs.c +++ b/src/scheduler/src/plugins/builtin_fcfs.c @@ -1062,13 +1062,22 @@ static xce_job_picker_iface_t builtin_fcfs_picker_iface = { * extraction: register_builtin_fcfs() below is unchanged in what it builds * and registers, just delegates the build step here. */ +/* bug-1884: the registry's fini_fn for the heap plugin built below. + * builtin_fcfs_fini() frees only the picker state, since callers also run + * it on plugins they own (tests use stack ones). */ +static void builtin_fcfs_plugin_fini(void* picker) +{ + builtin_fcfs_fini(picker); + FREEUP(picker); +} + xce_sched_plugin_t* builtin_fcfs_new_plugin(void) { static xce_sched_plugin_t builtin_fcfs_plugin_template = { .name = "builtin_fcfs", .picker_iface = &builtin_fcfs_picker_iface, .init_fn = builtin_fcfs_init_fn, - .fini_fn = (plugin_fini_fn_t)builtin_fcfs_fini, + .fini_fn = (plugin_fini_fn_t)builtin_fcfs_plugin_fini, .data = NULL }; xce_sched_plugin_t* plugin diff --git a/src/scheduler/src/schedobj.c b/src/scheduler/src/schedobj.c index bc812cce..fb024788 100644 --- a/src/scheduler/src/schedobj.c +++ b/src/scheduler/src/schedobj.c @@ -5,6 +5,8 @@ #include "controller.capnp.h" #include "kvmgr.h" +#include "misc.h" +#include "resreq.h" #include "sched/types.h" #include "scheduler.h" #include "utils.h" @@ -14,6 +16,67 @@ #include "xvec.h" #include "xstring.h" +/* kill_sched_obj - bug-1884: frees a job's schedobj: its interned names, + * limit lists, the per-primary/compound wrappers and their pend reasons, + * and the allocation/pend reason of its last attempt. The resreqs the + * wrappers point at belong to the job (job->resreq). */ +void kill_sched_obj(xce_sched_obj_t* obj) +{ + int i; + + if (obj == NULL) { + return; + } + + if (obj->user != NULL) { + dereference(obj->user); + } + if (obj->queue != NULL) { + dereference(obj->queue); + } + if (obj->project != NULL) { + dereference(obj->project); + } + + xlist_fini(&(obj->glimits)); + zarray_fini(&(obj->hlimits)); + + switch (obj->tag) { + case XCE_RESREQ_PRIMARY: + if (obj->u.primary != NULL) { + free_PendReason_ptr(&(obj->u.primary->reason)); + FREEUP(obj->u.primary); + } + break; + case XCE_RESREQ_COMPOUND: + if (obj->u.compound != NULL) { + free_PendReason_ptr(&(obj->u.compound->reason)); + FREEUP(obj->u.compound); + } + break; + case XCE_RESREQ_ALTER: + for (i = 0; i < obj->u.alters.count; i++) { + xce_compound_schedobj_t* cobj + = (xce_compound_schedobj_t*)xlist_get(&(obj->u.alters), i); + + if (cobj != NULL) { + free_PendReason_ptr(&(cobj->reason)); + free(cobj); + } + } + xlist_fini(&(obj->u.alters)); + break; + default: + break; + } + + free_JobAlloc_ptr(&(obj->alloc)); + free_PendReason_ptr(&(obj->reason)); + free(obj); +} + +/* kill_sched_job - datakiller of core->jobs (core.c). bug-1884: frees the + * whole job, not only its rusage report (Req 6 / bug-765 G-5). */ void kill_sched_job(void* arg) { xce_sched_job_t* job = (xce_sched_job_t*)arg; @@ -22,9 +85,15 @@ void kill_sched_job(void* arg) return; } - /* release the stored rusage report, if any (Req 6 / bug-765 G-5). - * free_JobRusages_ptr is NULL-safe and clears the field. */ free_JobRusages_ptr(&job->rusages); + kill_sched_obj(job->schedobj); + if (job->resreq != NULL) { + kill_resreq(job->resreq); + FREEUP(job->resreq); + } + free_JobAlloc_ptr(&(job->allocs)); + free_job_account(job->accounts); + free(job); } request_res_t* make_resource_request(char* res, float value) diff --git a/src/scheduler/src/sim_loop.c b/src/scheduler/src/sim_loop.c index 42e4515d..1812322f 100644 --- a/src/scheduler/src/sim_loop.c +++ b/src/scheduler/src/sim_loop.c @@ -1324,6 +1324,7 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, * site matters (bug-1610). */ sim_print_decision_gate(sim, d); xce_repl_check_decision(sim, d); + free(d); /* bug-1884: its alloc belongs to the job */ } } @@ -1698,6 +1699,9 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim, } sim_print_decision_gate(sim, d); xce_repl_check_decision(sim, d); + /* bug-1884: the decision struct is this loop's; its alloc + * or pend reason belongs to the job's schedobj */ + free(d); } if (sim->verify && (decoded_session != NULL)