fix(scheduler): sim mode picker never learns about the initial snapshot's own jobs/queues (bug-1567)
xce_scheduler -i -s <replay>: WARN "failed to resolve job picker plugin (rc=-2)" on load, and every job already present in the replay file's INITIAL snapshot stayed PEND forever -- never even reached the picker, let alone got dispatched. Root cause: sched_sim_driver_start() (sim.c) loads the initial snapshot via xce_sched_core_load_snapshot() BEFORE any picker plugin is registered (sim.h's own documented "Known scope boundary" -- only the picker's NAME is threaded through from the ReplayHeader). sched_resolve_job_picker() therefore always returns -ENOENT there, and xce_sched_core_load_snapshot()'s own picker-rebuild walk (on_queue_update/on_job_push per queue/job) is silently skipped. The real builtin_fcfs picker only gets registered moments later in sched_sim_driver_run() -- but nothing then replayed that rebuild walk, so any job/queue from the INITIAL snapshot (as opposed to one arriving later via a ReplayEvent, which correctly reaches the picker through xce_core_push_events()'s own picker!=NULL check) was permanently invisible to next_job(). Fixed by a new file, sim_picker.c: sim_bootstrap_picker() (replacing sim_loop.c's sim_register_default_picker(), moved out because sim_loop.c was already at its own documented 800-line "zero line-budget headroom" cap) registers the picker AND then replays the missed rebuild walk against it. Wired into scheduler_srcs, sched_sim_isolation_lib, and all 9 white-box test TUs that #include sim_loop.c directly. New regression test (test_sim_event_loop.c): a job baked into a fixture's INITIAL snapshot (not a ReplayEvent) now gets a live PEND decision. RED confirmed by temporarily no-op'ing the rebuild walk (test failed as expected), GREEN with the real fix. Full scheduler suite green (41/41, excluding the pre-existing unrelated bug-1560). Note: sim mode still never DISPATCHes (only PENDs) -- that is a separate, intentionally out-of-scope gap (real resource-matching allocation belongs to the not-yet-designed scheduler-allocation spec), not addressed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
0f1df4998e
commit
0c8e417bda
SSH key fingerprint:
SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
14 changed files with 395 additions and 93 deletions
|
|
@ -4,25 +4,27 @@
|
|||
* or structural check confirms no controller-facing symbol is reachable
|
||||
* from the simulation driver's compiled object" -- plus the AC's own
|
||||
* "confirmed by the struct definition itself, not runtime checks alone"
|
||||
* clause. Scope: sim.h + sim.c + sim_loop.c + sim_record.c + repl.c/repl.h/
|
||||
* repl_break.c/repl_info.c/repl_internal.h/repl_params.c together (task
|
||||
* 11.1 + task 11.2's sibling file + task 11.5's sibling file, the
|
||||
* --sim-output writer + task 12.1's REPL files + task 12.2's own
|
||||
* clause. Scope: sim.h + sim.c + sim_loop.c + sim_picker.c + sim_record.c +
|
||||
* repl.c/repl.h/repl_break.c/repl_info.c/repl_internal.h/repl_params.c
|
||||
* together (task 11.1 + task 11.2's sibling file + task 11.5's sibling
|
||||
* file, the --sim-output writer + task 12.1's REPL files + task 12.2's own
|
||||
* repl_break.c/repl_info.c/repl_internal.h + task 12.3's own repl_params.c,
|
||||
* each an 800-line-cap split of repl.c -- see repl_internal.h's own "Why a
|
||||
* split at all" section) -- NOT a re-audit of core.c (task 4.1's own
|
||||
* territory, test_core_isolation.c) and NOT plugin.c/builtin_fcfs.c (shared
|
||||
* normal-mode infra, out of this task's boundary). sim_verify.c (task 11.4)
|
||||
* is likewise out of scope here -- it is never compiled into
|
||||
* sched_sim_isolation_lib below and carries no isolation-sensitive logic of
|
||||
* its own (pure struct comparison, no I/O); extending this guard to it was
|
||||
* not part of task 11.4's own text and is left as-is. repl.c/repl.h were
|
||||
* added per task 12.1's own text (extending this exact 3-file precedent to
|
||||
* a 5-file one); repl_break.c/repl_info.c/repl_internal.h extended it again
|
||||
* (task 12.2, to an 8-file one); repl_params.c extends it once more (task
|
||||
* 12.3, to a 9-file one) -- all touch xce_sched_sim_driver_t directly or
|
||||
* read/write stdin/stdout, the same category of driver-level code this
|
||||
* guard already covers.
|
||||
* split at all" section -- + bug-1567's own sim_picker.c, the identical
|
||||
* split applied to sim_loop.c once IT hit the same cap) -- NOT a re-audit
|
||||
* of core.c (task 4.1's own territory, test_core_isolation.c) and NOT
|
||||
* plugin.c/builtin_fcfs.c (shared normal-mode infra, out of this task's
|
||||
* boundary). sim_verify.c (task 11.4) is likewise out of scope here -- it
|
||||
* is never compiled into sched_sim_isolation_lib below and carries no
|
||||
* isolation-sensitive logic of its own (pure struct comparison, no I/O);
|
||||
* extending this guard to it was not part of task 11.4's own text and is
|
||||
* left as-is. repl.c/repl.h were added per task 12.1's own text (extending
|
||||
* this exact 3-file precedent to a 5-file one); repl_break.c/repl_info.c/
|
||||
* repl_internal.h extended it again (task 12.2, to an 8-file one);
|
||||
* repl_params.c extends it once more (task 12.3, to a 9-file one);
|
||||
* sim_picker.c extends it a final time (bug-1567, to a 10-file one) -- all
|
||||
* touch xce_sched_sim_driver_t directly or read/write stdin/stdout, the
|
||||
* same category of driver-level code this guard already covers.
|
||||
*
|
||||
* Three layers, extending test_core_isolation.c's two-layer pattern (same
|
||||
* genre: source-text scans of the real, current on-disk files, not a frozen
|
||||
|
|
@ -90,6 +92,10 @@
|
|||
#error \
|
||||
"XCE_SIM_LOOP_C_PATH must be defined by meson.build to sim_loop.c's absolute path"
|
||||
#endif
|
||||
#ifndef XCE_SIM_PICKER_C_PATH
|
||||
#error \
|
||||
"XCE_SIM_PICKER_C_PATH must be defined by meson.build to sim_picker.c's absolute path"
|
||||
#endif
|
||||
#ifndef XCE_SIM_RECORD_C_PATH
|
||||
#error \
|
||||
"XCE_SIM_RECORD_C_PATH must be defined by meson.build to sim_record.c's absolute path"
|
||||
|
|
@ -120,7 +126,7 @@
|
|||
#endif
|
||||
#ifndef XCE_SIM_LIB_PATH
|
||||
#error \
|
||||
"XCE_SIM_LIB_PATH must be defined by meson.build to the absolute path of sim.c+sim_loop.c+sim_record.c+repl.c+repl_break.c+repl_info.c+repl_params.c's isolated static_library() archive"
|
||||
"XCE_SIM_LIB_PATH must be defined by meson.build to the absolute path of sim.c+sim_loop.c+sim_picker.c+sim_record.c+repl.c+repl_break.c+repl_info.c+repl_params.c's isolated static_library() archive"
|
||||
#endif
|
||||
|
||||
/* Forbidden headers (design.md "Allowed Dependencies", same list
|
||||
|
|
@ -393,6 +399,7 @@ static void test_sim_source_paths_are_readable(void)
|
|||
char* h = read_whole_file(XCE_SIM_H_PATH);
|
||||
char* c = read_whole_file(XCE_SIM_C_PATH);
|
||||
char* loop = read_whole_file(XCE_SIM_LOOP_C_PATH);
|
||||
char* picker = read_whole_file(XCE_SIM_PICKER_C_PATH);
|
||||
char* record = read_whole_file(XCE_SIM_RECORD_C_PATH);
|
||||
char* repl_c = read_whole_file(XCE_SIM_REPL_C_PATH);
|
||||
char* repl_h = read_whole_file(XCE_SIM_REPL_H_PATH);
|
||||
|
|
@ -404,6 +411,7 @@ static void test_sim_source_paths_are_readable(void)
|
|||
TEST_ASSERT_NOT_NULL_MESSAGE(h, XCE_SIM_H_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(c, XCE_SIM_C_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(loop, XCE_SIM_LOOP_C_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(picker, XCE_SIM_PICKER_C_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(record, XCE_SIM_RECORD_C_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(repl_c, XCE_SIM_REPL_C_PATH);
|
||||
TEST_ASSERT_NOT_NULL_MESSAGE(repl_h, XCE_SIM_REPL_H_PATH);
|
||||
|
|
@ -415,6 +423,7 @@ static void test_sim_source_paths_are_readable(void)
|
|||
free(h);
|
||||
free(c);
|
||||
free(loop);
|
||||
free(picker);
|
||||
free(record);
|
||||
free(repl_c);
|
||||
free(repl_h);
|
||||
|
|
@ -429,6 +438,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
char* h_raw = read_whole_file(XCE_SIM_H_PATH);
|
||||
char* c_raw = read_whole_file(XCE_SIM_C_PATH);
|
||||
char* loop_raw = read_whole_file(XCE_SIM_LOOP_C_PATH);
|
||||
char* picker_raw = read_whole_file(XCE_SIM_PICKER_C_PATH);
|
||||
char* record_raw = read_whole_file(XCE_SIM_RECORD_C_PATH);
|
||||
char* repl_c_raw = read_whole_file(XCE_SIM_REPL_C_PATH);
|
||||
char* repl_h_raw = read_whole_file(XCE_SIM_REPL_H_PATH);
|
||||
|
|
@ -439,6 +449,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
char* h;
|
||||
char* c;
|
||||
char* loop;
|
||||
char* picker;
|
||||
char* record;
|
||||
char* repl_c;
|
||||
char* repl_h;
|
||||
|
|
@ -450,6 +461,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
TEST_ASSERT_NOT_NULL(h_raw);
|
||||
TEST_ASSERT_NOT_NULL(c_raw);
|
||||
TEST_ASSERT_NOT_NULL(loop_raw);
|
||||
TEST_ASSERT_NOT_NULL(picker_raw);
|
||||
TEST_ASSERT_NOT_NULL(record_raw);
|
||||
TEST_ASSERT_NOT_NULL(repl_c_raw);
|
||||
TEST_ASSERT_NOT_NULL(repl_h_raw);
|
||||
|
|
@ -461,6 +473,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
h = strip_c_comments(h_raw);
|
||||
c = strip_c_comments(c_raw);
|
||||
loop = strip_c_comments(loop_raw);
|
||||
picker = strip_c_comments(picker_raw);
|
||||
record = strip_c_comments(record_raw);
|
||||
repl_c = strip_c_comments(repl_c_raw);
|
||||
repl_h = strip_c_comments(repl_h_raw);
|
||||
|
|
@ -471,6 +484,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
TEST_ASSERT_NOT_NULL(h);
|
||||
TEST_ASSERT_NOT_NULL(c);
|
||||
TEST_ASSERT_NOT_NULL(loop);
|
||||
TEST_ASSERT_NOT_NULL(picker);
|
||||
TEST_ASSERT_NOT_NULL(record);
|
||||
TEST_ASSERT_NOT_NULL(repl_c);
|
||||
TEST_ASSERT_NOT_NULL(repl_h);
|
||||
|
|
@ -482,6 +496,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
assert_no_forbidden_includes(h, "sim.h");
|
||||
assert_no_forbidden_includes(c, "sim.c");
|
||||
assert_no_forbidden_includes(loop, "sim_loop.c");
|
||||
assert_no_forbidden_includes(picker, "sim_picker.c");
|
||||
assert_no_forbidden_includes(record, "sim_record.c");
|
||||
assert_no_forbidden_includes(repl_c, "repl.c");
|
||||
assert_no_forbidden_includes(repl_h, "repl.h");
|
||||
|
|
@ -493,6 +508,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
free(h_raw);
|
||||
free(c_raw);
|
||||
free(loop_raw);
|
||||
free(picker_raw);
|
||||
free(record_raw);
|
||||
free(repl_c_raw);
|
||||
free(repl_h_raw);
|
||||
|
|
@ -503,6 +519,7 @@ static void test_sim_never_includes_forbidden_headers(void)
|
|||
free(h);
|
||||
free(c);
|
||||
free(loop);
|
||||
free(picker);
|
||||
free(record);
|
||||
free(repl_c);
|
||||
free(repl_h);
|
||||
|
|
@ -517,6 +534,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
char* h_raw = read_whole_file(XCE_SIM_H_PATH);
|
||||
char* c_raw = read_whole_file(XCE_SIM_C_PATH);
|
||||
char* loop_raw = read_whole_file(XCE_SIM_LOOP_C_PATH);
|
||||
char* picker_raw = read_whole_file(XCE_SIM_PICKER_C_PATH);
|
||||
char* record_raw = read_whole_file(XCE_SIM_RECORD_C_PATH);
|
||||
char* repl_c_raw = read_whole_file(XCE_SIM_REPL_C_PATH);
|
||||
char* repl_h_raw = read_whole_file(XCE_SIM_REPL_H_PATH);
|
||||
|
|
@ -527,6 +545,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
char* h;
|
||||
char* c;
|
||||
char* loop;
|
||||
char* picker;
|
||||
char* record;
|
||||
char* repl_c;
|
||||
char* repl_h;
|
||||
|
|
@ -538,6 +557,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
TEST_ASSERT_NOT_NULL(h_raw);
|
||||
TEST_ASSERT_NOT_NULL(c_raw);
|
||||
TEST_ASSERT_NOT_NULL(loop_raw);
|
||||
TEST_ASSERT_NOT_NULL(picker_raw);
|
||||
TEST_ASSERT_NOT_NULL(record_raw);
|
||||
TEST_ASSERT_NOT_NULL(repl_c_raw);
|
||||
TEST_ASSERT_NOT_NULL(repl_h_raw);
|
||||
|
|
@ -549,6 +569,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
h = strip_c_comments(h_raw);
|
||||
c = strip_c_comments(c_raw);
|
||||
loop = strip_c_comments(loop_raw);
|
||||
picker = strip_c_comments(picker_raw);
|
||||
record = strip_c_comments(record_raw);
|
||||
repl_c = strip_c_comments(repl_c_raw);
|
||||
repl_h = strip_c_comments(repl_h_raw);
|
||||
|
|
@ -559,6 +580,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
TEST_ASSERT_NOT_NULL(h);
|
||||
TEST_ASSERT_NOT_NULL(c);
|
||||
TEST_ASSERT_NOT_NULL(loop);
|
||||
TEST_ASSERT_NOT_NULL(picker);
|
||||
TEST_ASSERT_NOT_NULL(record);
|
||||
TEST_ASSERT_NOT_NULL(repl_c);
|
||||
TEST_ASSERT_NOT_NULL(repl_h);
|
||||
|
|
@ -570,6 +592,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
assert_no_forbidden_symbols(h, "sim.h");
|
||||
assert_no_forbidden_symbols(c, "sim.c");
|
||||
assert_no_forbidden_symbols(loop, "sim_loop.c");
|
||||
assert_no_forbidden_symbols(picker, "sim_picker.c");
|
||||
assert_no_forbidden_symbols(record, "sim_record.c");
|
||||
assert_no_forbidden_symbols(repl_c, "repl.c");
|
||||
assert_no_forbidden_symbols(repl_h, "repl.h");
|
||||
|
|
@ -581,6 +604,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
free(h_raw);
|
||||
free(c_raw);
|
||||
free(loop_raw);
|
||||
free(picker_raw);
|
||||
free(record_raw);
|
||||
free(repl_c_raw);
|
||||
free(repl_h_raw);
|
||||
|
|
@ -591,6 +615,7 @@ static void test_sim_never_calls_forbidden_symbols(void)
|
|||
free(h);
|
||||
free(c);
|
||||
free(loop);
|
||||
free(picker);
|
||||
free(record);
|
||||
free(repl_c);
|
||||
free(repl_h);
|
||||
|
|
@ -641,9 +666,10 @@ static void test_sim_driver_struct_has_no_forbidden_fields(void)
|
|||
|
||||
/*
|
||||
* test_sim_object_has_no_forbidden_undefined_symbols - the literal
|
||||
* build-time/link-time proof: sim.c + sim_loop.c + sim_record.c + repl.c +
|
||||
* repl_break.c + repl_info.c + repl_params.c, compiled together in
|
||||
* isolation into sched_sim_isolation_lib (meson.build), must not reference
|
||||
* build-time/link-time proof: sim.c + sim_loop.c + sim_picker.c +
|
||||
* sim_record.c + repl.c + repl_break.c + repl_info.c + repl_params.c,
|
||||
* compiled together in isolation into sched_sim_isolation_lib
|
||||
* (meson.build), must not reference
|
||||
* (as an undefined external symbol in the compiled archive) anything
|
||||
* matching the forbidden prefixes above. Independent of what any header
|
||||
* transitively declares -- reflects only what this driver's compiled code
|
||||
|
|
@ -672,9 +698,9 @@ static void test_sim_object_has_no_forbidden_undefined_symbols(void)
|
|||
/* posix format for an undefined symbol: "<name> U" (no
|
||||
* value/size columns). Skip anything that doesn't parse as
|
||||
* exactly that -- e.g. a blank line, or an archive member header
|
||||
* some nm versions print (this archive has seven member objects,
|
||||
* sim.c.o, sim_loop.c.o, sim_record.c.o, repl.c.o, repl_break.c.o,
|
||||
* repl_info.c.o, and repl_params.c.o). */
|
||||
* some nm versions print (this archive has eight member objects,
|
||||
* sim.c.o, sim_loop.c.o, sim_picker.c.o, sim_record.c.o, repl.c.o,
|
||||
* repl_break.c.o, repl_info.c.o, and repl_params.c.o). */
|
||||
if (sscanf(line, "%511s %c", symbol, &type) != 2) {
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue