From 31ef5d709387a70b5d8a1fa454af09d9cbef49dd Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 5 Aug 2026 04:18:01 +0000 Subject: [PATCH] test(scheduler-core): prove snapshot -EBUSY guard against a genuine active session (task 8.2) Task 8.1's own -EBUSY guard test only proved rejection against a synthetic single-thread session_active flag flip, with a comment misattributing the technique to test_core_full_cycle.c's actually- genuine-session precedent. Real gap, not busywork: xce_core_go() publishes session_active via memory_order_release while the snapshot guards read it via plain atomic_load() -- the synthetic same-thread test never exercised any real cross-thread visibility of that flag. Adds a genuine driver-driven session test (real picker thread + real xce_core_go()) proving both save and load reject with -EBUSY during the same active session, leaving state genuinely unchanged (sentinel bytes for save; a distinguishable non-existent queue + unchanged candidate count for load). Zero production changes -- task 8.1's guards were already correct. Closes out Group 8 (Core: Snapshot Serialization) entirely. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_019dw5WnN3DXStaEaPYfACU1 --- .kiro/specs/scheduler-core/tasks.md | 3 +- .../testsuite/test_snapshot_save_load.c | 142 ++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/.kiro/specs/scheduler-core/tasks.md b/.kiro/specs/scheduler-core/tasks.md index 7e826b81..a809190a 100644 --- a/.kiro/specs/scheduler-core/tasks.md +++ b/.kiro/specs/scheduler-core/tasks.md @@ -207,11 +207,12 @@ - _Depends: 7.3_ - _Verified: 2026-08-04 — xce_sched_core_save_snapshot()/xce_sched_core_load_snapshot() implemented, split across snapshot.c/snapshot_wire.c/snapshot_load.c (800-line cap) along real logical boundaries (save/load/shared-helpers). New prng_seed field added to xce_sched_core_t (a genuine, well-investigated gap requirements.md/design.md both presupposed but no prior task added), seeded to 0, no PRNG consumer wired yet (documented, deliberate). Cross-core behavioral-equivalence Observable proven for real: two independent cores, each with its own registered picker, save on one/load into a freshly-initialized other, both driven through an identical real session (picker thread + decision drain), decision sequences asserted identical including a pinned spot-check (201-PEND/202-DISPATCH/203-PEND on both). -EBUSY/-EINVAL guards checked before any mutation, tested directly. Found and fixed two real bugs along the way: a missing prng_seed restore in this task's own new code, and a genuine, independently-traced-to-the-primitive-level guard bug in already-committed core.c (sched_get_user/sched_get_project treating key_find()'s expected -ENOENT as fatal, silently breaking get-or-create for any new name) that also affected the live production path (make_sched_job(), JOBS_PUSH) — narrowly scoped two-line fix, folded into this task's diff since only this task's own test path discovered it. 23/23 scheduler suite, full project build clean, review APPROVED first pass with every claim independently re-derived._ -- [ ] 8.2 Enforce the snapshot session-idle precondition +- [x] 8.2 Enforce the snapshot session-idle precondition - Reject snapshot save/load calls made while a scheduling session is active, returning a distinct busy error rather than capturing or restoring torn state - Observable: attempting a snapshot save/load mid-session returns the busy error and leaves core state unchanged - _Requirements: 5.21_ - _Depends: 8.1_ + - _Verified: 2026-08-04 — task 8.1's own -EBUSY guard test only proved rejection against a synthetic single-thread session_active flag flip, and its comment misattributed the technique to test_core_full_cycle.c's actually-genuine-session precedent. Real gap, not busywork: xce_core_go() publishes session_active via memory_order_release while the snapshot guards read it via plain atomic_load() — the synthetic same-thread test never exercised any real cross-thread visibility of that flag. Added a genuine driver-driven session test (real picker thread + real xce_core_go()) proving both save and load reject with -EBUSY during the same active session, leaving state genuinely unchanged (sentinel bytes for save; distinguishable non-existent queue + unchanged candidate count for load) -- zero production changes, task 8.1's guards were already correct. 23/23 scheduler suite, 10/10 standalone flakiness-clean, review APPROVED first pass with an even sharper independent justification than the implementer's own. Closes out Group 8 (Core: Snapshot Serialization) entirely -- tasks 8.1-8.2 both complete._ - [ ] 9. Core: Recording Control diff --git a/src/scheduler/testsuite/test_snapshot_save_load.c b/src/scheduler/testsuite/test_snapshot_save_load.c index cdab661b..aae1cf11 100644 --- a/src/scheduler/testsuite/test_snapshot_save_load.c +++ b/src/scheduler/testsuite/test_snapshot_save_load.c @@ -515,6 +515,146 @@ test_snapshot_guards_reject_null_args_and_busy_session_without_mutating_state( atomic_store(&(core.session_active), 0); } +/* ---------------------------------------------------------------------- */ +/* Guard tests against a GENUINE active session (task 8.2, Req 5 AC 21) -- */ +/* the guard test above proves the check fires against a hand-set atomic; */ +/* this one drives a real session the same way */ +/* test_full_cycle_push_go_drain_end_with_precondition_rejections() */ +/* (test_core_full_cycle.c) proves xce_core_go()/xce_core_push_events()'s */ +/* own -EBUSY/-EINVAL guards reject reentrant calls while GENUINELY */ +/* active -- a real picker pthread plus a real xce_core_go() call, not a */ +/* synthetic session_active flip. */ +/* ---------------------------------------------------------------------- */ + +static void +test_snapshot_save_and_load_reject_during_genuinely_active_session(void) +{ + xce_sched_core_t core; + pthread_t picker_tid; + xce_core_event_t events[2]; + xce_queue_t wire_queue; + xce_job_t* job; + sched_snapshot_data_t snap; + sched_snapshot_data_t reject_in; + xce_queue_t reject_wire_queue; + xce_queue_t* reject_queues[1]; + xce_sched_queue_t* q_after; + xce_pendreason_t* reason; + xce_decision_t* decision; + test_driver_ctx_t driver_ctx; + int ncandidates_before; + int rc; + uint64_t jobid; + + memset(&driver_ctx, 0, sizeof(driver_ctx)); + memset(&g_stub_state_a, 0, sizeof(g_stub_state_a)); + fixture_init(&core, &g_stub_plugin_a); + + rc = xce_core_set_decision_cb(&core, test_on_decision, &driver_ctx); + TEST_ASSERT_EQUAL_INT(0, rc); + + /* =================================================================== + * Phase 1: real state while the session is inactive -- one open queue, + * one pending job (same shape as test_core_full_cycle.c's own Phase 1). + * =================================================================== */ + memset(&wire_queue, 0, sizeof(wire_queue)); + wire_queue.queue = "normal"; + wire_queue.priority = 1; + wire_queue.status = QUEUE_STAT_OPEN; + + job = make_job(301, JOB_STAT_PEND, "normal", "alice", "proj1"); + + memset(events, 0, sizeof(events)); + events[0].type = XCE_CORE_EVENT_QUEUE_UPDATE; + events[0].u.queue_update.queue = &wire_queue; + events[1].type = XCE_CORE_EVENT_JOBS_PUSH; + events[1].u.jobs_push.njobs = 1; + events[1].u.jobs_push.jobs = &job; + + rc = xce_core_push_events(&core, events, 2); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(1, g_stub_state_a.ncandidates); + ncandidates_before = g_stub_state_a.ncandidates; + + /* =================================================================== + * Phase 2: spawn the real picker thread, then start a GENUINE session + * via xce_core_go(). While it is active, both snapshot functions must + * be rejected with -EBUSY (distinct from the -EINVAL NULL-arg case + * task 8.1's own guard test above already covers), and must leave + * state genuinely untouched -- not just "returned an error code". + * =================================================================== */ + rc = pthread_create(&picker_tid, NULL, test_picker_thread_fn, &core); + TEST_ASSERT_EQUAL_INT(0, rc); + + rc = xce_core_go(&core); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT( + 1, atomic_load_explicit(&(core.session_active), memory_order_acquire)); + + /* save: sentinel bytes untouched -- proves the guard runs before + * save_snapshot()'s own memset(out, 0, ...), against a real session. */ + memset(&snap, 0xAA, sizeof(snap)); + rc = xce_sched_core_save_snapshot(&core, &snap); + TEST_ASSERT_EQUAL_INT(-EBUSY, rc); + TEST_ASSERT_EQUAL_UINT8(0xAA, ((uint8_t*)&snap)[0]); + + /* load: a distinguishable payload (a queue name never pushed to this + * core) proves the load never ran -- if the guard were bypassed, this + * queue would be added and the picker would gain a 2nd candidate. */ + memset(&reject_wire_queue, 0, sizeof(reject_wire_queue)); + reject_wire_queue.queue = "should-not-appear"; + reject_wire_queue.priority = 99; + reject_wire_queue.status = QUEUE_STAT_OPEN; + reject_queues[0] = &reject_wire_queue; + + memset(&reject_in, 0, sizeof(reject_in)); + reject_in.queues = reject_queues; + reject_in.n_queues = 1; + + rc = xce_sched_core_load_snapshot(&core, &reject_in); + TEST_ASSERT_EQUAL_INT(-EBUSY, rc); + TEST_ASSERT_NULL(sched_get_queue(&core, "should-not-appear")); + + q_after = sched_get_queue(&core, "normal"); + TEST_ASSERT_NOT_NULL(q_after); + TEST_ASSERT_EQUAL_INT(1, q_after->priority); + TEST_ASSERT_EQUAL_INT(ncandidates_before, g_stub_state_a.ncandidates); + + /* both rejections happened without the session ever having ended. */ + TEST_ASSERT_EQUAL_INT( + 1, atomic_load_explicit(&(core.session_active), memory_order_acquire)); + + /* =================================================================== + * Phase 3: drain the still-running real session to completion -- same + * drain protocol run_identical_session() above uses (decision content + * is not under test here, only that the session ends cleanly). + * =================================================================== */ + for (;;) { + decision = NULL; + + cbuf_get(&(core.jbuf), &jobid); + if (jobid == 0) { + cbuf_put(&(core.decisions), &decision); + break; + } + + reason = make_pendreason(jobid); + decision = make_pend_decision(reason); + TEST_ASSERT_NOT_NULL(decision); + cbuf_put(&(core.decisions), &decision); + } + + rc = pthread_join(picker_tid, NULL); + TEST_ASSERT_EQUAL_INT(0, rc); + + (void)drain_and_dispatch(&core); + + rc = xce_core_end_session(&core); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT( + 0, atomic_load_explicit(&(core.session_active), memory_order_acquire)); +} + /* ---------------------------------------------------------------------- */ /* The Observable, end to end. */ /* ---------------------------------------------------------------------- */ @@ -840,6 +980,8 @@ int main(void) UNITY_BEGIN(); RUN_TEST( test_snapshot_guards_reject_null_args_and_busy_session_without_mutating_state); + RUN_TEST( + test_snapshot_save_and_load_reject_during_genuinely_active_session); RUN_TEST( test_snapshot_save_then_load_produces_behaviorally_equivalent_core); return UNITY_END();