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

fix(scheduler): mirror plugin_begin_session/plugin_end_session into sim-mode driver (bug-1681)

Browse source 
sim_loop.c never called plugin_begin_session()/plugin_end_session() at
its round boundary, unlike driver.c's normal-mode round -- a driver-
equivalence gap found by an /xce-audit-invariants scheduler pass.
Currently inert (sim mode's only reachable picker, builtin_fcfs, leaves
both hooks NULL) but structurally identical to bug-1613's class of bug.

Relocated plugin_begin_session()/plugin_end_session() from an implicit
xce_scheduler_t* to xce_sched_core_t* directly (mirroring
sched_account_jobs()'s prior relocation) -- their bodies never touched
anything beyond core->plugins, so driver.c's comment claiming they
"needed the full xce_scheduler_t*" was itself stale. sim_loop.c now
calls both, same relative position as driver.c. A full-tree grep (done
after the first build attempt failed on an untouched file) turned up a
third live call site in scheduler.c's legacy sched_begin_session()/
sched_end_session(), plus four testsuite files with their own stub
definitions of the same functions -- all updated to match the new
signature.

Extended test_sim_session_ordering.c's existing bug-1613 regression
tests with new plugin_begin_session()/plugin_end_session() spies
(call-count + session_active-window proof, same technique as the
existing sched_account_jobs spy). RED/GREEN independently verified:
temporarily disabled the two new call sites, confirmed both new
assertions fail (Expected 2 Was 0 / Expected 1 Was 0), restored,
reconfirmed green. Full scheduler suite: 51/52 green, the one failure
is the pre-existing unrelated bug-1560 sandbox flake.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AgYRU3zDTqNGV5Jk8aiKLg
This commit is contained in:
Rongsong Shen 2026-08-25 06:40:14 +00:00
parent 6d2668ced3
commit 6400f96d9d
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
12 changed files with 248 additions and 54 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/scheduler/include/core.h
View file

@ -413,10 +413,12 @@ extern int xce_core_set_decision_cb(xce_sched_core_t*, xce_core_decision_cb_t,
* state reset and the picker signal -- specifically
* plugin_begin_session()/sched_account_jobs() (the OLD per-plugin hook
* category and per-running-job resource-in-use accounting; both real,
* load-bearing production behavior, both xce_scheduler_t*-coupled and
* therefore outside this pure-core module's reach, same exclusion
* load-bearing production behavior). Both now take xce_sched_core_t*
* directly (sched_account_jobs() since scheduler-sim-decision-parity task
* 1.1, plugin_begin_session()/plugin_end_session() since bug-1681) but stay
* outside this pure-core module's reach regardless -- same exclusion
* rationale core_session.c's own file header comment documents for why
* core_begin_session_internal() never called them itself).
* core_begin_session_internal() never called them itself.
*
* This split exists to preserve a safety property main.c's pre-task-5.2
* `sched_begin_session()` + `sched_go()` sequence already guaranteed: the

19
src/scheduler/include/plugin.h
View file

@ -60,6 +60,12 @@ typedef struct {
* -- same idiom as cluster_t across myconf.h/persist.h/ha.h/cluster_def.h. */
typedef struct xce_job_picker_iface_s xce_job_picker_iface_t;
/* xce_sched_core_t is defined in core.h -- forward-declared here (re-declared
* again, harmlessly, near plugin_register_core()'s own declaration below)
* so plugin_begin_session()/plugin_end_session() can be declared against it
* ahead of that later block. */
typedef struct xce_sched_core_s xce_sched_core_t;
typedef struct {
char* name;
void* sched;
@ -89,8 +95,17 @@ extern int plugin_queue_update(void*, xce_sched_queue_t*);
extern int plugin_job_update(void*, xce_sched_job_t*);
extern int plugin_worker_update(void*, xce_sched_worker_t*);
extern int plugin_job_state_change(void*, xce_sched_job_t*, uint32_t);
extern int plugin_begin_session(void*);
extern int plugin_end_session(void*);
/* plugin_begin_session()/plugin_end_session() - relocated to xce_sched_core_t*
* (bug-1681, mirroring sched_account_jobs()'s identical scheduler-sim-
* decision-parity task 1.1 relocation): the per-plugin begin/end_session_fn
* iteration only ever touches core->plugins, never anything else on
* xce_scheduler_t* -- so, like plugin_register_core() below, these are
* callable from a driver with no xce_scheduler_t* (the simulation driver).
* See driver.c's xce_sched_driver_run_normal_mode_round() for the normal-
* mode call site and sim_loop.c's sched_sim_driver_run() for the mirrored
* simulation-mode one. */
extern int plugin_begin_session(xce_sched_core_t*);
extern int plugin_end_session(xce_sched_core_t*);
extern int plugin_commit_decision(void*, xce_decision_t*);
extern int plugin_prepare_allocation(void*, xce_sched_ctx_t*);
extern int plugin_job_election(void*, xce_sched_job_t*);

22
src/scheduler/src/core_session.c
View file

@ -119,17 +119,19 @@ static void core_reset_workers(xce_sched_core_t* core)
* shared/per-worker resource-value reset.
*
* Deliberately excludes two pieces of the real sched_begin_session():
* - plugin_begin_session(sched): fires the OLD per-plugin
* - plugin_begin_session(core): fires the OLD per-plugin
* begin_session_fn hook category (plugin.c), unrelated to the NEW
* picker interface this spec is about -- same exclusion rationale
* core_events.c's own header comment documents for JOB_UPDATE's
* plugin_job_update() fan-out ("the OLD per-plugin hook category,
* unrelated to the picker interface this event drives"). Also
* structurally unreachable without depending on xce_scheduler_t*
* (plugin_begin_session(void* arg) casts arg to xce_scheduler_t* and
* reads sched->core.plugins -- the exact "reuse only if genuinely
* core-scoped" test task 4.3's own precedent applies here too, and
* this one fails it).
* unrelated to the picker interface this event drives"). As of bug-1681
* it is now xce_sched_core_t*-scoped (relocated off xce_scheduler_t*,
* same as sched_account_jobs() below), so it is no longer structurally
* unreachable from here -- the exclusion is purely the "unrelated to
* the picker interface" scoping rationale above, not a type barrier.
* Both driver.c's xce_sched_driver_run_normal_mode_round() and
* sim_loop.c's sched_sim_driver_run() call it themselves, at their own
* per-round boundary, same as sched_account_jobs() below.
* - sched_account_jobs(sched) (per-running-job resource charging via
* sched_charge_account()): sched_charge_account() is exported with an
* xce_scheduler_t* signature (scheduler.h) and its real implementation
@ -318,9 +320,11 @@ static void core_gc_select_nodes(xce_sched_core_t* core)
* (core); // GC, reset workers`. Reproduces sched_end_session()'s
* core-scoped cleanup: select-eval-ctx teardown, then select-node GC.
*
* Excludes plugin_end_session(sched) for the same reason
* Excludes plugin_end_session(core) for the same reason
* core_begin_session_internal() excludes plugin_begin_session() above (OLD
* per-plugin hook category, xce_scheduler_t*-coupled).
* per-plugin hook category, unrelated to the picker interface -- see that
* comment for the bug-1681 type-coupling correction, which applies here
* identically).
*/
static void core_end_session_internal(xce_sched_core_t* core)
{

26
src/scheduler/src/driver.c
View file

@ -692,12 +692,14 @@ int xce_sched_driver_on_decision(void* driver_ctx, xce_decision_t* decision)
* builtin_fcfs) and sched_account_jobs() (re-charges every RUNNING job's
* resources after the resval reset zeroes them) BEFORE sched_go() signals
* the picker thread. xce_core_begin_session()/xce_core_end_session()
* (core_session.c, task 4.4) deliberately exclude both -- plugin_begin_session()
* needs the full xce_scheduler_t* (sched_account_jobs() no longer does, as
* of scheduler-sim-decision-parity task 1.1's relocation to xce_sched_core_t*,
* but this call site still holds an xce_scheduler_t* and passes &(sched->core)
* for it) -- so this function calls them itself, in the same slot: strictly
* AFTER xce_core_begin_session()'s resval reset, strictly BEFORE
* (core_session.c, task 4.4) deliberately exclude both -- both are
* xce_sched_core_t*-scoped (plugin_begin_session() relocated from
* xce_scheduler_t* to xce_sched_core_t* by bug-1681, mirroring
* sched_account_jobs()'s identical scheduler-sim-decision-parity task 1.1
* relocation -- neither ever touched anything on xce_scheduler_t* beyond
* &(sched->core)) but outside core_session.c's own pure-core module reach --
* so this function calls them itself, in the same slot: strictly AFTER
* xce_core_begin_session()'s resval reset, strictly BEFORE
* xce_core_signal_go() unblocks the picker. Neither plugin_begin_session()
* nor sched_account_jobs() has any data dependency on WHERE within that
* window it runs (verified: plugin_begin_session() only touches
@ -717,6 +719,14 @@ int xce_sched_driver_on_decision(void* driver_ctx, xce_decision_t* decision)
* calls xce_core_begin_session() concurrently), so it runs immediately
* before xce_core_end_session(), mirroring "plugin bookkeeping is part of
* ending the session."
*
* bug-1681 (driver-equivalence audit, 2026-08-24): sim_loop.c's
* sched_sim_driver_run() never called plugin_begin_session()/
* plugin_end_session() at all -- inert only because sim mode's picker
* registry (sim_bootstrap_picker(), sim_picker.c) never registers a plugin
* whose begin/end_session_fn touches real state, so it would have silently
* reactivated the moment one did. Now mirrored at sim_loop.c's own round
* boundary, same relative position as here.
*/
void xce_sched_driver_run_normal_mode_round(void* driver_ctx)
{
@ -747,7 +757,7 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx)
/* See this function's header comment: MUST run after the resval reset
* xce_core_begin_session() just performed, MUST complete before
* xce_core_signal_go() below. */
plugin_begin_session(sched);
plugin_begin_session(&(sched->core));
sched_account_jobs(&(sched->core));
/* Req 5 AC 10 (scheduler-core spec, task 9.2): "On core_go(): begin
@ -799,7 +809,7 @@ void xce_sched_driver_run_normal_mode_round(void* driver_ctx)
}
}
plugin_end_session(sched);
plugin_end_session(&(sched->core));
xce_core_end_session(&(sched->core));
/* Req 5 AC 10: "On core_end_session(): finalize the ReplaySession with

22
src/scheduler/src/plugin.c
View file

@ -367,17 +367,16 @@ int plugin_worker_update(void* arg, xce_sched_worker_t* worker)
return rc;
}
int plugin_begin_session(void* arg)
int plugin_begin_session(xce_sched_core_t* core)
{
xce_scheduler_t* sched = (xce_scheduler_t*)arg;
iterator_t it;
int rc;
iterator_t it;
int rc;
if (sched == NULL) {
if (core == NULL) {
return -EINVAL;
}
rc = iter_make(&it, sched->core.plugins);
rc = iter_make(&it, core->plugins);
if (rc < 0) {
return rc;
}
@ -404,17 +403,16 @@ int plugin_begin_session(void* arg)
return rc;
}
int plugin_end_session(void* arg)
int plugin_end_session(xce_sched_core_t* core)
{
xce_scheduler_t* sched = (xce_scheduler_t*)arg;
iterator_t it;
int rc;
iterator_t it;
int rc;
if (sched == NULL) {
if (core == NULL) {
return -EINVAL;
}
rc = iter_make(&it, sched->core.plugins);
rc = iter_make(&it, core->plugins);
if (rc < 0) {
return rc;
}

4
src/scheduler/src/scheduler.c
View file

@ -1303,7 +1303,7 @@ void sched_begin_session(xce_scheduler_t* sched)
select_eval_ctx_fini(sched->core.sel_eval_ctx);
sched->core.sel_eval_ctx = select_eval_ctx_init(sel_get_node_count());
plugin_begin_session(sched);
plugin_begin_session(&(sched->core));
sched_reset_shared_resval(sched);
sched_reset_workers(sched);
@ -1323,7 +1323,7 @@ void sched_end_session(xce_scheduler_t* sched)
select_eval_ctx_fini(sched->core.sel_eval_ctx);
sched->core.sel_eval_ctx = NULL;
plugin_end_session(sched);
plugin_end_session(&(sched->core));
/* Guard: GC requires the evaluation pipeline to be idle */
if (sched_eval_in_progress(sched) || !sched_staging_merge_complete(sched)) {

11
src/scheduler/src/sim_loop.c
View file

@ -947,6 +947,13 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
* sim_verify.c (Requirement 5.2). */
sim_reconcile_unaccounted_jobs(sim, decoded_session, record_ctx);
/* bug-1681: mirror driver.c's xce_sched_driver_run_normal_mode_round()
* -- plugin_begin_session() (OLD per-plugin begin_session_fn hook
* category) was never called here, unlike sched_account_jobs()
* just below. Same window requirement applies: strictly after
* xce_core_begin_session()'s resval reset, strictly before
* xce_core_signal_go() unblocks the picker. */
plugin_begin_session(&(sim->core));
sched_account_jobs(&(sim->core));
xce_core_signal_go(&(sim->core));
@ -998,6 +1005,10 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim,
free_ReplaySession_ptr(&decoded_session);
/* bug-1681: mirror driver.c's plugin_end_session(&(sched->core))
* call site -- runs immediately before xce_core_end_session(),
* same as normal mode. */
plugin_end_session(&(sim->core));
xce_core_end_session(&(sim->core));
xce_repl_post_session_pause(sim);

8
src/scheduler/testsuite/test_admin_request.c
View file

@ -166,15 +166,15 @@ int xce_core_end_session(xce_sched_core_t* core)
return 0;
}
int plugin_begin_session(void* arg)
int plugin_begin_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}
int plugin_end_session(void* arg)
int plugin_end_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}

8
src/scheduler/testsuite/test_driver_decision_cb.c
View file

@ -130,15 +130,15 @@ int xce_core_end_session(xce_sched_core_t* core)
return 0;
}
int plugin_begin_session(void* arg)
int plugin_begin_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}
int plugin_end_session(void* arg)
int plugin_end_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}

8
src/scheduler/testsuite/test_driver_translate.c
View file

@ -216,15 +216,15 @@ int xce_core_end_session(xce_sched_core_t* core)
return 0;
}
int plugin_begin_session(void* arg)
int plugin_begin_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}
int plugin_end_session(void* arg)
int plugin_end_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}

8
src/scheduler/testsuite/test_resval_ownership.c
View file

@ -221,15 +221,15 @@ int xce_core_end_session(xce_sched_core_t* core)
return 0;
}
int plugin_begin_session(void* arg)
int plugin_begin_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}
int plugin_end_session(void* arg)
int plugin_end_session(xce_sched_core_t* core)
{
(void)arg;
(void)core;
return 0;
}

158
src/scheduler/testsuite/test_sim_session_ordering.c
View file

@ -101,14 +101,39 @@
#include "xstring.h"
/* Pull in the real functions under test into this TU (white-box #include,
* same combined set test_sim_equivalence.c already establishes). */
* same combined set test_sim_equivalence.c already establishes). NOTE:
* ../src/plugin.c is deliberately NOT included here (unlike
* test_driver_session_ordering.c, which needs the real plugin_begin_session()/
* plugin_end_session() driven through a genuinely-registered observer
* plugin) -- this file spies plugin_begin_session()/plugin_end_session()
* instead (see "PLUGIN SESSION-HOOK SPY" section below), same rationale as
* its pre-existing sched_account_jobs() spy: this file's target is sim_loop.c's
* round-boundary CALL-SITE discipline (bug-1681), already independently
* proven to drive real per-plugin hook firing by test_driver_session_ordering.c
* for the normal-mode side; re-proving hook-firing here would be redundant
* AND is not practically constructible here anyway -- sim.core.plugins does
* not exist until sched_sim_driver_run()'s own internal
* sched_sim_driver_start()/sim_bootstrap_picker() calls run, with no hook
* point exposed for a test to register an observer plugin in between.
* plugin_register_core() (needed for real, both by this file's own
* fixture_init() and transitively by sim_picker.c's sim_bootstrap_picker())
* is reimplemented verbatim below -- same "real small reimplementation
* instead of a whole extra file" convention this file's own
* sched_resolve_job_picker() copy already establishes. plugin_register()/
* plugin_unregister() are trivial link-satisfying no-op stubs: plugin_register()
* is referenced only by plugins/builtin_fcfs.c's register_builtin_fcfs()
* (this fixture never calls that entry point, using plugin_register_core()
* directly instead, exactly as sim_bootstrap_picker() does in production);
* plugin_unregister() is referenced only as core.c's plugins-kvmgr
* datakiller (never invoked -- this fixture is never torn down, same
* rationale test_driver_session_ordering.c's own file header documents for
* not needing plugin_unregister() to do anything real either). */
#include "../src/core.c"
#include "../src/core_clock.c"
#include "../src/core_events.c"
#include "../src/core_session.c"
#include "../src/decision.c"
#include "../src/driver.c"
#include "../src/plugin.c"
#include "../src/plugins/builtin_fcfs.c"
#include "../src/recording.c"
#include "../src/recording_events.c"
@ -257,6 +282,109 @@ static void reset_spy(void)
memset(g_spy_session_active_ok, 0, sizeof(g_spy_session_active_ok));
}
/* ---------------------------------------------------------------------- */
/* plugin_register_core() -- REAL reimplementation (verbatim logic from */
/* plugin.c), needed because this file does not #include ../src/plugin.c */
/* (see the #include block's own comment above for why). Used for real by */
/* this file's own fixture_init() (recording-side registration) AND */
/* transitively by sim_picker.c's sim_bootstrap_picker() (replay-side). */
/* plugin_register()/plugin_unregister() are trivial link-satisfying no-op */
/* stubs -- neither is ever actually invoked by this file's fixtures (see */
/* #include block comment). */
/* ---------------------------------------------------------------------- */
int plugin_register_core(xce_sched_core_t* core, xce_sched_plugin_t* plugin)
{
int rc = 0;
if ((core == NULL) || (plugin == NULL)) {
return -EINVAL;
}
if (plugin->init_fn != NULL) {
rc = plugin->init_fn(plugin);
if (rc < 0) {
return rc;
}
}
lock(core->plugins, RTTI_LOCK_WRITE);
rc = key_add(core->plugins, (void*)plugin->name, (void*)plugin);
unlock(core->plugins);
if (rc >= 0) {
plugin->plugin_id = rc;
}
return rc;
}
int plugin_register(void* arg1, void* arg2)
{
(void)arg1;
(void)arg2;
return 0;
}
void plugin_unregister(void* arg) { (void)arg; }
/* ---------------------------------------------------------------------- */
/* plugin_begin_session()/plugin_end_session() SPY (bug-1681) -- same */
/* call-count + session_active-window proof technique as the */
/* sched_account_jobs() spy above; this file's target is sim_loop.c's */
/* CALL-SITE discipline, not per-plugin hook-firing semantics (already */
/* proven for the normal-mode side by test_driver_session_ordering.c's own */
/* real-observer-plugin design -- see #include block comment for why that */
/* design is not practically constructible against sim_loop.c). */
/* ---------------------------------------------------------------------- */
static _Atomic int g_begin_session_spy_calls;
static _Atomic int g_end_session_spy_calls;
static int g_begin_session_spy_active_ok[MAX_SPY_CALLS];
static int g_end_session_spy_active_ok[MAX_SPY_CALLS];
int plugin_begin_session(xce_sched_core_t* core)
{
int idx = atomic_fetch_add(&g_begin_session_spy_calls, 1);
if ((core != NULL) && (idx < MAX_SPY_CALLS)) {
g_begin_session_spy_active_ok[idx]
= (atomic_load_explicit(&(core->session_active),
memory_order_acquire)
== 1)
? 1
: 0;
}
return 0;
}
int plugin_end_session(xce_sched_core_t* core)
{
int idx = atomic_fetch_add(&g_end_session_spy_calls, 1);
if ((core != NULL) && (idx < MAX_SPY_CALLS)) {
g_end_session_spy_active_ok[idx]
= (atomic_load_explicit(&(core->session_active),
memory_order_acquire)
== 1)
? 1
: 0;
}
return 0;
}
static void reset_plugin_session_spies(void)
{
atomic_store(&g_begin_session_spy_calls, 0);
atomic_store(&g_end_session_spy_calls, 0);
memset(g_begin_session_spy_active_ok, 0,
sizeof(g_begin_session_spy_active_ok));
memset(g_end_session_spy_active_ok, 0,
sizeof(g_end_session_spy_active_ok));
}
void setUp(void) { return; }
void tearDown(void) { return; }
@ -501,6 +629,7 @@ test_sim_replay_calls_account_jobs_once_per_round_within_session_window(
* test_driver_session_ordering.c); this test's own target is
* sim_loop.c's call site (task 2.1), isolated by resetting here. */
reset_spy();
reset_plugin_session_spies();
/* ---- Replay the SAME file through the REAL, unmodified simulation
* driver ---- */
@ -528,6 +657,25 @@ test_sim_replay_calls_account_jobs_once_per_round_within_session_window(
"contract");
}
/* bug-1681's own central regression proof: before the fix, both counts
* are 0 (sim_loop.c never called plugin_begin_session()/
* plugin_end_session() at all) -- after, each is 2, one per replayed
* round. */
TEST_ASSERT_EQUAL_INT(2, atomic_load(&g_begin_session_spy_calls));
TEST_ASSERT_EQUAL_INT(2, atomic_load(&g_end_session_spy_calls));
for (i = 0; i < 2; i++) {
TEST_ASSERT_TRUE_MESSAGE(
g_begin_session_spy_active_ok[i],
"plugin_begin_session() must run with core->session_active==1, "
"matching driver.c's own ordering contract (bug-1681)");
TEST_ASSERT_TRUE_MESSAGE(
g_end_session_spy_active_ok[i],
"plugin_end_session() must run with core->session_active==1 "
"(strictly before xce_core_end_session() clears it), matching "
"driver.c's own ordering contract (bug-1681)");
}
printf("test_sim_replay_calls_account_jobs_once_per_round_within_"
"session_window: PASS\n");
}
@ -576,6 +724,7 @@ static void test_sim_replay_batch_mode_uses_the_same_round_boundary(void)
TEST_ASSERT_EQUAL_INT(0, rc);
reset_spy();
reset_plugin_session_spies();
memset(&sim_args, 0, sizeof(sim_args));
sim_args.replay_path = r_path;
@ -591,6 +740,11 @@ static void test_sim_replay_batch_mode_uses_the_same_round_boundary(void)
TEST_ASSERT_EQUAL_INT(1, atomic_load(&g_spy_calls));
TEST_ASSERT_TRUE(g_spy_session_active_ok[0]);
TEST_ASSERT_EQUAL_INT(1, atomic_load(&g_begin_session_spy_calls));
TEST_ASSERT_TRUE(g_begin_session_spy_active_ok[0]);
TEST_ASSERT_EQUAL_INT(1, atomic_load(&g_end_session_spy_calls));
TEST_ASSERT_TRUE(g_end_session_spy_active_ok[0]);
printf("test_sim_replay_batch_mode_uses_the_same_round_boundary: "
"PASS\n");
}