From 4f94638a1d14cee2958b93608c4899294990758b Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Mon, 21 Sep 2026 12:58:57 +0000 Subject: [PATCH] fix(scheduler): route sim reconciliation's on_dispatch() through the allocator thread (bug-1837) sim_reconcile_unaccounted_jobs() (sim_loop.c) was calling picker->picker_iface->on_dispatch() directly from the sim driver thread, violating plugin.h's documented "allocator thread only" contract for Post-allocation hooks. Dormant in practice only because builtin_fcfs.c happens to lock internally around every hook -- a defensive measure the contract text doesn't actually require of picker plugins. Since scheduler-sim-live-allocation, sim mode spawns the same real sched_job_picker()/sched_allocator() thread pair normal mode uses, so a real allocator thread now exists to route through. Added a reconcile_req/reconcile_ack cbuf pair on xce_sched_core_t and a reserved XCE_SCHED_RECONCILE_WAKE sentinel pushed through jbuf to wake the allocator thread without touching jbuf's pinned capacity-1/item-type invariant (Req 5 AC 7). sched_allocator() services the request and calls on_dispatch() itself; sim_reconcile_invoke_on_dispatch() blocks on the ack. Falls back to a direct call when no allocator thread is live for a given core (standalone unit tests), which is safe by construction. TDD: new test spawns a stand-in allocator-relay thread and asserts on_dispatch() fires there, not on the caller's thread -- confirmed RED against the old direct-call code, GREEN after. Four other test files with hand-rolled sched_allocator() stubs needed the same relay branch (found by an actual hang at meson's timeout). Full scheduler suite: 85/85, zero regressions. Co-Authored-By: Claude Sonnet 5 --- .../design.md | 2 + .wolf/anatomy-index.json | 118 ++++++----- .wolf/anatomy.md | 24 ++- .wolf/buglog.json | 11 +- .wolf/memory.md | 26 +++ src/scheduler/include/core.h | 30 +++ src/scheduler/include/sim.h | 19 ++ src/scheduler/src/core.c | 13 ++ src/scheduler/src/sched_election.c | 21 ++ src/scheduler/src/sim_loop.c | 59 +++++- .../testsuite/test_sim_contention_parity.c | 24 +++ .../test_sim_e2e_original_motivation.c | 24 +++ .../test_sim_reconcile_unaccounted.c | 189 ++++++++++++++++++ .../test_sim_round_boundary_reconciliation.c | 24 +++ src/scheduler/testsuite/test_sim_verify.c | 24 +++ 15 files changed, 541 insertions(+), 67 deletions(-) diff --git a/.kiro/specs/scheduler-sim-eager-reconciliation/design.md b/.kiro/specs/scheduler-sim-eager-reconciliation/design.md index 0496baca..357d26b5 100644 --- a/.kiro/specs/scheduler-sim-eager-reconciliation/design.md +++ b/.kiro/specs/scheduler-sim-eager-reconciliation/design.md @@ -25,6 +25,8 @@ **Correction (2026-09-21, bug-1838, superseding the 2026-09-11 revision above):** the 2026-09-11 widening (capture "regardless of the job's current state," including PEND) turned out to be the root cause of a live-observed over-dispatch: a 22-slot host took 44 concurrent single-slot jobs during replay (bug-1836), because a PEND job's recorded DISPATCH decision was force-applied by reconciliation with no capacity check against current resource state. bug-1836 first closed the immediate hole by adding a capacity gate (`sim_reconciled_alloc_fits_capacity()`) to reconciliation itself — but investigation into why a PEND job could ever reach this force-apply path at all found that the underlying justification for the 2026-09-11 widening no longer holds: the since-completed `scheduler-sim-live-allocation` spec gave simulation mode a genuine, capacity-checked live picker/allocator thread pair (`sched_job_picker()`/`sched_allocator()`, `sched_election.c`) that both drivers now spawn identically — "one core, two consumers" (`.kiro/steering/scheduler-driver-equivalence.md`) is now actually true for dispatch, not just aspirational. A freshly-PEND job now correctly reaches that SAME live capacity-checked path simulation mode itself runs, exactly like normal mode; reconciliation force-applying a stale recorded decision for it is redundant at best and a second, competing source of truth at worst. The trigger is therefore narrowed back to RUN-only (`JOB_IS_RUN(job->state)`, mutually excluding DONE/EXIT for free under this codebase's 4-state job model): reconciliation's remaining, genuinely necessary scope is a job already confirmed RUN (via a separately-recorded `JOB_STATE_CHANGE`/`JOB_UPDATE` event) whose allocation data was never captured — structurally invisible to the live picker, since `next_job()` never offers a RUN job. This does NOT reopen the 2026-09-11 revision's OTHER change (synchronous RUN-transition-at-capture for the jobs reconciliation still does handle) — that remains correct and unchanged; only the ELIGIBILITY condition for entering capture at all is revised. See `sim_reconcile_unaccounted_jobs()`'s own updated header comment (`sim_loop.c`) for the complete before/after rationale, and Requirement 1's own 2026-09-21 correction (requirements.md) for the corresponding Acceptance Criteria update. +**Correction (2026-09-21, bug-1837, user-directed):** the `on_dispatch()` call bug-1759/bug-1779's own correction above added (2026-09-07) called `picker_iface->on_dispatch()` directly from `sim_reconcile_unaccounted_jobs()`'s own caller thread (`sched_sim_driver_run()`'s driver thread) — but `plugin.h` documents `on_dispatch()`/`on_pend()` as "Post-allocation hooks (MUST) -- allocator thread, after each allocation attempt," a threading-contract violation an `/xce-audit-invariants scheduler` run flagged as dormant/inert (real only because `builtin_fcfs.c`'s own internal locking happened to make every call site safe, a defensive measure plugin.h's contract text does not actually require of picker plugins in general). Now that `scheduler-sim-live-allocation` gives simulation mode the SAME real `sched_job_picker()`/`sched_allocator()` thread pair normal mode spawns (see the 2026-09-21 bug-1838 correction above), a real allocator thread genuinely exists to route this call through instead of calling it directly: `sim_reconcile_invoke_on_dispatch()` (`sim_loop.c`) relays the request to `sched_allocator()` (`sched_election.c`) via two dedicated cbufs on `xce_sched_core_t` (`core.h`) -- `reconcile_req`/`reconcile_ack` -- waking it through a reserved jbuf sentinel (`XCE_SCHED_RECONCILE_WAKE`) rather than widening jbuf's own pinned item type/capacity (`test_core_jbuf_capacity_pin.c`, Req 5 AC 7). A standalone caller with no live allocator thread for its core (e.g. `test_sim_reconcile_unaccounted.c`'s own direct-call unit tests) still gets a direct call, safe by construction since only one thread exists to touch picker state in that case. This is a threading-mechanism correction only -- reconciliation's own trigger condition, capacity gate, and RUN-only eligibility (both corrections above) are unchanged. + ## Boundary Commitments ### This Spec Owns diff --git a/.wolf/anatomy-index.json b/.wolf/anatomy-index.json index 2ba96eaf..0c851dc4 100644 --- a/.wolf/anatomy-index.json +++ b/.wolf/anatomy-index.json @@ -1,13 +1,13 @@ { "version": 1, "meta": { - "lastScanned": "2026-09-21T09:31:36.646Z", - "fileCount": 668, + "lastScanned": "2026-09-21T11:41:46.809Z", + "fileCount": 670, "hits": 0, "misses": 0, - "renderedHash": "56a6cca5dea3c105bb5d64b41019c5e07814ffd9ab069c9aab6f152e32fce1f2", - "storeUpdatedAt": "2026-09-21T09:31:36.655Z", - "rootHash": "cca4c863d5077424" + "renderedHash": "9704b1ab4907098decf513d0818bd0fb1c4f301de23b9de49ce6882672c6da25", + "storeUpdatedAt": "2026-09-21T11:41:46.817Z", + "rootHash": "e3297d48be0b28ab" }, "files": { ".agent-shell/transcripts/2026-08-11-11-39-59.md": { @@ -3225,11 +3225,11 @@ }, ".kiro/specs/scheduler-sim-eager-reconciliation/design.md": { "description": "Design Document", - "tokens": 7078, - "hash": "388510e52bf7c634", - "size": 28419, - "mtimeMs": 1789980075150.255, - "updatedAt": "2026-09-21T08:41:15.194Z", + "tokens": 7534, + "hash": "00d77598996cc78b", + "size": 30247, + "mtimeMs": 1789990906771.3093, + "updatedAt": "2026-09-21T11:41:46.809Z", "source": "hook" }, ".kiro/specs/scheduler-sim-eager-reconciliation/requirements.md": { @@ -9944,38 +9944,38 @@ }, "src/scheduler/include/sim.h": { "description": "if !defined(_SIM_H_)", - "tokens": 4213, - "hash": "2832b81c98fbb84a", - "size": 15797, - "mtimeMs": 1789118690779.5962, - "updatedAt": "2026-09-11T09:24:50.824Z", + "tokens": 4579, + "hash": "ec049ebdf9d49f43", + "size": 17169, + "mtimeMs": 1789987410813.0005, + "updatedAt": "2026-09-21T10:43:30.855Z", "source": "hook" }, "src/scheduler/src/sim_loop.c": { "description": "include ", - "tokens": 22511, - "hash": "6bacd553e27e51b7", - "size": 84414, - "mtimeMs": 1789977028368.3345, - "updatedAt": "2026-09-21T07:50:28.417Z", + "tokens": 23299, + "hash": "667da1ed4512bed0", + "size": 87369, + "mtimeMs": 1789990380544.1218, + "updatedAt": "2026-09-21T11:33:00.598Z", "source": "hook" }, "src/scheduler/testsuite/test_sim_reconcile_unaccounted.c": { "description": "Declares documents", - "tokens": 34635, - "hash": "ae87004ae3cee9a6", - "size": 129879, - "mtimeMs": 1789979940956.2974, - "updatedAt": "2026-09-21T08:39:00.991Z", + "tokens": 36877, + "hash": "6fdeab919a508b53", + "size": 138288, + "mtimeMs": 1789987855400.8535, + "updatedAt": "2026-09-21T10:50:55.452Z", "source": "hook" }, "src/scheduler/testsuite/test_sim_contention_parity.c": { "description": "Declares calls", - "tokens": 21740, - "hash": "d824c6b350e69b1d", - "size": 81524, - "mtimeMs": 1789978567598.1646, - "updatedAt": "2026-09-21T08:16:07.642Z", + "tokens": 22061, + "hash": "2e6fd463398ac8c0", + "size": 82727, + "mtimeMs": 1789989606580.9124, + "updatedAt": "2026-09-21T11:20:06.629Z", "source": "hook" }, "reports/sep06-sep12-2026.md": { @@ -11566,11 +11566,11 @@ }, "src/scheduler/include/core.h": { "description": "if !defined(_CORE_H_)", - "tokens": 11223, - "hash": "cc63291ebec75cc8", - "size": 42083, - "mtimeMs": 1789969624960.9233, - "updatedAt": "2026-09-21T05:47:05.008Z", + "tokens": 11793, + "hash": "6dac6d713ac66270", + "size": 44221, + "mtimeMs": 1789987351656.3103, + "updatedAt": "2026-09-21T10:42:31.689Z", "source": "hook" }, "src/scheduler/include/sched/alloc_cache.h": { @@ -11701,11 +11701,11 @@ }, "src/scheduler/testsuite/test_sim_e2e_original_motivation.c": { "description": "", - "tokens": 18078, - "hash": "76a64665f421db65", - "size": 67790, - "mtimeMs": 1789733394844.816, - "updatedAt": "2026-09-18T12:09:54.894Z", + "tokens": 19514, + "hash": "1886f99720ba5ae9", + "size": 73176, + "mtimeMs": 1789989633059.8286, + "updatedAt": "2026-09-21T11:20:33.100Z", "source": "hook" }, "src/scheduler/testsuite/test_sim_resource_contention.c": { @@ -11764,11 +11764,11 @@ }, "src/scheduler/src/sched_election.c": { "description": "Declares Core", - "tokens": 2964, - "hash": "0833041a4dd5a46e", - "size": 11113, - "mtimeMs": 1789750266742.442, - "updatedAt": "2026-09-18T16:51:06.779Z", + "tokens": 3249, + "hash": "6552587ac2699257", + "size": 12182, + "mtimeMs": 1789987391435.319, + "updatedAt": "2026-09-21T10:43:11.470Z", "source": "hook" }, "src/scheduler/testsuite/test_picker_core_loop_boundary.c": { @@ -12219,11 +12219,11 @@ }, "src/scheduler/testsuite/test_sim_verify.c": { "description": "Declares mismatch", - "tokens": 18569, - "hash": "ce46afa8186ebac0", - "size": 69631, - "mtimeMs": 1789978226655.7988, - "updatedAt": "2026-09-21T08:10:26.695Z", + "tokens": 18890, + "hash": "222d4fed39c1fccc", + "size": 70834, + "mtimeMs": 1789989575780.3364, + "updatedAt": "2026-09-21T11:19:35.816Z", "source": "hook" }, "src/scheduler/src/sim_verify.c": { @@ -12234,6 +12234,24 @@ "mtimeMs": 1789983096595.9946, "updatedAt": "2026-09-21T09:31:36.645Z", "source": "hook" + }, + "src/scheduler/src/core.c": { + "description": "include ", + "tokens": 4495, + "hash": "428af28e7a5fd8ae", + "size": 16853, + "mtimeMs": 1789987377556.307, + "updatedAt": "2026-09-21T10:42:57.599Z", + "source": "hook" + }, + "src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c": { + "description": "", + "tokens": 9980, + "hash": "4bede92fa2d3e1a7", + "size": 37423, + "mtimeMs": 1789989591378.6973, + "updatedAt": "2026-09-21T11:19:51.419Z", + "source": "hook" } } } \ No newline at end of file diff --git a/.wolf/anatomy.md b/.wolf/anatomy.md index cd74999d..451c08d4 100644 --- a/.wolf/anatomy.md +++ b/.wolf/anatomy.md @@ -1,7 +1,7 @@ # anatomy.md -> Auto-maintained by OpenWolf. Last scanned: 2026-09-21T09:31:36.646Z -> Files: 668 tracked | Anatomy hits: 0 | Misses: 0 +> Auto-maintained by OpenWolf. Last scanned: 2026-09-21T11:41:46.809Z +> Files: 670 tracked | Anatomy hits: 0 | Misses: 0 ## ./ @@ -541,7 +541,7 @@ ## .kiro/specs/scheduler-sim-eager-reconciliation/ -- `design.md` — Design Document (~7078 tok) +- `design.md` — Design Document (~7534 tok) - `requirements.md` — Requirements Document (~3361 tok) - `research.md` — Research & Design Decisions (~5166 tok) - `spec.json` (~135 tok) @@ -957,10 +957,10 @@ ## src/scheduler/include/ -- `core.h` — if !defined(_CORE_H_) (~11223 tok) +- `core.h` — if !defined(_CORE_H_) (~11793 tok) - `plugin.h` — if !defined(_PLUGIN_H_) (~5194 tok) - `scheduler.h` — if !defined(_SCHEDULER_H_) (~6140 tok) -- `sim.h` — if !defined(_SIM_H_) (~4213 tok) +- `sim.h` — if !defined(_SIM_H_) (~4579 tok) ## src/scheduler/include/sched/ @@ -972,6 +972,7 @@ - `alloc_cache.c` — Declares xce_cache_key_t (~3176 tok) - `core_clock.c` — include (~3441 tok) - `core_events.c` — include (~16984 tok) +- `core.c` — include (~4495 tok) - `driver.c` — include (~11968 tok) - `handler.c` — include (~5162 tok) - `host_iter.c` — Declares entry (~1718 tok) @@ -983,12 +984,12 @@ - `repl_internal.h` — if !defined(_REPL_INTERNAL_H_) (~7304 tok) - `repl.c` — include (~7114 tok) - `resource.c` — include (~9725 tok) -- `sched_election.c` — Declares Core (~2964 tok) +- `sched_election.c` — Declares Core (~3249 tok) - `schedobj.c` — include (~5328 tok) - `scheduler.c` — include (~17581 tok) - `sim_alloc.c` — include (~2585 tok) - `sim_limit.c` — include (~2102 tok) -- `sim_loop.c` — include (~22511 tok) +- `sim_loop.c` — include (~23299 tok) - `sim_verify.c` — include (~4694 tok) - `sim.c` — include (~7432 tok) @@ -1028,16 +1029,17 @@ - `test_seqjob_rewrap_equivalence.c` — Declares char (~6448 tok) - `test_sim_alloc_limit_fanout.c` — Declares char (~4716 tok) - `test_sim_contention_limit_driver_parity.c` — Declares char (~6514 tok) -- `test_sim_contention_parity.c` — Declares calls (~21740 tok) +- `test_sim_contention_parity.c` — Declares calls (~22061 tok) - `test_sim_contention_resource_driver_parity.c` — Declares char (~7336 tok) - `test_sim_e2e_limit_visibility.c` — Declares char (~8076 tok) -- `test_sim_e2e_original_motivation.c` (~18078 tok) +- `test_sim_e2e_original_motivation.c` (~19514 tok) - `test_sim_equivalence.c` — Declares BOTH (~11161 tok) - `test_sim_live_allocation.c` — Declares already (~9156 tok) -- `test_sim_reconcile_unaccounted.c` — Declares documents (~34635 tok) +- `test_sim_reconcile_unaccounted.c` — Declares documents (~36877 tok) - `test_sim_resource_contention.c` — Declares is (~10149 tok) +- `test_sim_round_boundary_reconciliation.c` (~9980 tok) - `test_sim_session_ordering.c` (~10531 tok) -- `test_sim_verify.c` — Declares mismatch (~18569 tok) +- `test_sim_verify.c` — Declares mismatch (~18890 tok) - `test_snapshot_save_load.c` (~29478 tok) - `test.c` — define UNIT_TEST 1 (~7044 tok) diff --git a/.wolf/buglog.json b/.wolf/buglog.json index e0bf3558..3ca4402d 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -24816,7 +24816,7 @@ "error_message": "Found during /xce-audit-invariants scheduler (dimension 3, jbuf capacity-1 rendezvous audit), not user-reported and not a live symptom: sim_reconcile_unaccounted_jobs() (sim_loop.c) calls picker->picker_iface->on_dispatch(picker, job->jobid) directly from the sim driver thread (sched_sim_driver_run()'s own thread, between xce_core_begin_session() and xce_core_signal_go()), but plugin.h documents on_dispatch()/on_pend() as 'Post-allocation hooks (MUST) -- allocator thread, after each allocation attempt' -- a MUST-level threading contract this call site violates by construction (driver-thread code calling an allocator-thread-only hook).", "file": "src/scheduler/src/sim_loop.c", "root_cause": "bug-1759/bug-1779's own fix added this on_dispatch() call to sim_reconcile_unaccounted_jobs() specifically to prevent sim_picker_thread_fn() from re-offering (and re-PENDing) an already-reconciled job on a later round -- a real, necessary fix for that narrower symptom (a duplicate PEND report) -- but the fix's own header comment justifies it only in those terms ('the picker never re-decides an already-resolved job'), never addressing thread-safety of calling an allocator-thread-only hook from the driver thread instead. It currently causes no observable corruption only because the sole real (non-no-op) picker plugin, builtin_fcfs.c, happens to guard its entire candidate queue with an internal lock (priv->lock, taken in every hook including end_session()) -- an implementation detail plugin.h's own documented contract does not require picker plugins to provide (\"the picker thread holds no lock the allocator thread needs\"). A future or alternate picker plugin implemented per that documented (lock-free) contract would race sim_reconcile_unaccounted_jobs()'s driver-thread on_dispatch() call against the picker thread's own end_session()/next_job() tail-end activity from the prior round, unsynchronized.", - "fix": "NOT FIXED -- audit finding only, logged per user request to run the full scheduler-driver-equivalence audit; not addressed in the same pass as bug-1836 to keep that fix's diff properly scoped and reviewable (a real fix here means either moving the on_dispatch() call onto the allocator thread's own loop -- a larger restructuring of the reconciliation mechanism's call boundary -- or documenting/enforcing that every picker plugin must be internally lock-safe against this specific driver-thread call, which would need to be added to plugin.h's own interface contract, not just builtin_fcfs.c's implementation). Currently dormant/inert: builtin_fcfs.c is the only registered non-no-op picker plugin in production, and its internal locking happens to make every call site safe today. Needs its own design decision (which of the two fix directions above) before implementation.", + "fix": "FIXED (2026-09-21, TDD). Chose the fuller of the two directions the audit finding itself proposed: moved the on_dispatch() call onto the real allocator thread instead of documenting an exception to plugin.h's contract. Since the scheduler-sim-live-allocation spec, sim mode's sched_sim_driver_run() (sim_loop.c) spawns the SAME unmodified sched_job_picker()/sched_allocator() thread pair (sched_election.c) normal mode uses, so a real allocator thread now genuinely exists to route through in production. Mechanism: two new dedicated cbufs on xce_sched_core_t (core.h) -- reconcile_req (jobid) and reconcile_ack (rc) -- deliberately SEPARATE from jbuf, not a widened jbuf item type, since jbuf's item size/capacity are a pinned determinism invariant (test_core_jbuf_capacity_pin.c, Req 5 AC 7) that a byte-for-byte source-text-scanning regression test forbids changing. A new reserved sentinel, XCE_SCHED_RECONCILE_WAKE (== UINT64_MAX, core.h; same reserved-value idiom this codebase already uses for SCHED_SIM_START_TS_LATEST, sim.h, and jbuf's own pre-existing 0 end-of-session marker), is what the driver pushes onto jbuf itself to wake the allocator thread out of its cbuf_get(&core->jbuf) block -- never a real jobid, so it can never be mistaken for one by sched_get_job(). sim_reconcile_unaccounted_jobs()'s on_dispatch call site (sim_loop.c) now goes through a new sim_reconcile_invoke_on_dispatch() helper: pushes jobid onto reconcile_req, THEN pushes XCE_SCHED_RECONCILE_WAKE onto jbuf (ordering matters -- cbuf_put()'s own mutex gives happens-before, so reconcile_req is guaranteed populated by the time the allocator's cbuf_get(&core->jbuf) returns the sentinel), then blocks on reconcile_ack for the result -- one request in flight at a time, matching jbuf's own capacity-1 rendezvous discipline without touching jbuf itself. sched_election.c's sched_allocator() gained a matching branch: on XCE_SCHED_RECONCILE_WAKE, drain reconcile_req, call on_dispatch() (no plugin_job_election()/sched_make_allocation()/decision -- reconciliation already did its own accounting/charging on the driver thread), ack via reconcile_ack, continue -- a true no-op for normal mode, which never populates reconcile_req. A new xce_sched_sim_driver_t.allocator_thread_live bool (sim.h) tells sim_reconcile_invoke_on_dispatch() whether to route through the relay at all: sched_sim_driver_run() sets it true immediately after successfully spawning the allocator thread (same thread, program order -- no atomics needed, no startup race, since both the write and every later read happen on the driver thread only); it stays false for a bare xce_sched_sim_driver_t that never spawned any thread pair at all (test_sim_reconcile_unaccounted.c's own documented Testing Strategy: ~23 existing tests call sim_reconcile_unaccounted_jobs() directly, standalone, with no picker/allocator threads running), in which case a direct on_dispatch() call is safe by construction -- there is provably only one thread touching picker state, so plugin.h's allocator-thread-only contract has nothing to violate. This let all ~23 pre-existing direct-call tests in that file pass unchanged. Blast radius (grepped the full src+testsuite tree for core->jbuf and every hand-rolled white-box sched_allocator()/sched_job_picker() stub before assuming any of this was safe, per this project's own established discipline): 4 other test files drive sim_reconcile_unaccounted_jobs()'s success path through the REAL sched_sim_driver_run() round loop with their own local sched_allocator() stub (a simplified PEND-only stand-in, never the real sched_election.c symbol) -- test_sim_verify.c, test_sim_round_boundary_reconciliation.c, test_sim_contention_parity.c, test_sim_e2e_original_motivation.c. Confirmed empirically, not just by static grep: a first full-suite build+test run with the fix applied but before patching these four hung all four at meson's 30s per-test timeout (SIGTERM) -- exactly the deadlock a stub that never answers reconcile_ack would cause. Added the identical relay branch to each of their local sched_allocator() stubs; re-ran the full suite and all four passed. TDD: new test_reconcile_invokes_on_dispatch_on_allocator_thread_not_caller (test_sim_reconcile_unaccounted.c) uses a minimal test-owned picker plugin (not builtin_fcfs, whose own internal locking was exactly what made the pre-fix violation dormant/unobservable) whose on_dispatch() records pthread_self(), plus a one-shot relay_one_reconcile_request() thread standing in for sched_allocator(); asserts the recorded tid equals the spawned relay thread's tid and NOT the calling (test) thread's own tid. RED confirmed by temporarily forcing sim_reconcile_invoke_on_dispatch() back to an unconditional direct call and rebuilding: exactly this new test hung (the relay thread's pthread_join() never returned, since nothing was ever pushed onto jbuf/reconcile_req) and was killed at meson's 30s timeout, all 84 other tests still passed. GREEN confirmed after restoring the real fix: full scheduler suite 85/85 Ok, zero regressions, matching the exact pre-fix baseline count.", "tags": [ "scheduler", "sim", @@ -24825,11 +24825,12 @@ "threading-contract", "jbuf", "xce-audit-invariants", - "dormant", - "not-fixed", - "needs-design-decision", "related-to-bug-1759", - "related-to-bug-1779" + "related-to-bug-1779", + "fixed", + "tdd", + "allocator-thread-relay", + "user-directed" ], "related_bugs": [ "bug-1759", diff --git a/.wolf/memory.md b/.wolf/memory.md index 7a4ac186..7134da06 100644 --- a/.wolf/memory.md +++ b/.wolf/memory.md @@ -5600,3 +5600,29 @@ | 02:31 | Edited src/scheduler/testsuite/test_sim_live_allocation.c | expanded (+22 lines) | ~556 | | 02:31 | Edited src/scheduler/testsuite/test_sim_live_allocation.c | 4→3 lines | ~28 | | 02:31 | Edited src/scheduler/src/sim_verify.c | removed 13 lines | ~17 | +| 02:57 | Session end: 71 writes across 14 files (controller.capnp, typemap.h, test_e2e_capnp.c, core.h, scheduler.h) | 51 reads | ~91046 tok | + +## Session: 2026-09-21 02:57 + +| Time | Action | File(s) | Outcome | ~Tokens | +|------|--------|---------|---------|--------| +| 03:41 | Edited src/scheduler/include/core.h | modified relay() | ~448 | +| 03:42 | Edited src/scheduler/include/core.h | expanded (+10 lines) | ~182 | +| 03:42 | Edited src/scheduler/src/core.c | expanded (+11 lines) | ~197 | +| 03:42 | Edited src/scheduler/src/core.c | 3→5 lines | ~46 | +| 03:43 | Edited src/scheduler/src/sched_election.c | added 1 condition(s) | ~357 | +| 03:43 | Edited src/scheduler/include/sim.h | modified struct() | ~486 | +| 03:44 | Edited src/scheduler/src/sim_loop.c | added 1 condition(s) | ~708 | +| 03:44 | Edited src/scheduler/src/sim_loop.c | on_dispatch() → sim_reconcile_invoke_on_dispatch() | ~33 | +| 03:45 | Edited src/scheduler/src/sim_loop.c | added 1 condition(s) | ~188 | +| 03:45 | Edited src/scheduler/src/sim_loop.c | modified 16() | ~125 | +| 03:49 | Edited src/scheduler/testsuite/test_sim_reconcile_unaccounted.c | 7→8 lines | ~43 | +| 03:50 | Edited src/scheduler/testsuite/test_sim_reconcile_unaccounted.c | added 1 condition(s) | ~2281 | +| 03:50 | Edited src/scheduler/testsuite/test_sim_reconcile_unaccounted.c | 2→4 lines | ~65 | +| 04:19 | Edited src/scheduler/testsuite/test_sim_verify.c | added 2 condition(s) | ~381 | +| 04:19 | Edited src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c | added 2 condition(s) | ~381 | +| 04:20 | Edited src/scheduler/testsuite/test_sim_contention_parity.c | added 2 condition(s) | ~381 | +| 04:20 | Edited src/scheduler/testsuite/test_sim_e2e_original_motivation.c | added 2 condition(s) | ~381 | +| 04:26 | Edited src/scheduler/src/sim_loop.c | added 1 condition(s) | ~128 | +| 04:33 | Edited src/scheduler/src/sim_loop.c | modified sim_reconcile_invoke_on_dispatch() | ~90 | +| 04:41 | Edited .kiro/specs/scheduler-sim-eager-reconciliation/design.md | modified Correction() | ~557 | diff --git a/src/scheduler/include/core.h b/src/scheduler/include/core.h index 6054d4d7..75477c29 100644 --- a/src/scheduler/include/core.h +++ b/src/scheduler/include/core.h @@ -165,6 +165,16 @@ typedef void (*xce_unsatisfiable_populate_cb_t)(xce_sched_core_t* core, xce_sched_obj_t* obj, int reason_category); +/* Reserved jbuf sentinel for the Recorded-Decision Reconciliation + * allocator-thread relay (bug-1837) -- see xce_sched_core_s's reconcile_req/ + * reconcile_ack fields' own doc comment below for the full protocol. Never a + * real jobid (job ids are assigned by the controller's monotonic id + * generator, starting near 1, nowhere near UINT64_MAX) and distinct from + * jbuf's own pre-existing 0 "end of session" sentinel -- same reserved-value + * idiom this codebase already uses elsewhere for an analogous "not a real + * value" marker (see SCHED_SIM_START_TS_LATEST, sim.h). */ +#define XCE_SCHED_RECONCILE_WAKE ((uint64_t)-1) + struct xce_sched_core_s { mylock_t lock; int stop; @@ -198,6 +208,26 @@ struct xce_sched_core_s { pthread_t allocator; cbuf_t wbuf; /* action to trigger scheduling */ cbuf_t jbuf; /* job picked for allocation */ + /* Recorded-Decision Reconciliation allocator-thread relay (bug-1837): + * plugin.h documents on_dispatch()/on_pend() as allocator-thread-only + * hooks, but sim_loop.c's sim_reconcile_unaccounted_jobs() (the driver + * thread) needs to invoke on_dispatch() for a job it reconstructs from + * replay history -- there is no fresh allocation to run through the + * normal jbuf/next_job() pipeline for it (the job is already RUN per + * history, not a live candidate), so it cannot flow through jbuf's + * existing real-jobid protocol. These two dedicated cbufs let the driver + * hand the call off to the real sched_allocator() thread (sched_ + * election.c) instead of calling it directly: reconcile_req carries the + * jobid to dispatch, reconcile_ack echoes on_dispatch()'s return code + * once applied. Deliberately SEPARATE from jbuf, not a widened jbuf item + * type -- jbuf's item size/capacity are a pinned determinism invariant + * (test_core_jbuf_capacity_pin.c, Req 5 AC 7) that must never change. + * See XCE_SCHED_RECONCILE_WAKE below and sched_allocator()'s matching + * branch (sched_election.c) for the full handoff protocol. Normal mode + * never populates reconcile_req (only sim_loop.c's reconciliation does), + * so this is a true no-op for normal mode's allocator thread. */ + cbuf_t reconcile_req; /* jobid for on_dispatch, allocator-thread relay */ + cbuf_t reconcile_ack; /* on_dispatch()'s rc, echoed back once applied */ cbuf_t decisions; /* scheduler decisions */ /* Req 5 AC 6 (scheduler-core spec, task 4.4): driver-registered * decision callback + opaque driver context, set via diff --git a/src/scheduler/include/sim.h b/src/scheduler/include/sim.h index 77f21603..0887e1bb 100644 --- a/src/scheduler/include/sim.h +++ b/src/scheduler/include/sim.h @@ -93,6 +93,25 @@ typedef struct { bool interactive; uint64_t snapshot_ts; xce_sched_repl_state_t* repl_state; + /* bug-1837 fix: set true by sched_sim_driver_run() (sim_loop.c), + * immediately after it successfully spawns the real sched_allocator() + * thread (sched_election.c) -- both the write here and every read in + * sim_reconcile_unaccounted_jobs() happen on the SAME thread (the one + * running sched_sim_driver_run()'s own round loop), in program order, so + * this is plain single-threaded state, not a cross-thread flag; no + * atomics needed. Tells sim_reconcile_invoke_on_dispatch() (sim_loop.c) + * whether a real allocator thread exists to relay on_dispatch() through + * (core->reconcile_req/reconcile_ack, core.h) or whether this xce_sched_ + * sim_driver_t was only ever driven directly (no thread pair spawned at + * all -- e.g. test_sim_reconcile_unaccounted.c's own "call sim_ + * reconcile_unaccounted_jobs() standalone" Testing Strategy), in which + * case calling on_dispatch() directly is safe by construction: there is + * only ever one thread touching picker state in that case, so plugin.h's + * allocator-thread-only contract has nothing to violate. Stays false + * forever for a bare xce_sched_sim_driver_t built via fixture_init()- + * style direct core init, matching sched_sim_driver_start()'s own + * memset(sim, 0, sizeof(*sim)) default. */ + bool allocator_thread_live; } xce_sched_sim_driver_t; /* diff --git a/src/scheduler/src/core.c b/src/scheduler/src/core.c index 82d3596f..10865bf0 100644 --- a/src/scheduler/src/core.c +++ b/src/scheduler/src/core.c @@ -162,6 +162,17 @@ int xce_sched_core_init(xce_sched_core_t* core) * indirection for this value without first re-deriving the * determinism argument in AC 7. */ cbuf_init(&(core->jbuf), 1, sizeof(uint64_t)); + + /* bug-1837 fix: Recorded-Decision Reconciliation's allocator-thread + * relay -- see core.h's own doc comment on these two fields for the full + * protocol. Capacity 1 each, matching jbuf's own one-request-in-flight + * discipline (this is not the jbuf determinism invariant itself, so not + * subject to test_core_jbuf_capacity_pin.c's pin -- just a sized-for- + * purpose choice, since only one reconciliation request is ever + * outstanding at a time by construction). */ + cbuf_init(&(core->reconcile_req), 1, sizeof(uint64_t)); + cbuf_init(&(core->reconcile_ack), 1, sizeof(int)); + cbuf_init(&(core->decisions), ndecisions, sizeof(xce_decision_t*)); core->resmgr = makeInstance(&xkvmgr_type, &resmgr_params); @@ -196,6 +207,8 @@ void xce_sched_core_fini(xce_sched_core_t* core) } cbuf_fini(&(core->decisions)); + cbuf_fini(&(core->reconcile_ack)); + cbuf_fini(&(core->reconcile_req)); cbuf_fini(&(core->jbuf)); cbuf_fini(&(core->wbuf)); diff --git a/src/scheduler/src/sched_election.c b/src/scheduler/src/sched_election.c index c1486d8e..98d40767 100644 --- a/src/scheduler/src/sched_election.c +++ b/src/scheduler/src/sched_election.c @@ -202,6 +202,27 @@ void* sched_allocator(void* arg) continue; } + if (jobid == XCE_SCHED_RECONCILE_WAKE) { + /* bug-1837: Recorded-Decision Reconciliation relay + * (sim_reconcile_unaccounted_jobs(), sim_loop.c) -- see core.h's + * reconcile_req/reconcile_ack doc comment for the full + * handshake. Not a real job: no plugin_job_election()/ + * sched_make_allocation() call, no decision built, nothing + * pushed to core->decisions -- purely a threading-contract + * relay so on_dispatch() genuinely runs here, on the allocator + * thread, exactly as plugin.h's Post-allocation hooks contract + * requires, instead of being called directly by the driver + * thread. */ + uint64_t reconcile_jobid = 0; + int reconcile_rc; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + reconcile_rc = job_picker->picker_iface->on_dispatch( + job_picker, reconcile_jobid); + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + continue; + } + job = sched_get_job(core, jobid); if (job == NULL) { fmt_jobid(jobidString, sizeof(jobidString), jobid); diff --git a/src/scheduler/src/sim_loop.c b/src/scheduler/src/sim_loop.c index e5a2da94..15521aa2 100644 --- a/src/scheduler/src/sim_loop.c +++ b/src/scheduler/src/sim_loop.c @@ -764,6 +764,56 @@ static void sim_reconcile_toggle_prior_charges(xce_sched_core_t* core, iter_destroy(&it); } +/* + * sim_reconcile_invoke_on_dispatch - bug-1837 fix: invoke picker-> + * picker_iface->on_dispatch() on behalf of Recorded-Decision Reconciliation + * (sim_reconcile_unaccounted_jobs(), below), routed through the real + * allocator thread when one is live, per plugin.h's documented Post- + * allocation hooks contract ("allocator thread, after each allocation + * attempt"). Direct-call fallback when `sim->allocator_thread_live` is + * false: no thread pair was ever spawned against this xce_sched_sim_driver_t + * (e.g. a standalone unit test that calls sim_reconcile_unaccounted_jobs() + * directly, never sched_sim_driver_run() -- test_sim_reconcile_unaccounted.c's + * own documented Testing Strategy), so there is provably only one thread in + * the whole process touching picker state and nothing to race. + * + * Protocol when a real allocator thread IS live (core.h's reconcile_req/ + * reconcile_ack doc comment has the full rationale): push `jobid` onto + * core->reconcile_req, THEN push the reserved XCE_SCHED_RECONCILE_WAKE + * sentinel onto core->jbuf (never a real jobid, never jbuf's own 0 + * end-of-session marker) to wake sched_allocator() (sched_election.c) out of + * its cbuf_get(&core->jbuf) block. Ordering matters: cbuf_put()'s own + * mutex-protected write to reconcile_req happens-before the jbuf push that + * follows it (same thread, program order), and cbuf_get()'s own + * mutex-protected read on the allocator thread happens-after the matching + * cbuf_put() (POSIX mutex unlock->lock ordering) -- so by the time the + * allocator thread's cbuf_get(&core->jbuf) returns the wake sentinel, + * reconcile_req is guaranteed to already hold this call's jobid. Blocks on + * core->reconcile_ack until sched_allocator() echoes on_dispatch()'s return + * code back -- exactly one request in flight at a time, matching jbuf's own + * capacity-1 rendezvous discipline (never widened; a completely separate + * pair of cbufs carries this protocol instead). + */ +static int sim_reconcile_invoke_on_dispatch(xce_sched_sim_driver_t* sim, + xce_sched_plugin_t* picker, + uint64_t jobid) +{ + if (!sim->allocator_thread_live) { + return picker->picker_iface->on_dispatch(picker, jobid); + } + + cbuf_put(&(sim->core.reconcile_req), &jobid); + { + uint64_t wake = XCE_SCHED_RECONCILE_WAKE; + int rc = 0; + + cbuf_put(&(sim->core.jbuf), &wake); + cbuf_get(&(sim->core.reconcile_ack), &rc); + + return rc; + } +} + /* * sim_reconcile_unaccounted_jobs - Recorded-Decision Reconciliation * (scheduler-sim-decision-parity spec, design.md's "Recorded-Decision @@ -1163,7 +1213,7 @@ static void sim_reconcile_unaccounted_jobs(xce_sched_sim_driver_t* sim, * confirmed dispatched, so a later round's sim_picker_thread_fn() * never re-offers (and re-PENDs) it. */ if (picker != NULL) { - picker->picker_iface->on_dispatch(picker, job->jobid); + sim_reconcile_invoke_on_dispatch(sim, picker, job->jobid); } found = 1; @@ -1349,6 +1399,13 @@ int sched_sim_driver_run(xce_sched_sim_driver_t* sim, return -rc; } + /* bug-1837 fix: recorded exactly once, right here, before the round loop + * below can possibly run -- see xce_sched_sim_driver_t's own doc comment + * (sim.h) on this field for why this is plain single-threaded state + * (same thread sets it and later reads it, in program order) rather than + * a cross-thread flag needing atomics. */ + sim->allocator_thread_live = true; + /* Req 5 AC 16 (task 11.5, "What-if branching"): once --sim-alt-events * is set, the ONGOING event/session stream comes from * sim->alt_events_file instead of sim->replay_file -- the initial diff --git a/src/scheduler/testsuite/test_sim_contention_parity.c b/src/scheduler/testsuite/test_sim_contention_parity.c index 64d594d2..4b002fb1 100644 --- a/src/scheduler/testsuite/test_sim_contention_parity.c +++ b/src/scheduler/testsuite/test_sim_contention_parity.c @@ -531,6 +531,30 @@ void* sched_allocator(void* arg) continue; } + if (jobid == XCE_SCHED_RECONCILE_WAKE) { + /* bug-1837: this file's stand-in sched_allocator() must service + * the same Recorded-Decision Reconciliation relay protocol the + * real sched_election.c implementation does (see that file's + * matching branch for the full rationale) -- this file's own + * sched_sim_driver_run() calls set sim->allocator_thread_live, + * so sim_reconcile_unaccounted_jobs() routes on_dispatch() + * through jbuf/reconcile_req/reconcile_ack exactly as + * production does; without this branch, a reconciliation + * success here would hang forever waiting on an ack this stub + * never sends. */ + uint64_t reconcile_jobid = 0; + int reconcile_rc = 0; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + (void)sched_resolve_job_picker(core, &picker); + if (picker != NULL) { + reconcile_rc = picker->picker_iface->on_dispatch( + picker, reconcile_jobid); + } + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + continue; + } + job = sched_get_job(core, jobid); if (job == NULL) { continue; diff --git a/src/scheduler/testsuite/test_sim_e2e_original_motivation.c b/src/scheduler/testsuite/test_sim_e2e_original_motivation.c index a65eef7e..24bc8d42 100644 --- a/src/scheduler/testsuite/test_sim_e2e_original_motivation.c +++ b/src/scheduler/testsuite/test_sim_e2e_original_motivation.c @@ -483,6 +483,30 @@ void* sched_allocator(void* arg) continue; } + if (jobid == XCE_SCHED_RECONCILE_WAKE) { + /* bug-1837: this file's stand-in sched_allocator() must service + * the same Recorded-Decision Reconciliation relay protocol the + * real sched_election.c implementation does (see that file's + * matching branch for the full rationale) -- this file's own + * sched_sim_driver_run() calls set sim->allocator_thread_live, + * so sim_reconcile_unaccounted_jobs() routes on_dispatch() + * through jbuf/reconcile_req/reconcile_ack exactly as + * production does; without this branch, a reconciliation + * success here would hang forever waiting on an ack this stub + * never sends. */ + uint64_t reconcile_jobid = 0; + int reconcile_rc = 0; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + (void)sched_resolve_job_picker(core, &picker); + if (picker != NULL) { + reconcile_rc = picker->picker_iface->on_dispatch( + picker, reconcile_jobid); + } + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + continue; + } + job = sched_get_job(core, jobid); if (job == NULL) { continue; diff --git a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c index d474cd8c..eb6b3a53 100644 --- a/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c +++ b/src/scheduler/testsuite/test_sim_reconcile_unaccounted.c @@ -90,6 +90,7 @@ */ #include #include +#include #include #include #include @@ -2524,6 +2525,192 @@ test_reconcile_removes_reconciled_job_from_picker_candidates(void) "PASS\n"); } +/* ---------------------------------------------------------------------- */ +/* Test (bug-1837, TDD): plugin.h documents on_dispatch()/on_pend() as */ +/* "Post-allocation hooks (MUST) -- allocator thread, after each allocation */ +/* attempt". Recorded-Decision Reconciliation used to call on_dispatch() */ +/* directly from ITS OWN caller thread (sim_reconcile_unaccounted_jobs()'s */ +/* thread -- normally sched_sim_driver_run()'s driver thread in production), */ +/* violating that contract by construction. Dormant in production only */ +/* because builtin_fcfs.c happens to guard every hook with its own internal */ +/* lock -- a defensive measure plugin.h's contract text does not actually */ +/* require of picker plugins in general ("the picker thread holds no lock */ +/* the allocator thread needs"). */ +/* */ +/* Fix: once a real allocator thread is live for a core (xce_sched_sim_ */ +/* driver_t.allocator_thread_live, sim.h), sim_reconcile_invoke_on_dispatch()*/ +/* (sim_loop.c) routes the call through it via core->reconcile_req/ */ +/* core->jbuf's XCE_SCHED_RECONCILE_WAKE sentinel/core->reconcile_ack */ +/* (core.h), and sched_election.c's sched_allocator() is what actually */ +/* invokes on_dispatch() on that thread. */ +/* */ +/* This test proves it directly, independent of builtin_fcfs's own internal */ +/* locking: a minimal test-owned picker plugin records pthread_self() from */ +/* inside its on_dispatch() implementation, and a one-shot thread standing */ +/* in for sched_allocator() (relay_one_reconcile_request(), below) services */ +/* the relay protocol. RED before the bug-1837 fix (on_dispatch() ran on */ +/* this test's OWN thread -- g_dispatch_calling_tid would equal */ +/* pthread_self(), and g_seen_jbuf_wake_value would never be observed since */ +/* nothing was ever pushed onto jbuf at all), GREEN after. */ +/* ---------------------------------------------------------------------- */ + +static pthread_t g_dispatch_calling_tid; +static int g_dispatch_call_count; +static uint64_t g_seen_jbuf_wake_value; + +static int thread_recording_on_dispatch(void* picker, uint64_t jobid) +{ + (void)picker; + (void)jobid; + g_dispatch_calling_tid = pthread_self(); + g_dispatch_call_count++; + return 0; +} + +static int noop_picker_session_hook(void* picker) +{ + (void)picker; + return 0; +} + +static xce_sched_job_t* noop_picker_next_job_hook(void* picker) +{ + (void)picker; + return NULL; +} + +static int noop_picker_jobid_hook(void* picker, uint64_t jobid) +{ + (void)picker; + (void)jobid; + return 0; +} + +static xce_job_picker_iface_t g_thread_recording_picker_iface = { + .init = NULL, + .fini = NULL, + .begin_session = noop_picker_session_hook, + .next_job = noop_picker_next_job_hook, + .end_session = noop_picker_session_hook, + .on_dispatch = thread_recording_on_dispatch, + .on_pend = noop_picker_jobid_hook, + .on_queue_update = NULL, + .on_job_push = NULL, + .on_job_state_change = NULL, + .on_job_update = NULL, + .on_config = NULL, + .on_job_rusage = NULL, +}; + +static xce_sched_plugin_t g_thread_recording_plugin + = { .name = (char*)"test_reconcile_thread_picker", + .picker_iface = &g_thread_recording_picker_iface }; + +/* One-shot stand-in for sched_election.c's real sched_allocator() -- + * services exactly one Recorded-Decision Reconciliation relay request + * (core->jbuf's XCE_SCHED_RECONCILE_WAKE sentinel, then core->reconcile_req, + * then acks via core->reconcile_ack) and returns, mirroring that function's + * own SIM_RECONCILE_JBUF_WAKE branch exactly (see its own comment, + * sched_election.c, for the full protocol this mirrors). Deliberately not a + * forever loop: this test only ever sends one request. No TEST_ASSERT_* + * calls in here -- Unity's assertion macros longjmp against the MAIN test + * thread's context, unsafe to invoke from a spawned thread; this function + * only records observations into statics, asserted on after pthread_join() + * back on the main test thread. */ +static void* relay_one_reconcile_request(void* arg) +{ + xce_sched_core_t* core = (xce_sched_core_t*)arg; + uint64_t wake_val = 0; + uint64_t reconcile_jobid = 0; + xce_sched_plugin_t* job_picker = NULL; + int reconcile_rc = 0; + + cbuf_get(&(core->jbuf), &wake_val); + g_seen_jbuf_wake_value = wake_val; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + (void)sched_resolve_job_picker(core, &job_picker); + if (job_picker != NULL) { + reconcile_rc = job_picker->picker_iface->on_dispatch(job_picker, + reconcile_jobid); + } + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + + return NULL; +} + +static void +test_reconcile_invokes_on_dispatch_on_allocator_thread_not_caller(void) +{ + xce_sched_sim_driver_t sim; + xce_sched_job_t* job; + xce_job_alloc_t* jalloc; + replay_session_t* decoded; + int residx; + pthread_t relay_tid; + int prc; + + fixture_init(&sim); + + g_dispatch_calling_tid = pthread_self(); /* sentinel: only meaningful if + * overwritten below */ + g_dispatch_call_count = 0; + g_seen_jbuf_wake_value = 0; + + TEST_ASSERT_EQUAL_INT( + 0, plugin_register_core(&(sim.core), &g_thread_recording_plugin)); + sim.core.job_pick_plugin = g_thread_recording_plugin.name; + + residx = register_slots_resource(&(sim.core)); + add_worker(&(sim.core), "worker1", residx, 1000.0f); + + job = add_job(&(sim.core), 7200ULL, JOB_STAT_RUN, NULL); + TEST_ASSERT_NULL(job->accounts); + + jalloc = make_test_job_alloc_with_host(7200ULL, "worker1", "slots", 2.0f); + decoded = make_decoded_session(1000ULL, make_dispatch_entry_from_alloc(jalloc)); + + /* bug-1837 fix: mark a real allocator thread as live for this core, and + * actually spawn its stand-in BEFORE triggering reconciliation -- cbuf's + * own mutex+condvar makes the exact spawn/relay-vs-post ordering + * irrelevant (see relay_one_reconcile_request()'s own comment). */ + sim.allocator_thread_live = true; + prc = pthread_create(&relay_tid, NULL, relay_one_reconcile_request, + &(sim.core)); + TEST_ASSERT_EQUAL_INT(0, prc); + + sim_reconcile_unaccounted_jobs(&sim, decoded, NULL); + + pthread_join(relay_tid, NULL); + + TEST_ASSERT_NOT_NULL_MESSAGE( + job->accounts, "fixture sanity: reconciliation must still have found " + "and captured the matching recorded decision"); + TEST_ASSERT_EQUAL_INT_MESSAGE(1, g_dispatch_call_count, + "on_dispatch() must be invoked exactly " + "once"); + TEST_ASSERT_EQUAL_UINT64_MESSAGE( + XCE_SCHED_RECONCILE_WAKE, g_seen_jbuf_wake_value, + "bug-1837: reconciliation must wake the allocator thread via the " + "reserved jbuf sentinel, never by pushing the real jobid onto jbuf " + "directly (that would incorrectly route it through the live " + "allocation chain)"); + TEST_ASSERT_FALSE_MESSAGE( + pthread_equal(g_dispatch_calling_tid, pthread_self()), + "bug-1837: on_dispatch() must run on the allocator thread, never on " + "sim_reconcile_unaccounted_jobs()'s own caller thread, once a real " + "allocator thread is live for this core -- plugin.h's documented " + "Post-allocation hooks contract (\"allocator thread, after each " + "allocation attempt\")"); + TEST_ASSERT_TRUE_MESSAGE( + pthread_equal(g_dispatch_calling_tid, relay_tid), + "on_dispatch() must run specifically on the spawned allocator-relay " + "thread, not some other thread"); + + printf("test_reconcile_invokes_on_dispatch_on_allocator_thread_not_" + "caller: PASS\n"); +} + /* ---------------------------------------------------------------------- */ /* Task 4.2 fixtures (scheduler-sim-live-allocation spec, "Sim-Mode */ /* Reconciliation Narrowing" component, Requirement 3.4/3.5): register a */ @@ -2816,6 +3003,8 @@ int main(void) RUN_TEST( test_reconcile_info_worker_and_summary_hide_completed_job_after_stray_dispatch); RUN_TEST(test_reconcile_removes_reconciled_job_from_picker_candidates); + RUN_TEST( + test_reconcile_invokes_on_dispatch_on_allocator_thread_not_caller); RUN_TEST(test_reconcile_registers_global_limit_usage_for_reconciled_job); RUN_TEST( test_reconcile_does_not_reregister_limit_for_already_decided_job); diff --git a/src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c b/src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c index dfc8c382..d2ae4a1d 100644 --- a/src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c +++ b/src/scheduler/testsuite/test_sim_round_boundary_reconciliation.c @@ -375,6 +375,30 @@ void* sched_allocator(void* arg) continue; } + if (jobid == XCE_SCHED_RECONCILE_WAKE) { + /* bug-1837: this file's stand-in sched_allocator() must service + * the same Recorded-Decision Reconciliation relay protocol the + * real sched_election.c implementation does (see that file's + * matching branch for the full rationale) -- this file's own + * sched_sim_driver_run() calls set sim->allocator_thread_live, + * so sim_reconcile_unaccounted_jobs() routes on_dispatch() + * through jbuf/reconcile_req/reconcile_ack exactly as + * production does; without this branch, a reconciliation + * success here would hang forever waiting on an ack this stub + * never sends. */ + uint64_t reconcile_jobid = 0; + int reconcile_rc = 0; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + (void)sched_resolve_job_picker(core, &picker); + if (picker != NULL) { + reconcile_rc = picker->picker_iface->on_dispatch( + picker, reconcile_jobid); + } + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + continue; + } + job = sched_get_job(core, jobid); if (job == NULL) { continue; diff --git a/src/scheduler/testsuite/test_sim_verify.c b/src/scheduler/testsuite/test_sim_verify.c index 7edf78bb..9b5bad34 100644 --- a/src/scheduler/testsuite/test_sim_verify.c +++ b/src/scheduler/testsuite/test_sim_verify.c @@ -391,6 +391,30 @@ void* sched_allocator(void* arg) continue; } + if (jobid == XCE_SCHED_RECONCILE_WAKE) { + /* bug-1837: this file's stand-in sched_allocator() must service + * the same Recorded-Decision Reconciliation relay protocol the + * real sched_election.c implementation does (see that file's + * matching branch for the full rationale) -- this file's own + * sched_sim_driver_run() calls set sim->allocator_thread_live, + * so sim_reconcile_unaccounted_jobs() routes on_dispatch() + * through jbuf/reconcile_req/reconcile_ack exactly as + * production does; without this branch, a reconciliation + * success here would hang forever waiting on an ack this stub + * never sends. */ + uint64_t reconcile_jobid = 0; + int reconcile_rc = 0; + + cbuf_get(&(core->reconcile_req), &reconcile_jobid); + (void)sched_resolve_job_picker(core, &picker); + if (picker != NULL) { + reconcile_rc = picker->picker_iface->on_dispatch( + picker, reconcile_jobid); + } + cbuf_put(&(core->reconcile_ack), &reconcile_rc); + continue; + } + job = sched_get_job(core, jobid); if (job == NULL) { continue;