fix(scheduler): the simulator's threads stop when its run returns (bug-1632, bug-1763)

sched_sim_driver_run() started the picker and allocator threads on
&sim->core and returned with them still running; only
sched_sim_driver_fini() stopped them, and 24 test files (35 call sites)
run the driver on a stack-local sim without calling it. The leftover
threads kept a pointer into the dead frame, and ASAN occasionally caught
one reading it (stack-use-after-return in cbuf_get) in
sched_sim_verify_test and sched_sim_record_output_test.

sched_sim_driver_run() now wraps the loop and stops both threads on
every return (new sched_sim_driver_stop_threads(), shared with fini). If
the allocator fails to start, the running picker is stopped too. The
stop no longer pushes a jobid 0 to wake the allocator: cbuf_get/put wait
in pthread_cond_wait, a cancellation point, and the fake end-of-session
was visible to the allocator.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-09-30 02:47:24 +00:00
parent dc94da207a
commit cfa0a94166
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
4 changed files with 101 additions and 18 deletions

View file

@ -256,15 +256,26 @@ extern int sched_sim_driver_start(xce_sched_sim_driver_t* sim,
* marker is reached, Req 5 AC 20), or a negative errno: whatever
* sched_sim_driver_start() returned on a startup failure, or a pthread
* error standing up the picker-driving thread the event loop needs (see
* sim_loop.c's own file header comment for why one exists).
* sim_loop.c's own file header comment for why one exists). Whatever it
* returns, the picker and allocator threads it started have been stopped
* (bug-1632/bug-1763): `sim` may go out of scope right after.
*/
extern int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
const sched_sim_args_t* args);
/*
* sched_sim_driver_stop_threads - bug-1632/bug-1763: cancels and joins the
* picker and allocator threads sched_sim_driver_run() started (the way
* sched_fini() does in normal mode). sched_sim_driver_run() calls it on
* every return, so no thread outlives the run holding a pointer to `sim`
* (callers commonly keep it on the stack). A no-op when none are running.
*/
extern void sched_sim_driver_stop_threads(xce_sched_sim_driver_t* sim);
/*
* 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,
* that returned 0 left behind: stops the picker and allocator threads if
* still running (see sched_sim_driver_stop_threads), 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.

View file

@ -709,27 +709,36 @@ static void sim_free_alloc_wrappers_(xce_scheduler_t* wrappers[2])
}
}
void sched_sim_driver_stop_threads(xce_sched_sim_driver_t* sim)
{
void* res;
if ((sim == NULL) || !sim->allocator_thread_live) {
return;
}
/* Both threads block only in cbuf_get()/cbuf_put(), whose
* pthread_cond_wait() is a cancellation point (cbuf releases its mutex
* in a cleanup handler), so cancel + join needs no wake-up. A jobid 0
* pushed to wake the allocator would read as a real end-of-session. */
pthread_cancel(sim->core.jobpicker);
pthread_join(sim->core.jobpicker, &res);
pthread_cancel(sim->core.allocator);
pthread_join(sim->core.allocator, &res);
sim->allocator_thread_live = false;
}
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;
}
sched_sim_driver_stop_threads(sim);
if (sim->record_file != NULL) {
fclose(sim->record_file);

View file

@ -1379,8 +1379,8 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim,
* later, unrelated sim run in the same test binary) would read a
* dangling/reused pointer.
*/
int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
const sched_sim_args_t* args)
static int sim_driver_run_body(xce_sched_sim_driver_t* sim,
const sched_sim_args_t* args)
{
xce_core_event_t* buf_events = NULL;
replay_event_t* buf_decoded = NULL;
@ -1443,8 +1443,13 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
rc = pthread_create(&(sim->core.allocator), NULL, sched_allocator,
&(sim->core));
if (rc != 0) {
void* res;
ERROR("%s failed to start the simulation allocator thread: %s",
__func__, strerror(rc));
/* bug-1632: the picker is already running and would outlive `sim` */
pthread_cancel(sim->core.jobpicker);
pthread_join(sim->core.jobpicker, &res);
xce_core_set_active_for_logging(NULL);
return -rc;
}
@ -1795,3 +1800,20 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
return 0;
}
/*
* bug-1632/bug-1763: the picker and allocator threads the body starts hold
* &sim->core, and most callers keep `sim` on their stack and never call
* sched_sim_driver_fini() -- a thread left running read that frame after it
* was gone (ASAN stack-use-after-return in cbuf_get). The threads end with
* the run, on every return path.
*/
int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
const sched_sim_args_t* args)
{
int rc = sim_driver_run_body(sim, args);
sched_sim_driver_stop_threads(sim);
return rc;
}

View file

@ -1579,6 +1579,46 @@ static void test_verify_against_self_reports_zero_mismatches(void)
printf("test_verify_against_self_reports_zero_mismatches: PASS\n");
}
/*
* bug-1632/bug-1763: sched_sim_driver_run() must not return with its picker
* and allocator threads still running. Nearly every caller (this file's
* run_sim_and_capture_stdout() among them) runs the driver on a stack-local
* xce_sched_sim_driver_t and never calls sched_sim_driver_fini(); threads
* left running kept a pointer into that dead frame, and ASAN caught one
* reading it (stack-use-after-return in cbuf_get).
*/
static void test_run_returns_with_its_threads_stopped(void)
{
char path[PATH_MAX];
FILE* f;
sched_sim_args_t args;
xce_sched_sim_driver_t sim;
int rc;
f = write_header_and_snapshot(path, sizeof(path), "_threads");
TEST_ASSERT_NOT_NULL(f);
append_queue_update_event(f, 1000ULL, "normal");
append_jobs_push_event(f, 1500ULL, 777ULL, "normal");
append_replay_session_with_pend(f, 2000ULL, 777ULL, 0);
append_end_of_stream_marker(f);
fclose(f);
memset(&args, 0, sizeof(args));
args.replay_path = path;
args.sim_start_ts = SCHED_SIM_START_TS_LATEST;
rc = sched_sim_driver_run(&sim, &args);
TEST_ASSERT_EQUAL_INT(0, rc);
TEST_ASSERT_FALSE_MESSAGE(sim.allocator_thread_live,
"bug-1632: sched_sim_driver_run() returned with its picker and "
"allocator threads still running");
sched_sim_driver_fini(&sim);
printf("test_run_returns_with_its_threads_stopped: PASS\n");
}
/*
* Observable (b): replaying against a deliberately-altered copy (recorded
* main_reason flipped to 99, which the real picker thread can never
@ -2005,6 +2045,7 @@ int main(void)
test_verify_with_multiple_already_resolved_jobs_across_rounds_reports_no_mismatch);
RUN_TEST(
test_reconciliation_no_longer_masks_a_pend_jobs_dispatch_mismatch);
RUN_TEST(test_run_returns_with_its_threads_stopped);
return UNITY_END();
}