feat(scheduler-allocation): clear unsatisfiable-resreq cache per session

Clear the cache inside core_begin_session_internal() so a job cached as
unsatisfiable in one session is re-evaluated fresh in the next (Req 7.2).

An initial weak-symbol cast-based approach was rejected: it corrupted
xce_sched_sim_driver_t.alt_events_file, since core_begin_session_internal()
is shared between driver.c (real xce_scheduler_t) and sim_loop.c (a
structurally different, deliberately isolated sim driver struct with no
cache field at the same byte offset) -- the strong cache_clear() symbol
won unconditionally for both, invisible to ASAN and untested by any
test_sim_*.c file.

Fixed via a driver-registered callback: xce_sched_core_t gains a
cache_clear_cb hook (NULL by default, per xce_sched_core_init()'s
full-struct memset), set only in main.c's driver-mode branch --
mirroring the existing on_decision/xce_core_set_decision_cb() pattern.
--sim mode returns before that registration is ever reached, so its hook
stays NULL and core_begin_session_internal()'s guarded call is a no-op.
The real xce_scheduler_t-casting adapter lives in alloc_cache.c rather
than driver.c, since ~11 tests white-box-include driver.c without
linking alloc_cache.c.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYdtrEWWZfKRRGED7raEYg

src/scheduler/testsuite/meson.build

@@ -1579,6 +1579,58 @@ sched_alloc_cache_test = executable(
     dependencies: [scheduler_test_deps],
     install: false)
 
+# Unit test for task 5.2 (scheduler-allocation spec, Req 7.2): clearing the
+# unsatisfiable-resreq cache at the start of every scheduling session, from
+# inside core_begin_session_internal() (core_session.c), via the
+# driver-registered xce_core_cache_clear_cb_t hook (round 2 -- replaces
+# round 1's unconditional weak-symbol cache_clear() call, found to be a
+# CRITICAL type-confusion bug against simulation-mode sessions; see
+# test_sim_cache_clear_isolation_test below). See this test file's own
+# header comment for why it lists core.c/core_clock.c/core_session.c/
+# decision.c/alloc_cache.c as genuinely SEPARATE compiled sources rather
+# than white-box #include-ing them into one TU (the same "real,
+# separately-linked scheduler internals" technique task 5.1's own
+# sched_cache_dispatch_test above already established for this feature,
+# scaled down to just the files this narrower test needs) -- this test's
+# own registered cache_clear_cb calls the REAL, strong cache_clear()
+# (alloc_cache.c), which is only meaningfully distinguishable from a stub
+# across genuinely separate object files, never within one merged
+# translation unit.
+sched_core_session_cache_clear_test = executable(
+    'sched_core_session_cache_clear_test',
+    ['test_core_session_cache_clear.c',
+     '../src/core.c',
+     '../src/core_clock.c',
+     '../src/core_session.c',
+     '../src/decision.c',
+     '../src/alloc_cache.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    install: false)
+
+# Regression test for task 5.2 ROUND 2 (scheduler-allocation spec, Req 7.2):
+# the CRITICAL defect that got round 1 REJECTED -- core_begin_session_
+# internal() must NEVER touch xce_sched_sim_driver_t's alt_events_file (or
+# any other sim-only field) even with alloc_cache.c's real, strong
+# cache_clear(xce_scheduler_t*) linked into the same binary, because
+# sim_loop.c never registers a cache_clear_cb (Req 5 AC 17 "Simulation
+# isolation" -- a simulation-mode core has no cache to clear at all). Same
+# separate-compiled-sources technique as sched_core_session_cache_clear_test
+# above, for the same reason: this test needs the real, strong
+# cache_clear() actually present in the binary to prove the NULL-hook path
+# still never calls it.
+sched_sim_cache_clear_isolation_test = executable(
+    'sched_sim_cache_clear_isolation_test',
+    ['test_sim_cache_clear_isolation.c',
+     '../src/core.c',
+     '../src/core_clock.c',
+     '../src/core_session.c',
+     '../src/decision.c',
+     '../src/alloc_cache.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    install: false)
+
 # Task 3.1 (scheduler-allocation spec, Req 9.1, 4.6): seqjob_allocator.c's
 # rewrap onto the shared plan_iter/host_iter interfaces, plus the new
 # maxTasks==1 guard on seqjob_prepare_allocation() (Req 4.6). Same
@@ -1823,6 +1875,12 @@ test('scheduler host iter test',
 test('scheduler alloc cache test',
      sched_alloc_cache_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler core session cache clear test',
+     sched_core_session_cache_clear_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler sim cache clear isolation test',
+     sched_sim_cache_clear_isolation_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])
 test('scheduler seqjob allocator test',
      sched_seqjob_allocator_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])

src/scheduler/testsuite/test_core_session_cache_clear.c

@@ -0,0 +1,237 @@
+/*
+ * test_core_session_cache_clear.c - scheduler-allocation spec, task 5.2
+ * (Req 7.2): the per-session unsatisfiable-resreq cache (alloc_cache.c,
+ * task 2.2) must be cleared at the start of every scheduling session, from
+ * inside core_begin_session_internal() (core_session.c), alongside the
+ * other per-session state that function already resets.
+ *
+ * ── Link technique (round 2) ─────────────────────────────────────────────
+ * Round 1 called alloc_cache.c's real cache_clear(xce_scheduler_t*)
+ * unconditionally through a same-named WEAK fallback declared in
+ * core_session.c, relying on link-time weak/strong override -- discovered
+ * CRITICALLY broken (bug: every simulation-mode session reinterpreted
+ * xce_sched_sim_driver_t's alt_events_file FILE* as the cache, because
+ * weak/strong resolution picks one winner process-wide, not per call
+ * site). Round 2 replaces this with a driver-registered hook
+ * (core.h's xce_core_cache_clear_cb_t / xce_core_set_cache_clear_cb()):
+ * core_begin_session_internal() now calls core->cache_clear_cb(core) only
+ * if non-NULL, and this fixture explicitly registers the REAL production
+ * adapter (alloc_cache.c's xce_alloc_cache_clear_cb() -- see that
+ * function's own doc comment for why it lives in alloc_cache.c rather
+ * than driver.c) on init_sched()'s xce_scheduler_t, the same call main.c
+ * makes in production.
+ *
+ * This file still does NOT use the white-box #include convention most of
+ * this test suite uses for core_session.c; its meson.build entry still
+ * lists core.c/core_clock.c/core_session.c/decision.c/alloc_cache.c as
+ * genuinely separate compiled sources so this fixture links against the
+ * REAL cache_clear(xce_scheduler_t*) (alloc_cache.c) that its registered
+ * hook calls into -- not a stub.
+ *
+ * ── What this proves ─────────────────────────────────────────────────────
+ * A job's fingerprint+identity is inserted into a REAL, live
+ * xce_scheduler_t.unsatisfiable_cache (cache_insert(), simulating what
+ * task 5.1's allocator-exhaustion path would have done); a lookup for the
+ * identical key is confirmed a HIT; the REAL xce_core_begin_session()
+ * (core_session.c's public entry point, exercising the real
+ * core_begin_session_internal() and this fixture's registered
+ * cache_clear_cb) is then called; a lookup for the SAME key is then
+ * confirmed a MISS -- proving the cache was genuinely emptied, not merely
+ * marked stale, at the exact per-session reset point Req 7.2 names. A
+ * second test proves the cache is still genuinely usable (insert+lookup
+ * behaves normally) immediately after a begin_session clear, ruling out a
+ * cache left permanently broken by whatever cache_clear() does internally.
+ * See test_sim_cache_clear_isolation.c (new, round 2) for the companion
+ * proof that a simulation-mode core -- which never registers this hook --
+ * is NEVER touched by it.
+ *
+ * kill_sched_* / plugin_unregister / sched_worker_release_host_ctx stubs
+ * below are identical, for identical reasons, to test_core_session.c's own (see
+ * that file's header comment) -- this file calls the exact same
+ * xce_sched_core_init()/xce_core_begin_session()/xce_core_end_session()
+ * entry points, just with alloc_cache.c linked in as a separate source
+ * alongside core.c/core_clock.c/core_session.c/decision.c instead of
+ * white-box #include'd.
+ */
+#include <errno.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#include "core.h"
+#include "sched/alloc_cache.h"
+
+void kill_sched_job(void* arg) { (void)arg; }
+void kill_sched_queue(void* arg) { (void)arg; }
+void kill_sched_worker(void* arg) { (void)arg; }
+void kill_sched_user(void* arg) { (void)arg; }
+void kill_sched_project(void* arg) { (void)arg; }
+void kill_sched_resval(xce_val_t* val) { (void)val; }
+void plugin_unregister(void* arg) { (void)arg; }
+
+/* sched_worker_release_host_ctx (resource.c) is outside this test's linked
+ * source set (only core.c/core_clock.c/core_session.c/decision.c/
+ * alloc_cache.c, meson.build) -- stubbed the same way test_core_session.c
+ * stubs it: satisfies the linker for core_reset_workers()'s call
+ * (core_session.c). This fixture never adds a worker, so it is never
+ * actually invoked at runtime either. */
+void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; }
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+/* ---------------------------------------------------------------------- */
+/* Fixture helpers                                                         */
+/* ---------------------------------------------------------------------- */
+
+/*
+ * init_sched - a real xce_scheduler_t with BOTH core (xce_sched_core_init(),
+ * core.c) and unsatisfiable_cache (mirroring sched_init()'s own
+ * construction call, scheduler.c, and test_alloc_cache.c's own
+ * init_sched()) constructed -- everything core_begin_session_internal()
+ * touches, without sched_init()'s heavier networking/threading setup
+ * (this test never calls sched_init() at all). Also registers the REAL
+ * production cache_clear_cb (round 2, xce_alloc_cache_clear_cb() --
+ * alloc_cache.c, linked into this exact test binary, see this file's own
+ * header comment) -- the same one-time-at-startup registration main.c
+ * performs in production (xce_core_set_cache_clear_cb(), right alongside
+ * xce_core_set_decision_cb()), so this fixture's own registration is not
+ * a stand-in wrapper but the identical call production makes.
+ */
+static void init_sched(xce_scheduler_t* sched)
+{
+    kv_params_t params = { .nslots     = XCE_UNSAT_CACHE_NSLOTS,
+                           .keyops     = &cache_key_ops,
+                           .datakiller = NULL };
+    int         rc;
+
+    memset(sched, 0, sizeof(*sched));
+
+    rc = xce_sched_core_init(&(sched->core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    rc = xce_core_set_cache_clear_cb(&(sched->core), xce_alloc_cache_clear_cb);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    sched->unsatisfiable_cache = makeInstance(&kvtab_type, &params);
+    TEST_ASSERT_NOT_NULL(sched->unsatisfiable_cache);
+}
+
+/*
+ * init_primary_obj - identical in shape to test_alloc_cache.c's own helper
+ * (task 2.2): a minimal, valid xce_sched_obj_t/xce_primary_schedobj_t pair
+ * ready for cache_insert()/cache_lookup().
+ */
+static void init_primary_obj(xce_sched_obj_t*        obj,
+                             xce_primary_schedobj_t* primary, int min_tasks,
+                             int max_tasks, uint32_t select_node_id,
+                             uint64_t rusage_hash, const char* user,
+                             const char* queue, const char* project)
+{
+    memset(obj, 0, sizeof(*obj));
+    memset(primary, 0, sizeof(*primary));
+
+    primary->resreq_fp.min_tasks      = min_tasks;
+    primary->resreq_fp.max_tasks      = max_tasks;
+    primary->resreq_fp.select_node_id = select_node_id;
+    primary->resreq_fp.rusage_hash    = rusage_hash;
+
+    obj->tag       = XCE_RESREQ_PRIMARY;
+    obj->u.primary = primary;
+    obj->user      = (char*)user;
+    obj->queue     = (char*)queue;
+    obj->project   = (char*)project;
+}
+
+/* ---------------------------------------------------------------------- */
+/* Tests                                                                   */
+/* ---------------------------------------------------------------------- */
+
+/*
+ * THE Observable this task adds (Req 7.2): a job cached as unsatisfiable
+ * in one session is re-evaluated fresh -- not immediately pended from the
+ * cache -- in the next session. Proven end to end through the REAL
+ * cache_insert()/xce_core_begin_session()/cache_lookup(), not a stub.
+ */
+static void test_begin_session_clears_previously_cached_entry(void)
+{
+    xce_scheduler_t        sched;
+    xce_sched_obj_t        obj;
+    xce_primary_schedobj_t primary;
+    int                    category = 7;
+    int                    out      = -1;
+    int                    rc;
+
+    init_sched(&sched);
+    init_primary_obj(&obj, &primary, 1, 4, 100u, 0xdeadbeefULL, "alice",
+                     "normal", "proj1");
+
+    rc = cache_insert(&sched, &obj, category);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    /* Baseline: proves the fixture/insert are sound before relying on them
+     * below -- same "prove the harness works first" discipline
+     * test_core_session.c's own worker-reset tests already follow. */
+    rc = cache_lookup(&sched, &obj, &out);
+    TEST_ASSERT_EQUAL_INT(1, rc);
+    TEST_ASSERT_EQUAL_INT(category, out);
+
+    rc = xce_core_begin_session(&(sched.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    out = -1;
+    rc  = cache_lookup(&sched, &obj, &out);
+    TEST_ASSERT_EQUAL_INT_MESSAGE(
+        0, rc,
+        "Req 7.2: a job cached as unsatisfiable in one session must be "
+        "re-evaluated fresh (a cache MISS), not immediately pended from "
+        "the cache, in the next session -- core_begin_session_internal() "
+        "must clear xce_scheduler_t.unsatisfiable_cache");
+
+    rc = xce_core_end_session(&(sched.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+}
+
+/*
+ * The cleared cache must still be genuinely USABLE afterward -- not left
+ * permanently broken by whatever cache_clear() does internally (it
+ * destroys and reconstructs the backing kvtab_t -- see that function's own
+ * comment, sched/alloc_cache.h). A fresh insert+lookup after begin_session
+ * must behave exactly like any other insert+lookup.
+ */
+static void test_cache_still_usable_after_begin_session_clear(void)
+{
+    xce_scheduler_t        sched;
+    xce_sched_obj_t        obj;
+    xce_primary_schedobj_t primary;
+    int                    out = -1;
+    int                    rc;
+
+    init_sched(&sched);
+
+    rc = xce_core_begin_session(&(sched.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    init_primary_obj(&obj, &primary, 2, 8, 200u, 0xfeedfaceULL, "bob", "batch",
+                     "proj2");
+
+    rc = cache_insert(&sched, &obj, 3);
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    rc = cache_lookup(&sched, &obj, &out);
+    TEST_ASSERT_EQUAL_INT(1, rc);
+    TEST_ASSERT_EQUAL_INT(3, out);
+
+    rc = xce_core_end_session(&(sched.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_begin_session_clears_previously_cached_entry);
+    RUN_TEST(test_cache_still_usable_after_begin_session_clear);
+    return UNITY_END();
+}

src/scheduler/testsuite/test_sim_cache_clear_isolation.c

@@ -0,0 +1,140 @@
+/*
+ * test_sim_cache_clear_isolation.c - scheduler-allocation spec, task 5.2
+ * round 2 (Req 7.2 regression, mandated by the round-1 review that
+ * REJECTED this task for a CRITICAL memory-corruption defect).
+ *
+ * ── The bug this guards against ──────────────────────────────────────────
+ * core_begin_session_internal() (core_session.c) is reached from BOTH
+ * driver.c's xce_sched_driver_run_normal_mode_round() (a real
+ * xce_scheduler_t*, which HAS an unsatisfiable_cache field, scheduler.h)
+ * and sim_loop.c's sched_sim_driver_run() (a structurally different
+ * xce_sched_sim_driver_t*, sim.h -- Req 5 AC 17 "Simulation isolation" --
+ * which has NO unsatisfiable_cache field at all). Round 1 called
+ * alloc_cache.c's real, strong cache_clear(xce_scheduler_t*) via a
+ * same-named WEAK fallback declared in core_session.c, relying on
+ * link-time weak/strong symbol resolution. Since that resolution picks
+ * ONE winner for the whole binary (not per call site), BOTH the driver.c
+ * call (correct) and the sim_loop.c call (WRONG -- there is no cache to
+ * clear) resolved to the same real cache_clear(xce_scheduler_t*), which
+ * reinterpreted whatever field sits at unsatisfiable_cache's byte offset
+ * in xce_sched_sim_driver_t as a kvtab_t* and clobbered it. That offset is
+ * `alt_events_file` (sim.h) -- a live FILE* every --sim-alt-events run
+ * depends on. Empirically confirmed by this task's round-1 reviewer, and
+ * reproduced by this exact test file before round 2's fix (see this
+ * task's own Status Report for the before/after run).
+ *
+ * ── The fix this proves ──────────────────────────────────────────────────
+ * core_begin_session_internal() now calls a driver-registered hook
+ * (core->cache_clear_cb, core.h's xce_core_cache_clear_cb_t) ONLY if
+ * non-NULL, instead of calling cache_clear() unconditionally.
+ * sim_loop.c never calls xce_core_set_cache_clear_cb() -- a simulation-
+ * mode xce_sched_core_t's hook therefore stays NULL (xce_sched_core_
+ * init()'s memset() default), and core_begin_session_internal() treats
+ * NULL as "this driver has no cache to clear" and skips the call
+ * entirely. This test constructs a real xce_sched_sim_driver_t, points
+ * alt_events_file at a real, open FILE*, deliberately does NOT register
+ * any cache_clear_cb (mirroring production sim_loop.c exactly), calls the
+ * REAL xce_core_begin_session() (core_session.c's public entry point),
+ * and asserts alt_events_file is bit-for-bit unchanged across the call.
+ *
+ * ── Link technique ───────────────────────────────────────────────────────
+ * Same technique as the sibling test_core_session_cache_clear.c (round 1):
+ * this fixture's meson.build entry lists core.c/core_clock.c/
+ * core_session.c/decision.c/alloc_cache.c as genuinely SEPARATE compiled
+ * sources rather than white-box #include-ing them, specifically so
+ * alloc_cache.c's real, strong cache_clear(xce_scheduler_t*) IS present
+ * in this binary -- if a future regression reintroduced an unconditional
+ * call to it, this test would still catch it (a white-box #include of
+ * core_session.c alone, without also linking alloc_cache.c, could not
+ * have caught round 1's bug at all -- that is exactly why round 1's bug
+ * was invisible to the existing suite).
+ *
+ * kill_sched_* / plugin_unregister / sched_worker_release_host_ctx stubs
+ * below are identical, for identical reasons, to test_core_session_
+ * cache_clear.c's own (see that file's header comment).
+ */
+#include <errno.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <unity/unity.h>
+
+#include "sched/alloc_cache.h"
+#include "sim.h"
+
+void kill_sched_job(void* arg) { (void)arg; }
+void kill_sched_queue(void* arg) { (void)arg; }
+void kill_sched_worker(void* arg) { (void)arg; }
+void kill_sched_user(void* arg) { (void)arg; }
+void kill_sched_project(void* arg) { (void)arg; }
+void kill_sched_resval(xce_val_t* val) { (void)val; }
+void plugin_unregister(void* arg) { (void)arg; }
+
+/* sched_worker_release_host_ctx (resource.c) is outside this test's linked
+ * source set (only core.c/core_clock.c/core_session.c/decision.c/
+ * alloc_cache.c, meson.build) -- this fixture never adds a worker, so it
+ * is never actually invoked at runtime either. */
+void sched_worker_release_host_ctx(xce_sched_worker_t* w) { (void)w; }
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+/*
+ * THE Observable this regression guards (Req 7.2, round-2 review
+ * requirement): a simulation-mode xce_sched_core_t, which never registers
+ * a cache_clear_cb, must never have any of its sibling xce_sched_sim_
+ * driver_t fields touched by xce_core_begin_session() -- even with the
+ * real, strong alloc_cache.c cache_clear(xce_scheduler_t*) linked into
+ * the very same binary.
+ */
+static void test_sim_begin_session_never_corrupts_alt_events_file(void)
+{
+    xce_sched_sim_driver_t sim;
+    FILE*                  f;
+    int                    rc;
+
+    memset(&sim, 0, sizeof(sim));
+
+    rc = xce_sched_core_init(&(sim.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    /* A real, live FILE* -- exactly what a --sim-alt-events run holds
+     * here in production. tmpfile() needs no cleanup path/permissions
+     * setup and is auto-removed on fclose()/process exit. */
+    f = tmpfile();
+    TEST_ASSERT_NOT_NULL(f);
+    sim.alt_events_file = f;
+
+    /* Deliberately NOT calling xce_core_set_cache_clear_cb() here --
+     * mirrors sim_loop.c exactly, which never registers this hook (Req 5
+     * AC 17 "Simulation isolation": sim mode has no cache to clear). This
+     * is the crux of the regression: before round 2's fix,
+     * core_begin_session_internal() called cache_clear() unconditionally,
+     * so linking alloc_cache.c's real, strong cache_clear(xce_scheduler_t*)
+     * into this binary was enough to corrupt alt_events_file below even
+     * though nothing here ever asked for a cache to be cleared. */
+    rc = xce_core_begin_session(&(sim.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    TEST_ASSERT_EQUAL_PTR_MESSAGE(
+        f, sim.alt_events_file,
+        "xce_core_begin_session() corrupted xce_sched_sim_driver_t's "
+        "alt_events_file -- core_begin_session_internal() must treat a "
+        "NULL cache_clear_cb as a true no-op (Req 7.2 round 2), never "
+        "fall through to alloc_cache.c's xce_scheduler_t*-typed "
+        "cache_clear() for a simulation-mode core");
+
+    rc = xce_core_end_session(&(sim.core));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    fclose(f);
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_sim_begin_session_never_corrupts_alt_events_file);
+    return UNITY_END();
+}