fix(scheduler): tear down the simulator and the core's registries at exit (bug-1884, part 1)
A batch --sim replay under ASAN exited 1 on 17.2 MB of LeakSanitizer reports. This removes all but 75 KB: - sched_sim_driver_fini() (called by main after a clean run) cancels and joins the picker/allocator threads like sched_fini(), closes the replay, alternate-events and output files, and finalizes the core. The threads' ids now live in core.jobpicker/allocator, as in normal mode. - xce_sched_core_fini() destroys the plugin, job, worker, queue, user and project registries and the shared-resource arrays it creates. - The seqjob/parjob sched_make_allocs() stand-ins set up only lock, nextid and the aliased resmgr. Each ran a full xce_sched_core_init(), which also replaced the process-wide string and selector tables mid-startup. - plugin_unregister(), the plugin registry's datakiller, only runs fini_fn; it key_del()'d from the registry being destroyed and skipped plugins registered with plugin_register_core(). - The limit plugin's usage-instance table used kill_limit as datakiller on xce_vres_t records (bad free on destroy); kill_vres now frees the record. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5f8d6b5146
commit
9b61fd99e4
9 changed files with 169 additions and 54 deletions
|
|
@ -633,9 +633,14 @@ int instantiate_limit(xce_vres_t** pvres, var resmgr, xce_limit_t* limit,
|
|||
return rc;
|
||||
}
|
||||
|
||||
/* kill_vres - bug-1884: the usage-instance table's datakiller. Frees only
|
||||
* the record: its vres/res/percent_res belong to the resource manager and
|
||||
* its limit to the plugin's limit table. */
|
||||
void kill_vres(xce_vres_t* vres)
|
||||
{
|
||||
if (vres == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
FREEUP(vres);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -261,6 +261,16 @@ extern int sched_sim_driver_start(xce_sched_sim_driver_t* sim,
|
|||
extern int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
|
||||
const sched_sim_args_t* args);
|
||||
|
||||
/*
|
||||
* sched_sim_driver_fini - bug-1884: releases what a sched_sim_driver_run()
|
||||
* that returned 0 left behind: cancels and joins the picker and allocator
|
||||
* threads (the way sched_fini() does in normal mode), closes the replay,
|
||||
* alternate-events and output files, and finalizes the core. Call it only
|
||||
* after sched_sim_driver_run() returned 0; a failed run has either
|
||||
* finalized the core already (a startup failure) or is about to exit.
|
||||
*/
|
||||
extern void sched_sim_driver_fini(xce_sched_sim_driver_t* sim);
|
||||
|
||||
/*
|
||||
* sim_prescan_next_nonempty_session - user-requested (2026-09-11): looks
|
||||
* ahead in sim->replay_file (from its CURRENT read position) for the next
|
||||
|
|
|
|||
|
|
@ -215,6 +215,35 @@ void xce_sched_core_fini(xce_sched_core_t* core)
|
|||
dataq_fini(core->pend_updates[0]);
|
||||
dataq_fini(core->pend_updates[1]);
|
||||
|
||||
/* bug-1884: every registry xce_sched_core_init() makes. Plugins go
|
||||
* first -- their fini hooks may still read the entity tables. */
|
||||
if (core->plugins != NULL) {
|
||||
destroy(core->plugins);
|
||||
core->plugins = NULL;
|
||||
}
|
||||
if (core->jobs != NULL) {
|
||||
destroy(core->jobs);
|
||||
core->jobs = NULL;
|
||||
}
|
||||
if (core->workers != NULL) {
|
||||
destroy(core->workers);
|
||||
core->workers = NULL;
|
||||
}
|
||||
if (core->queues != NULL) {
|
||||
destroy(core->queues);
|
||||
core->queues = NULL;
|
||||
}
|
||||
if (core->users != NULL) {
|
||||
destroy(core->users);
|
||||
core->users = NULL;
|
||||
}
|
||||
if (core->projects != NULL) {
|
||||
destroy(core->projects);
|
||||
core->projects = NULL;
|
||||
}
|
||||
zarray_fini(&(core->sharedInst));
|
||||
zarray_fini(&(core->sharedInUse));
|
||||
|
||||
destroy(core->resmgr);
|
||||
|
||||
destroy(core->ugmgr);
|
||||
|
|
|
|||
|
|
@ -403,6 +403,7 @@ int main(int argc, char* argv[])
|
|||
return 1;
|
||||
}
|
||||
|
||||
sched_sim_driver_fini(&sim);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -209,24 +209,20 @@ int plugin_register_core(xce_sched_core_t* core, xce_sched_plugin_t* plugin)
|
|||
return rc;
|
||||
}
|
||||
|
||||
/* plugin_unregister - datakiller of core->plugins (core.c): runs as the
|
||||
* registry drops the entry, so it only finalizes the plugin, whose fini_fn
|
||||
* frees it. bug-1884: it used to key_del() the entry from
|
||||
* plugin->sched's registry -- the one being destroyed -- and skipped fini_fn
|
||||
* for a plugin registered with plugin_register_core() (plugin->sched NULL);
|
||||
* no registry was ever destroyed before, so that path never ran. */
|
||||
void plugin_unregister(void* arg)
|
||||
{
|
||||
xce_sched_plugin_t* plugin = (xce_sched_plugin_t*)arg;
|
||||
xce_scheduler_t* sched = NULL;
|
||||
|
||||
if (plugin == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
sched = (xce_scheduler_t*)plugin->sched;
|
||||
if (sched == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
lock(sched->core.plugins, RTTI_LOCK_WRITE);
|
||||
key_del(sched->core.plugins, plugin->name);
|
||||
unlock(sched->core.plugins);
|
||||
|
||||
if (plugin->fini_fn != NULL) {
|
||||
plugin->fini_fn(plugin);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,11 +26,13 @@ static int limit_priv_data_init(limit_plugin_data_t* data)
|
|||
.delta = 64,
|
||||
.datakiller = (destroy_fn_t)kill_limit,
|
||||
};
|
||||
/* bug-1884: holds xce_vres_t usage instances, not limits -- kill_limit
|
||||
* here freed a pointer inside a resource object on destroy. */
|
||||
kvmgr_params_t instances_params = {
|
||||
.key_type = KVMGR_KEY_STRING,
|
||||
.nslots = 1021,
|
||||
.delta = 64,
|
||||
.datakiller = (destroy_fn_t)kill_limit,
|
||||
.datakiller = (destroy_fn_t)kill_vres,
|
||||
};
|
||||
|
||||
data->limits = makeInstance(&xkvmgr_type, &limits_params);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
#include <errno.h>
|
||||
#include <pthread.h>
|
||||
#include <stdbool.h>
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
|
|
@ -7,6 +8,8 @@
|
|||
|
||||
#include "controller.capnp.h"
|
||||
#include "sim.h"
|
||||
#include "scheduler.h"
|
||||
#include "plugin.h"
|
||||
#include "utils.h"
|
||||
#include "xstring.h"
|
||||
|
||||
|
|
@ -681,3 +684,70 @@ int sched_sim_driver_start(xce_sched_sim_driver_t* sim,
|
|||
* wrapper, and sim.c stays scoped to Req 5 AC 13's startup sequence (this
|
||||
* file's own header comment). Its declaration is unchanged in sim.h.
|
||||
*/
|
||||
|
||||
/* bug-1884: the sched_make_allocs() stand-ins sim_alloc.c hangs off the
|
||||
* seqjob/parjob allocator plugins' `sched` field (only lock, nextid and an
|
||||
* aliased resmgr are set up). The plugins are freed with the core's plugin
|
||||
* registry, so take the wrappers before xce_sched_core_fini() and free them
|
||||
* after it. */
|
||||
static void sim_take_alloc_wrappers_(xce_sched_sim_driver_t* sim,
|
||||
xce_scheduler_t* out[2])
|
||||
{
|
||||
xce_sched_plugin_t* seq = (xce_sched_plugin_t*)sim->core.seqjob_allocator;
|
||||
xce_sched_plugin_t* par = (xce_sched_plugin_t*)sim->core.parjob_allocator;
|
||||
|
||||
out[0] = (seq != NULL) ? (xce_scheduler_t*)seq->sched : NULL;
|
||||
out[1] = (par != NULL) ? (xce_scheduler_t*)par->sched : NULL;
|
||||
}
|
||||
|
||||
static void sim_free_alloc_wrappers_(xce_scheduler_t* wrappers[2])
|
||||
{
|
||||
int i;
|
||||
|
||||
for (i = 0; i < 2; i++) {
|
||||
if (wrappers[i] != NULL) {
|
||||
mylock_fini(&(wrappers[i]->core.lock));
|
||||
free(wrappers[i]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void sched_sim_driver_fini(xce_sched_sim_driver_t* sim)
|
||||
{
|
||||
uint64_t jobid = 0;
|
||||
xce_scheduler_t* wrappers[2] = { NULL, NULL };
|
||||
void* res;
|
||||
|
||||
if (sim == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (sim->allocator_thread_live) {
|
||||
pthread_cancel(sim->core.jobpicker);
|
||||
pthread_join(sim->core.jobpicker, &res);
|
||||
|
||||
pthread_cancel(sim->core.allocator);
|
||||
/* try to wake up allocator */
|
||||
cbuf_tryput(&(sim->core.jbuf), &jobid);
|
||||
pthread_join(sim->core.allocator, &res);
|
||||
|
||||
sim->allocator_thread_live = false;
|
||||
}
|
||||
|
||||
if (sim->record_file != NULL) {
|
||||
fclose(sim->record_file);
|
||||
sim->record_file = NULL;
|
||||
}
|
||||
if (sim->alt_events_file != NULL) {
|
||||
fclose(sim->alt_events_file);
|
||||
sim->alt_events_file = NULL;
|
||||
}
|
||||
if (sim->replay_file != NULL) {
|
||||
fclose(sim->replay_file);
|
||||
sim->replay_file = NULL;
|
||||
}
|
||||
|
||||
sim_take_alloc_wrappers_(sim, wrappers);
|
||||
xce_sched_core_fini(&(sim->core));
|
||||
sim_free_alloc_wrappers_(wrappers);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,9 +68,10 @@
|
|||
* calls sched_get_id(sched) (`sched->core.lock`/`.nextid`, a fresh
|
||||
* allocation-id counter) -- both my_alloc()'s own honest `core` parameter
|
||||
* AND this narrow legacy need are satisfied by keeping a small,
|
||||
* long-lived (heap-allocated, never freed -- same accepted-leak convention
|
||||
* this file family already uses for its own never-joined threads)
|
||||
* throwaway `xce_scheduler_t` wrapper, whose `.core.resmgr` alone is
|
||||
* long-lived throwaway `xce_scheduler_t` wrapper (bug-1884: only `lock`,
|
||||
* `nextid` and `resmgr` are set up -- a full xce_sched_core_init() here
|
||||
* built a whole unused core and re-initialized the process-wide string and
|
||||
* selector tables -- and sched_sim_driver_fini() (sim.c) frees it), whose `.core.resmgr` alone is
|
||||
* ALIASED (a `var`, core.h -- a plain assignable reference, not an embedded
|
||||
* value) to simulation mode's own real resmgr, so sched_make_allocs()
|
||||
* resolves resource names correctly. This wrapper is set on `plugin->sched`
|
||||
|
|
@ -83,6 +84,34 @@
|
|||
* wrapper-based fix and outside this task's own Observable.)
|
||||
*/
|
||||
|
||||
/* sim_alloc_wrapper_new_ - the sched_make_allocs() stand-in described in
|
||||
* this file's header comment: only the three fields that function and
|
||||
* sched_get_id() read. */
|
||||
static xce_scheduler_t* sim_alloc_wrapper_new_(xce_sched_sim_driver_t* sim)
|
||||
{
|
||||
xce_scheduler_t* wrapper = (xce_scheduler_t*)calloc(1, sizeof(*wrapper));
|
||||
|
||||
if (wrapper == NULL) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
mylock_init(&(wrapper->core.lock));
|
||||
wrapper->core.nextid = 1;
|
||||
wrapper->core.resmgr = sim->core.resmgr;
|
||||
|
||||
return wrapper;
|
||||
}
|
||||
|
||||
static void sim_alloc_wrapper_free_(xce_scheduler_t* wrapper)
|
||||
{
|
||||
if (wrapper == NULL) {
|
||||
return;
|
||||
}
|
||||
|
||||
mylock_fini(&(wrapper->core.lock));
|
||||
free(wrapper);
|
||||
}
|
||||
|
||||
/*
|
||||
* sim_bootstrap_seqjob_allocator/sim_bootstrap_parjob_allocator - consumed
|
||||
* by sim_loop.c via a plain `extern` declared there, same "extern declared
|
||||
|
|
@ -106,7 +135,7 @@ void sim_bootstrap_seqjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
return;
|
||||
}
|
||||
|
||||
wrapper = (xce_scheduler_t*)calloc(1, sizeof(*wrapper));
|
||||
wrapper = sim_alloc_wrapper_new_(sim);
|
||||
if (wrapper == NULL) {
|
||||
ERROR("%s out of memory bootstrapping the sim-mode seqjob "
|
||||
"allocator plugin's sched_make_allocs() stand-in",
|
||||
|
|
@ -114,26 +143,12 @@ void sim_bootstrap_seqjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
return;
|
||||
}
|
||||
|
||||
rc = xce_sched_core_init(&(wrapper->core));
|
||||
if (rc < 0) {
|
||||
ERROR("%s failed to init the seqjob allocator sched_make_allocs() "
|
||||
"stand-in core (rc=%d)",
|
||||
__func__, rc);
|
||||
free(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
/* See this file's own header comment: aliases the SAME underlying
|
||||
* rtti-managed resmgr object, not a disconnected copy -- needed only
|
||||
* for my_alloc()'s own remaining sched_make_allocs() calls. */
|
||||
wrapper->core.resmgr = sim->core.resmgr;
|
||||
|
||||
plugin = seqjob_allocator_new_plugin();
|
||||
if (plugin == NULL) {
|
||||
ERROR("%s out of memory building the sim-mode seqjob allocator "
|
||||
"plugin instance",
|
||||
__func__);
|
||||
free(wrapper);
|
||||
sim_alloc_wrapper_free_(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -151,7 +166,7 @@ void sim_bootstrap_seqjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
"against the simulation core (rc=%d)",
|
||||
__func__, rc);
|
||||
free(plugin);
|
||||
free(wrapper);
|
||||
sim_alloc_wrapper_free_(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -172,7 +187,7 @@ void sim_bootstrap_parjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
return;
|
||||
}
|
||||
|
||||
wrapper = (xce_scheduler_t*)calloc(1, sizeof(*wrapper));
|
||||
wrapper = sim_alloc_wrapper_new_(sim);
|
||||
if (wrapper == NULL) {
|
||||
ERROR("%s out of memory bootstrapping the sim-mode parjob "
|
||||
"allocator plugin's sched_make_allocs() stand-in",
|
||||
|
|
@ -180,23 +195,12 @@ void sim_bootstrap_parjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
return;
|
||||
}
|
||||
|
||||
rc = xce_sched_core_init(&(wrapper->core));
|
||||
if (rc < 0) {
|
||||
ERROR("%s failed to init the parjob allocator sched_make_allocs() "
|
||||
"stand-in core (rc=%d)",
|
||||
__func__, rc);
|
||||
free(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
wrapper->core.resmgr = sim->core.resmgr;
|
||||
|
||||
plugin = parjob_allocator_new_plugin();
|
||||
if (plugin == NULL) {
|
||||
ERROR("%s out of memory building the sim-mode parjob allocator "
|
||||
"plugin instance",
|
||||
__func__);
|
||||
free(wrapper);
|
||||
sim_alloc_wrapper_free_(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -206,7 +210,7 @@ void sim_bootstrap_parjob_allocator(xce_sched_sim_driver_t* sim)
|
|||
"against the simulation core (rc=%d)",
|
||||
__func__, rc);
|
||||
free(plugin);
|
||||
free(wrapper);
|
||||
sim_alloc_wrapper_free_(wrapper);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1335,8 +1335,6 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
|
|||
int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
|
||||
const sched_sim_args_t* args)
|
||||
{
|
||||
pthread_t picker_tid;
|
||||
pthread_t allocator_tid;
|
||||
xce_core_event_t* buf_events = NULL;
|
||||
int buf_cap = 0;
|
||||
int buf_n = 0;
|
||||
|
|
@ -1382,12 +1380,11 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
|
|||
* Precondition (design.md's Service Interface): sched_resolve_job_picker()
|
||||
* must succeed once either thread's own loop needs it -- satisfied here
|
||||
* because sim_bootstrap_picker() (above) already ran before this point,
|
||||
* unchanged ordering from the pre-4.1 sequence. Like the picker thread
|
||||
* these replace, neither is ever joined or cancelled -- no sim-driver
|
||||
* teardown/fini function exists (this file's own header comment), the
|
||||
* same pre-existing gap normal mode does not have only because
|
||||
* sched_fini() exists there. */
|
||||
rc = pthread_create(&picker_tid, NULL, sched_job_picker, &(sim->core));
|
||||
* unchanged ordering from the pre-4.1 sequence. Their ids live in the
|
||||
* core, as in normal mode; sched_sim_driver_fini() (sim.c) cancels and
|
||||
* joins both the way sched_fini() does (bug-1884). */
|
||||
rc = pthread_create(&(sim->core.jobpicker), NULL, sched_job_picker,
|
||||
&(sim->core));
|
||||
if (rc != 0) {
|
||||
ERROR("%s failed to start the simulation picker thread: %s", __func__,
|
||||
strerror(rc));
|
||||
|
|
@ -1395,7 +1392,8 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
|
|||
return -rc;
|
||||
}
|
||||
|
||||
rc = pthread_create(&allocator_tid, NULL, sched_allocator, &(sim->core));
|
||||
rc = pthread_create(&(sim->core.allocator), NULL, sched_allocator,
|
||||
&(sim->core));
|
||||
if (rc != 0) {
|
||||
ERROR("%s failed to start the simulation allocator thread: %s",
|
||||
__func__, strerror(rc));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue