lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(scheduler): stop sim threads before releasing trace settings on quit (bug-1925)

Browse source 
repl_cmd_quit() (quit and EOF) released the trace settings while the
picker and allocator threads could still run; at a mid-session
breakpoint the allocator reads core->trace_ctl unlocked on every job
pop, a possible use-after-free just before exit. quit now cancels and
joins both threads first, as sched_sim_driver_run() does on return.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-08 15:44:53 +00:00
parent a1cb9cba4b
commit fcca4c9cfa
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 109 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

7
src/scheduler/src/repl.c
View file

@ -347,9 +347,16 @@ static void repl_print_help(void)
* sim->record_file (sim_loop.c) so a `quit` mid-session leaves any partial
* --sim-output write as durable as a normal completion would. Never
* returns.
*
* bug-1925: at a breakpoint mid-session the allocator keeps working and
* reads core->trace_ctl unlocked on every job pop, so the picker and
* allocator are stopped (cancel + join; both park only in cbuf_get/put,
* cancellation points; a no-op when none run) before
* repl_trace_shutdown() frees the ctl. Nothing after this needs them.
*/
static void repl_cmd_quit(xce_sched_sim_driver_t* sim)
{
sched_sim_driver_stop_threads(sim);
repl_trace_shutdown(sim); /* flush and close trace files (8.6) */
if (sim->record_file != NULL) {
fflush(sim->record_file);

3
src/scheduler/testsuite/meson.build
View file

@ -2863,6 +2863,9 @@ sched_repl_trace_wiring_test = executable(
sched_repl_trace_wiring_test_srcs,
include_directories:[incdir],
dependencies: [scheduler_deps, unity_dep],
# bug-1925: the wrap checks the quit/EOF path stops the picker and
# allocator threads before it releases the trace settings.
link_args: ['-Wl,--wrap=repl_trace_shutdown'],
install: false)
test('scheduler repl trace wiring test',

105
src/scheduler/testsuite/test_repl_trace_wiring.c
View file

@ -19,7 +19,8 @@
* - `set output` rejects a trace file still open for writing, including
* one kept open by the session in progress and a hard link to it, and
* leaves the output file unchanged (8.4);
* - quit and end of input shut tracing down (8.6);
* - quit and end of input shut tracing down (8.6), and only after the
* picker and allocator threads are stopped (bug-1925);
* - the continue-mode rollup reports a traced round on its own, with the
* range-flush idle rule, and never starts a collapsed range from it
* (7.3, 7.4);
@ -28,6 +29,7 @@
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@ -52,6 +54,13 @@ static char g_dir[64];
static char g_cwd[4096];
static int g_report_fd = -1;
/* bug-1925: what the first repl_trace_shutdown() call saw (-1: not called
* yet; 0: no thread live; 1: a picker/allocator thread still live), and how
* many stand-in threads have been cancelled. */
static int g_shutdown_saw_live = -1;
static int g_standins_started = 0;
static int g_standins_exited = 0;
/* stdout capture */
static int g_cap_fd = -1;
static int g_cap_saved = -1;
@ -66,6 +75,9 @@ void setUp(void)
memset(&g_sim, 0, sizeof(g_sim));
memset(&g_rs, 0, sizeof(g_rs));
g_shutdown_saw_live = -1;
g_standins_started = 0;
g_standins_exited = 0;
g_rs.units_mode = XCE_REPL_UNITS_BOTH;
g_rs.mode = XCE_REPL_MODE_STEP;
g_sim.interactive = true;
@ -460,10 +472,64 @@ static void test_set_output_rejects_file_kept_open_by_session(void)
TEST_ASSERT_FALSE(repl_trace_file_conflicts(&g_sim, "a.trace"));
}
/* ---- quit and end of input (8.6) ----------------------------------------- */
/* ---- quit and end of input (8.6, bug-1925) ------------------------------ */
/* Linked with -Wl,--wrap=repl_trace_shutdown: notes whether a picker or
* allocator thread was still live when the trace settings were released.
* The allocator reads core->trace_ctl unlocked on every job pop
* (sched_allocator()), so a live one could use the ctl after the free. */
void __real_repl_trace_shutdown(xce_sched_sim_driver_t* sim);
void __wrap_repl_trace_shutdown(xce_sched_sim_driver_t* sim)
{
if (g_shutdown_saw_live < 0) {
g_shutdown_saw_live =
(sim->allocator_thread_live
|| (__atomic_load_n(&g_standins_exited, __ATOMIC_SEQ_CST)
< g_standins_started))
? 1
: 0;
}
__real_repl_trace_shutdown(sim);
}
static void standin_exited(void* arg)
{
(void)arg;
__atomic_add_fetch(&g_standins_exited, 1, __ATOMIC_SEQ_CST);
}
/* Stands in for the picker or allocator of a session paused at the REPL:
* parked in a cancellation point, as the real ones are in cbuf_get/put. */
static void* standin_thread(void* arg)
{
(void)arg;
pthread_cleanup_push(standin_exited, NULL);
for (;;) {
pause();
}
pthread_cleanup_pop(0);
return NULL;
}
/* Gives the child a live picker and allocator, as a session paused at a
* breakpoint has (sim_loop.c sets allocator_thread_live once both run). */
static void start_standin_threads(void)
{
if ((pthread_create(&(g_sim.core.jobpicker), NULL, standin_thread, NULL)
!= 0)
|| (pthread_create(&(g_sim.core.allocator), NULL, standin_thread,
NULL)
!= 0)) {
_exit(98);
}
g_standins_started = 2;
g_sim.allocator_thread_live = true;
}
/* Runs in the child at exit(), after the quit path ran: reports whether
* tracing was shut down, then frees the fixture so the child exits clean. */
* tracing was shut down ('N' if not) and, with threads, whether they were
* stopped before it ('L' if one was still live), then frees the fixture so
* the child exits clean. */
static void report_at_exit(void)
{
char ok = ((g_rs.trace.next == NULL) && (g_rs.trace.cur == NULL)
@ -471,12 +537,15 @@ static void report_at_exit(void)
? 'Y'
: 'N';
if ((ok == 'Y') && (g_shutdown_saw_live != 0)) {
ok = 'L';
}
(void)!write(g_report_fd, &ok, 1);
close(g_report_fd);
fixture_fini();
}
static void assert_child_shuts_down(bool eof)
static void assert_child_shuts_down(bool eof, bool threads)
{
int fds[2];
pid_t pid;
@ -499,6 +568,9 @@ static void assert_child_shuts_down(bool eof)
g_report_fd = fds[1];
dup2(devnull, STDIN_FILENO);
dup2(devnull, STDOUT_FILENO);
if (threads) {
start_standin_threads();
}
atexit(report_at_exit);
if (eof) {
repl_command_loop(&g_sim); /* readline() hits EOF */
@ -513,12 +585,31 @@ static void assert_child_shuts_down(bool eof)
TEST_ASSERT_EQUAL_INT(pid, waitpid(pid, &status, 0));
TEST_ASSERT_TRUE(WIFEXITED(status));
TEST_ASSERT_EQUAL_INT(0, WEXITSTATUS(status));
TEST_ASSERT_NOT_EQUAL_MESSAGE(
'L', got, "trace settings released while threads were live");
TEST_ASSERT_EQUAL_CHAR_MESSAGE('Y', got, "tracing still active at exit");
}
static void test_quit_shuts_tracing_down(void) { assert_child_shuts_down(false); }
static void test_quit_shuts_tracing_down(void)
{
assert_child_shuts_down(false, false);
}
static void test_eof_shuts_tracing_down(void) { assert_child_shuts_down(true); }
static void test_eof_shuts_tracing_down(void)
{
assert_child_shuts_down(true, false);
}
/* bug-1925: `quit` at a breakpoint mid-session, picker and allocator live. */
static void test_quit_stops_threads_before_trace_shutdown(void)
{
assert_child_shuts_down(false, true);
}
static void test_eof_stops_threads_before_trace_shutdown(void)
{
assert_child_shuts_down(true, true);
}
/* ---- continue-mode rollup (7.3, 7.4) ------------------------------------- */
@ -667,6 +758,8 @@ int main(void)
RUN_TEST(test_set_output_rejects_file_kept_open_by_session);
RUN_TEST(test_quit_shuts_tracing_down);
RUN_TEST(test_eof_shuts_tracing_down);
RUN_TEST(test_quit_stops_threads_before_trace_shutdown);
RUN_TEST(test_eof_stops_threads_before_trace_shutdown);
RUN_TEST(test_rollup_traced_round_emitted_alone_after_flush);
RUN_TEST(test_rollup_traced_idle_round_reported_as_idle);
RUN_TEST(test_rollup_traced_pend_only_round_not_idle);