diff --git a/src/scheduler/meson.build b/src/scheduler/meson.build index bc8905cd..6c28b386 100644 --- a/src/scheduler/meson.build +++ b/src/scheduler/meson.build @@ -16,6 +16,7 @@ scheduler_srcs = [ 'src/recording_session.c', 'src/sim.c', 'src/sim_loop.c', + 'src/sim_picker.c', 'src/sim_verify.c', 'src/sim_record.c', 'src/repl.c', @@ -82,23 +83,28 @@ sched_core_isolation_lib = static_library( # the simulation driver's own files, consumed by # testsuite/test_sim_isolation.c to prove at build time that # sim.c/sim_loop.c/sim_record.c/repl.c/repl_break.c/repl_info.c/ -# repl_params.c reference zero controller-facing symbols (client/netio/ -# xceapi/delay/idgen). All files are compiled together into ONE archive -- -# task 11.2 added sim_loop.c as a sibling file to task 11.1's sim.c, task -# 11.5 added sim_record.c as a third, task 12.1 added repl.c as a fourth, -# task 12.2 added repl_break.c/repl_info.c as a fifth/sixth (task 12.2's -# own 800-line-cap split of repl.c's breakpoint/state-inspection logic -- -# repl_internal.h's own "Why a split at all" section has the full -# rationale), and task 12.3 adds repl_params.c as a seventh (the same -# split, extended again for `snapshot`/`set`); covering only a subset of -# them would repeat core.c's own siblings' (core_events.c/core_session.c/ -# core_clock.c) never-mechanically-covered gap for the sim driver. Same -# "compiling to an object is enough, no real linking needed" rationale as -# sched_core_isolation_lib above. +# repl_params.c/sim_picker.c reference zero controller-facing symbols +# (client/netio/xceapi/delay/idgen). All files are compiled together into +# ONE archive -- task 11.2 added sim_loop.c as a sibling file to task +# 11.1's sim.c, task 11.5 added sim_record.c as a third, task 12.1 added +# repl.c as a fourth, task 12.2 added repl_break.c/repl_info.c as a +# fifth/sixth (task 12.2's own 800-line-cap split of repl.c's +# breakpoint/state-inspection logic -- repl_internal.h's own "Why a split +# at all" section has the full rationale), task 12.3 added repl_params.c +# as a seventh (the same split, extended again for `snapshot`/`set`), and +# bug-1567's fix added sim_picker.c as an eighth (sim_loop.c's own +# "zero line-budget headroom" -- same reason its siblings above exist -- +# so the fix's picker-registration + post-load picker-rebuild-walk logic +# lives here instead of growing sim_loop.c past its cap); covering only a +# subset of them would repeat core.c's own siblings' (core_events.c/ +# core_session.c/core_clock.c) never-mechanically-covered gap for the sim +# driver. Same "compiling to an object is enough, no real linking needed" +# rationale as sched_core_isolation_lib above. sched_sim_isolation_lib = static_library( 'sched_sim_isolation', - ['src/sim.c', 'src/sim_loop.c', 'src/sim_record.c', 'src/repl.c', - 'src/repl_break.c', 'src/repl_info.c', 'src/repl_params.c'], + ['src/sim.c', 'src/sim_loop.c', 'src/sim_picker.c', 'src/sim_record.c', + 'src/repl.c', 'src/repl_break.c', 'src/repl_info.c', + 'src/repl_params.c'], include_directories: incdir, dependencies: scheduler_deps, install: false) diff --git a/src/scheduler/src/sim_loop.c b/src/scheduler/src/sim_loop.c index 79917216..ac9c8e2d 100644 --- a/src/scheduler/src/sim_loop.c +++ b/src/scheduler/src/sim_loop.c @@ -11,7 +11,6 @@ #include "repl.h" #include "sim.h" #include "utils.h" -#include "xstring.h" /* * sim_loop.c - simulation-mode driver event loop (scheduler-core spec, task @@ -353,57 +352,17 @@ static int sim_process_replay_event_message(xce_sched_sim_driver_t* sim, return replay_event_to_core_event(&decoded, out_ev); } -/* - * sim_register_default_picker - Req 5 AC 17's "core.h-scoped plugin - * registration path" (task 11.1's own documented "Known scope boundary", - * quoted on sim.h's sched_sim_driver_start() doc comment) -- this task - * (11.2) is where a resolvable picker becomes load-bearing: without one, - * sim_picker_thread_fn()'s sched_resolve_job_picker() call always fails and - * every scheduling round closes with zero decisions (see that function's - * own graceful-degradation handling) -- harmless for a fixture with no - * pending jobs, but never actually exercises picker candidate selection for - * one that has any. - * - * Defaults sim->core.job_pick_plugin to "builtin_fcfs" when the - * ReplayHeader carried no schedPolicy (mirrors schedobj.c's - * sched_set_default_job_pick_plugin(), normal mode's own default), then - * registers the real builtin_fcfs plugin -- via builtin_fcfs_new_plugin() + - * plugin_register_core() (plugin.h), NOT plugin_register() (no - * xce_scheduler_t* exists here, Req 5 AC 17) -- ONLY when the configured - * policy is genuinely "builtin_fcfs". A replay file recorded under any - * OTHER picker name is left unresolvable here: this task builds no generic - * "load an arbitrary named plugin for simulation" mechanism (a materially - * bigger, separate problem) -- sched_resolve_job_picker() then legitimately - * fails for it, and the picker thread degrades gracefully, the same - * documented behavior xce_sched_core_load_snapshot() already establishes - * for an unresolvable picker (sim.h's own doc comment). - */ -static void sim_register_default_picker(xce_sched_sim_driver_t* sim) -{ - xce_sched_plugin_t* plugin; - - if (sim->core.job_pick_plugin == NULL) { - sim->core.job_pick_plugin = makeString("builtin_fcfs"); - } - - if (strcmp(sim->core.job_pick_plugin, "builtin_fcfs") != 0) { - return; - } - - plugin = builtin_fcfs_new_plugin(); - if (plugin == NULL) { - ERROR("%s out of memory building the builtin_fcfs plugin instance", - __func__); - return; - } - - if (plugin_register_core(&(sim->core), plugin) < 0) { - ERROR("%s failed to register builtin_fcfs against the simulation " - "core", - __func__); - free(plugin); - } -} +/* sim_bootstrap_picker - Req 5 AC 17's "core.h-scoped plugin registration + * path" (task 11.1's own documented "Known scope boundary", quoted on + * sim.h's sched_sim_driver_start() doc comment), PLUS (bug-1567) a replay + * of the picker-rebuild walk that boundary forces + * xce_sched_core_load_snapshot() to skip for the INITIAL snapshot's own + * jobs/queues. Moved to sim_picker.c (this file's own "zero line-budget + * headroom", see this file's header comment) -- extern'd here per this + * file family's established "declared where consumed" idiom. See + * sim_picker.c's own file header comment for the full mechanism/bug + * trace. */ +extern void sim_bootstrap_picker(xce_sched_sim_driver_t* sim); /* * sim_print_decision - Req 5 AC 19's default: "If --sim-output is not @@ -521,7 +480,7 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim, } xce_repl_pre_replay_pause(sim); - sim_register_default_picker(sim); + sim_bootstrap_picker(sim); /* Req 5 AC 19 (task 11.5): the first two --sim-output messages, written * exactly once, before the event loop below runs -- see sim_record.c's diff --git a/src/scheduler/src/sim_picker.c b/src/scheduler/src/sim_picker.c new file mode 100644 index 00000000..a02fc36b --- /dev/null +++ b/src/scheduler/src/sim_picker.c @@ -0,0 +1,122 @@ +#include +#include +#include +#include + +#include "plugin.h" +#include "sim.h" +#include "utils.h" +#include "xstring.h" + +/* + * sim_picker.c - simulation driver picker bootstrap (scheduler-core spec, + * task 11.2, Req 5 AC 17; bug-1567 fix). Split out of sim_loop.c (this + * project's 800-line-per-file convention, CLAUDE.md/coding-style.md -- + * sim_loop.c's own file header comment already states it has "zero + * line-budget headroom") rather than folded into sim.c: picker + * registration is explicitly task 11.2's concern (sim_loop.c's own + * original file header comment), not task 11.1's (sim.c owns only Req 5 + * AC 13 steps 1-4) -- moving it into sim.c would blur that documented + * ownership boundary. Same "new sibling file when the owning file hits + * its cap" precedent this family already established five times + * (sim_loop.c itself, sim_record.c, repl.c, repl_break.c/repl_info.c, + * repl_params.c). + * + * bug-1567: sched_sim_driver_start() (sim.c, Step 3) calls + * xce_sched_core_load_snapshot() (snapshot_load.c), which tries to + * rebuild the picker's internal state (on_queue_update per queue, then + * on_job_push per pending job) for whatever the INITIAL snapshot + * contains -- but at that point no picker plugin is registered yet + * (sim.h's own documented "Known scope boundary" on + * sched_sim_driver_start()), so sched_resolve_job_picker() always + * returns -ENOENT there, and that rebuild walk is unconditionally + * skipped (WARN "failed to resolve job picker plugin"). The picker + * plugin only gets registered afterward, by sim_bootstrap_picker() below + * (formerly sim_loop.c's sim_register_default_picker(), called from + * sched_sim_driver_run() right after sched_sim_driver_start() returns) + * -- and nothing then re-ran that rebuild walk, so any job/queue that + * was already part of the INITIAL snapshot (as opposed to arriving later + * via a ReplayEvent, which correctly reaches the picker through + * xce_core_push_events()'s own picker != NULL check, core_events.c) was + * permanently invisible to next_job() -- it never got dispatched OR + * pended, it just silently never came up. sim_bootstrap_picker() now + * replays that same rebuild walk a second time, against whichever picker + * it just registered, mirroring xce_sched_core_load_snapshot()'s own + * queues-before-jobs ordering (snapshot_load.c) exactly. + */ + +/* Consumed by sim_loop.c via a plain `extern` declared there -- same + * "extern declared where consumed" idiom this file family already uses + * throughout (msg_root_has_shape(), sim_decision_eq(), + * sim_record_write_event(), etc., sim_loop.c's own file header comment). */ +void sim_bootstrap_picker(xce_sched_sim_driver_t* sim) +{ + xce_sched_plugin_t* plugin; + + if (sim->core.job_pick_plugin == NULL) { + sim->core.job_pick_plugin = makeString("builtin_fcfs"); + } + + if (strcmp(sim->core.job_pick_plugin, "builtin_fcfs") != 0) { + /* A replay file recorded under any OTHER picker name is left + * unresolvable here: this codebase builds no generic "load an + * arbitrary named plugin for simulation" mechanism (a materially + * bigger, separate problem) -- sched_resolve_job_picker() then + * legitimately fails for it downstream, and the picker thread + * degrades gracefully, the same documented behavior + * xce_sched_core_load_snapshot() already establishes for an + * unresolvable picker (sim.h's own doc comment). */ + return; + } + + plugin = builtin_fcfs_new_plugin(); + if (plugin == NULL) { + ERROR("%s out of memory building the builtin_fcfs plugin instance", + __func__); + return; + } + + if (plugin_register_core(&(sim->core), plugin) < 0) { + ERROR("%s failed to register builtin_fcfs against the simulation " + "core", + __func__); + free(plugin); + return; + } + + /* bug-1567: replay the picker-rebuild walk xce_sched_core_load_snapshot() + * (snapshot_load.c) already attempted and had to skip, now that a + * picker actually exists -- same queues-then-jobs ordering that + * function's own comments document ("Picker rebuild ... fires here, + * per-queue" then "... per job"). */ + { + iterator_t it; + xce_sched_queue_t* q = NULL; + xce_sched_job_t* job = NULL; + int rc; + + rc = iter_make(&it, sim->core.queues); + if (rc >= 0) { + for (;;) { + rc = iter_next(&it, (void**)&q); + if ((rc < 0) || (rc == RTTI_ITER_NO_DATA)) { + break; + } + plugin->picker_iface->on_queue_update(plugin, q); + } + iter_destroy(&it); + } + + rc = iter_make(&it, sim->core.jobs); + if (rc >= 0) { + for (;;) { + rc = iter_next(&it, (void**)&job); + if ((rc < 0) || (rc == RTTI_ITER_NO_DATA)) { + break; + } + plugin->picker_iface->on_job_push(plugin, job); + } + iter_destroy(&it); + } + } +} diff --git a/src/scheduler/testsuite/meson.build b/src/scheduler/testsuite/meson.build index 30a940b4..e5b6fa01 100644 --- a/src/scheduler/testsuite/meson.build +++ b/src/scheduler/testsuite/meson.build @@ -524,7 +524,8 @@ sched_sim_event_loop_test = executable( # boundary doc comment legitimately names xcecli_ctx_t/xce_cluster_config_t # in prose). link_with establishes the build-order dependency on # sched_sim_isolation_lib (parent meson.build -- sim.c + sim_loop.c + -# sim_record.c compiled together, isolated from the rest of the scheduler); +# sim_picker.c (bug-1567's sim_loop.c-cap-driven sibling) + sim_record.c +# compiled together, isolated from the rest of the scheduler); # XCE_SIM_*_PATH bake in absolute paths the same way XCE_CORE_*_PATH does # above. sched_sim_isolation_test = executable( @@ -542,6 +543,9 @@ sched_sim_isolation_test = executable( '-DXCE_SIM_LOOP_C_PATH="' + (meson.current_source_dir() / '..' / 'src' / 'sim_loop.c') + '"', + '-DXCE_SIM_PICKER_C_PATH="' + + (meson.current_source_dir() / '..' / 'src' / 'sim_picker.c') + + '"', '-DXCE_SIM_RECORD_C_PATH="' + (meson.current_source_dir() / '..' / 'src' / 'sim_record.c') + '"', diff --git a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c index 9c95c9d8..d3cbe8d1 100644 --- a/src/scheduler/testsuite/test_repl_breakpoints_inspection.c +++ b/src/scheduler/testsuite/test_repl_breakpoints_inspection.c @@ -84,6 +84,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_repl_equivalence.c b/src/scheduler/testsuite/test_repl_equivalence.c index 68936a15..1abc7aa6 100644 --- a/src/scheduler/testsuite/test_repl_equivalence.c +++ b/src/scheduler/testsuite/test_repl_equivalence.c @@ -133,6 +133,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_repl_execution_control.c b/src/scheduler/testsuite/test_repl_execution_control.c index d3ef499f..295e600f 100644 --- a/src/scheduler/testsuite/test_repl_execution_control.c +++ b/src/scheduler/testsuite/test_repl_execution_control.c @@ -99,6 +99,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_repl_snapshot_params.c b/src/scheduler/testsuite/test_repl_snapshot_params.c index 805a8ce0..e0478827 100644 --- a/src/scheduler/testsuite/test_repl_snapshot_params.c +++ b/src/scheduler/testsuite/test_repl_snapshot_params.c @@ -63,6 +63,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_sim_alt_events.c b/src/scheduler/testsuite/test_sim_alt_events.c index b3b549ab..371abc0b 100644 --- a/src/scheduler/testsuite/test_sim_alt_events.c +++ b/src/scheduler/testsuite/test_sim_alt_events.c @@ -92,6 +92,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_sim_equivalence.c b/src/scheduler/testsuite/test_sim_equivalence.c index 60d42da7..1236a588 100644 --- a/src/scheduler/testsuite/test_sim_equivalence.c +++ b/src/scheduler/testsuite/test_sim_equivalence.c @@ -161,6 +161,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_sim_event_loop.c b/src/scheduler/testsuite/test_sim_event_loop.c index 0ff4ee5f..4d85fadc 100644 --- a/src/scheduler/testsuite/test_sim_event_loop.c +++ b/src/scheduler/testsuite/test_sim_event_loop.c @@ -82,6 +82,7 @@ #include #include #include +#include #include @@ -107,6 +108,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" @@ -551,6 +553,179 @@ test_run_processes_multiple_sessions_in_order_and_no_longer_returns_enosys(void) "returns_enosys: PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* Test 3 (bug-1567 regression): a job/queue already present in the */ +/* INITIAL snapshot -- as opposed to arriving later via a ReplayEvent -- */ +/* must still reach the real picker and get a live decision. */ +/* ---------------------------------------------------------------------- */ + +/* Same fixture technique as write_header_and_snapshot() above, but (a) + * records schedPolicy="builtin_fcfs" (test_sim_verify.c's own + * build_fixture_scheduler() precedent -- this file's own copy deliberately + * uses a non-matching stub name instead, so it cannot be reused here + * unmodified) so sim_bootstrap_picker() (sim_picker.c) registers the REAL + * builtin_fcfs plugin, not a no-op stub, and (b) adds one real queue and + * one real PEND job directly to core->queues/core->jobs (core_build_queue()/ + * core_build_job() + sched_add_queue()/sched_add_job(), core_events.c, + * already white-box #included above) BEFORE calling sched_record_start() -- + * so xce_sched_core_save_snapshot() (snapshot.c) captures both into the + * INITIAL snapshot itself, not as a later ReplayEvent. */ +static FILE* write_header_and_snapshot_with_pending_job(char* out_path, + size_t out_path_len, + const char* suffix, + const char* queue_name, + uint64_t jobid) +{ + char record_dir[PATH_MAX]; + xce_scheduler_t sched; + xce_queue_t wire_queue; + xce_sched_queue_t* q; + job_submit_data_t submit; + xce_job_t wire_job; + xce_sched_job_t* job; + int rc; + + make_tmp_dir(record_dir, sizeof(record_dir), suffix); + + memset(&sched, 0, sizeof(sched)); + mylock_init(&(sched.record_lock)); + rc = xce_sched_core_init(&(sched.core)); + TEST_ASSERT_EQUAL_INT(0, rc); + sched.core.job_pick_plugin = (char*)"builtin_fcfs"; + sched.record_dir = record_dir; + + memset(&wire_queue, 0, sizeof(wire_queue)); + wire_queue.queue = (char*)queue_name; + wire_queue.status = QUEUE_STAT_OPEN; + wire_queue.priority = 1; + q = core_build_queue(&wire_queue); + TEST_ASSERT_NOT_NULL(q); + TEST_ASSERT_GREATER_OR_EQUAL(0, sched_add_queue(&(sched.core), q)); + + memset(&submit, 0, sizeof(submit)); + submit.queue = (char*)queue_name; + submit.user = (char*)"alice"; + submit.mintasks = 1; + submit.maxtasks = 1; + + memset(&wire_job, 0, sizeof(wire_job)); + wire_job.jobid = jobid; + wire_job.state = JOB_STAT_PEND; + wire_job.submit = &submit; + job = core_build_job(&(sched.core), &wire_job); + TEST_ASSERT_NOT_NULL(job); + TEST_ASSERT_GREATER_OR_EQUAL(0, sched_add_job(&(sched.core), job)); + + rc = sched_record_start(&sched, "fixture.replay"); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_TRUE(strlen(sched.record_file_path) < out_path_len); + strcpy(out_path, sched.record_file_path); + + return sched.record_file; +} + +/* Same dup()/dup2() STDOUT_FILENO capture technique test_sim_verify.c's own + * run_sim_and_capture_stdout() establishes (this file's white-box TU does + * not #include that file, so a plain copy is needed here rather than a + * cross-file static call). */ +static char* run_sim_and_capture_stdout(const char* path, int* out_rc) +{ + sched_sim_args_t args; + xce_sched_sim_driver_t sim; + char tmp[] = "/tmp/xce_simloop_run_XXXXXX"; + int fd = mkstemp(tmp); + int saved_stdout; + off_t len; + char* buf; + + TEST_ASSERT_TRUE_MESSAGE(fd >= 0, "mkstemp for stdout capture must " + "succeed"); + + fflush(stdout); + saved_stdout = dup(STDOUT_FILENO); + TEST_ASSERT_TRUE_MESSAGE(saved_stdout >= 0, "dup(STDOUT_FILENO) must " + "succeed"); + TEST_ASSERT_TRUE_MESSAGE(dup2(fd, STDOUT_FILENO) >= 0, + "dup2 redirect of STDOUT_FILENO must succeed"); + + memset(&args, 0, sizeof(args)); + args.replay_path = path; + args.sim_start_ts = SCHED_SIM_START_TS_LATEST; + args.sim_speed = 0; /* frozen -- deterministic, no wall-clock wait */ + + *out_rc = sched_sim_driver_run(&sim, &args); + + fflush(stdout); + dup2(saved_stdout, STDOUT_FILENO); + close(saved_stdout); + + len = lseek(fd, 0, SEEK_CUR); + TEST_ASSERT_TRUE(len >= 0); + + buf = (char*)calloc(1, (size_t)len + 1); + TEST_ASSERT_NOT_NULL(buf); + TEST_ASSERT_TRUE(lseek(fd, 0, SEEK_SET) == 0); + if (len > 0) { + ssize_t nread = read(fd, buf, (size_t)len); + TEST_ASSERT_TRUE(nread == len); + } + close(fd); + unlink(tmp); + + return buf; +} + +/* + * bug-1567: before this fix, sched_sim_driver_start() (sim.c) loaded the + * initial snapshot while no picker plugin was registered yet (sim.h's own + * "Known scope boundary"), so xce_sched_core_load_snapshot()'s + * picker-rebuild walk (on_queue_update/on_job_push) was skipped for the + * job/queue this fixture bakes into the INITIAL snapshot -- and nothing + * ever replayed that walk once sim_bootstrap_picker() registered the real + * picker moments later. The job was therefore invisible to next_job() + * forever: no PEND, no DISPATCH, nothing -- exactly the user report this + * bug tracks ("no job will be scheduled and all jobs in pend state"). + * After the fix, sim_bootstrap_picker() (sim_picker.c) replays that walk + * itself, so this job -- despite never appearing in a single ReplayEvent + * -- still reaches the picker and gets a live PEND decision (this driver + * never DISPATCHes at all, a separate, intentionally out-of-scope gap -- + * see sim_picker_thread_fn()'s own doc comment, sim_loop.c). + */ +static void +test_job_present_only_in_initial_snapshot_still_reaches_the_picker(void) +{ + char path[PATH_MAX]; + FILE* f; + int rc; + char* captured; + + f = write_header_and_snapshot_with_pending_job(path, sizeof(path), + "_initsnap", "normal", + 4242ULL); + TEST_ASSERT_NOT_NULL(f); + + /* One session boundary is enough to trigger one scheduling round -- + * the job under test arrives via the INITIAL SNAPSHOT, not via any + * ReplayEvent appended here (deliberately: zero jobs/queues events in + * this stream is the whole point of this regression). */ + append_replay_session(f, 1000ULL); + append_end_of_stream_marker(f); + fclose(f); + + captured = run_sim_and_capture_stdout(path, &rc); + + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_NOT_NULL_MESSAGE( + strstr(captured, "PEND jobid=4242"), + "a job present only in the replay file's INITIAL snapshot must " + "still reach the real picker and receive a live PEND decision " + "(bug-1567)"); + + free(captured); + printf("test_job_present_only_in_initial_snapshot_still_reaches_the_" + "picker: PASS\n"); +} + int main(void) { UNITY_BEGIN(); @@ -559,6 +734,8 @@ int main(void) test_process_replay_event_advances_virtual_clock_to_exact_event_ts_in_file_order); RUN_TEST( test_run_processes_multiple_sessions_in_order_and_no_longer_returns_enosys); + RUN_TEST( + test_job_present_only_in_initial_snapshot_still_reaches_the_picker); return UNITY_END(); } diff --git a/src/scheduler/testsuite/test_sim_isolation.c b/src/scheduler/testsuite/test_sim_isolation.c index 17ed432c..bc8424ee 100644 --- a/src/scheduler/testsuite/test_sim_isolation.c +++ b/src/scheduler/testsuite/test_sim_isolation.c @@ -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: " 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; } diff --git a/src/scheduler/testsuite/test_sim_record_output.c b/src/scheduler/testsuite/test_sim_record_output.c index dc2c9724..f80c2142 100644 --- a/src/scheduler/testsuite/test_sim_record_output.c +++ b/src/scheduler/testsuite/test_sim_record_output.c @@ -79,6 +79,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c" diff --git a/src/scheduler/testsuite/test_sim_verify.c b/src/scheduler/testsuite/test_sim_verify.c index 3ed30563..7f112fcf 100644 --- a/src/scheduler/testsuite/test_sim_verify.c +++ b/src/scheduler/testsuite/test_sim_verify.c @@ -99,6 +99,7 @@ #include "../src/repl_params.c" #include "../src/sim.c" #include "../src/sim_loop.c" +#include "../src/sim_picker.c" /* bug-1567: sched_sim_driver_run() now calls sim_bootstrap_picker() (sim_loop.c-cap sibling) as an extern -- undefined reference at link time otherwise */ #include "../src/sim_record.c" #include "../src/sim_verify.c" #include "../src/snapshot.c"