feat(scheduler-core): implement REPL execution control (task 12.1)
Adds the interactive simulation REPL's execution-control slice (Req 5 AC 22): a single-threaded input loop (readline-based) supporting step [N], continue, quit, and help, pausing before the first replay event and blocking the driver loop while active. Review round 1 raised two findings, both resolved here: - GNU readline (GPL) was linked directly into xce_scheduler, a shipped production daemon. Switched the whole project to libedit (BSD) for line editing -- scheduler and controller (xce_test) both now use editline/readline.h, API-compatible (readline()/add_history()). - Getting a clean regression run after the libedit swap required a full clean rebuild, which exposed a genuine pre-existing SIGSEGV (bug-1504): SnapshotData.internalState was a capnp Data field, and this codebase's capnpc-c backend silently no-ops Data-field encoding without zero-initializing the caller's stack struct, leaving an uninitialized capn_ptr written onto the wire. Fixed by switching internalState to List(UInt8) (with a new internalStateLen wire field), matching the working FetchFileReply.content/bytesRead precedent -- no hand-written accessor code needed. A second, unrelated instance of the same bug class was found in the controller's WAL-catchup path (WalCatchupEntry.payload) and deliberately left unfixed as bug-1505, out of scheduler-core's boundary; needs its own project-wide Data-field audit. Scheduler suite: 34/34 clean. RED-phase reproduction recorded in tasks.md (repl_should_pause() step-budget check disabled -> 2/4 tests fail as expected -> reverted -> 34/34 green). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPPXeKSUXUqcqvzNXY1rzJ
This commit is contained in:
parent
de00ccff37
commit
02f7f48ff9
SSH key fingerprint:
SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
17 changed files with 1491 additions and 81 deletions
|
|
@ -14667,5 +14667,178 @@
|
|||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-04T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1498",
|
||||
"timestamp": "2026-08-05T00:00:00Z",
|
||||
"error_message": "test_toctou_hardlink_planted_after_validation_rejected FAILED: Expected Non-NULL -- fgets() on the outside 'victim' file returned NULL (file was 0 bytes) after the TOCTOU regression test ran, even though sched_record_start()/the reproduced open-hardening sequence both correctly REJECTED the hard-link swap and returned an error.",
|
||||
"file": "src/scheduler/src/recording.c",
|
||||
"root_cause": "sched_record_start()'s TOCTOU-hardened open used O_WRONLY|O_CREAT|O_TRUNC|O_NOFOLLOW. O_TRUNC truncates the target atomically as PART OF the open(2) syscall itself, before any userspace code -- including the fstat()+st_nlink==1 safety check right after open() -- ever runs. Against a hard-linked victim (same inode as the outside file, no symlink involved so O_NOFOLLOW never triggers), open() succeeds and O_TRUNC immediately zeroes the shared inode's content; the subsequent st_nlink>1 check then correctly rejects and closes the fd, but the outside victim file was already corrupted by that point. The check ran too late to matter -- classic 'check-after-side-effect' TOCTOU-adjacent bug, caught by the test's own outside-file-content assertion (added specifically to prove containment, not just that the return code was negative).",
|
||||
"fix": "Removed O_TRUNC from the initial open() flags (now O_WRONLY|O_CREAT|O_NOFOLLOW). After fstat()+st_nlink==1 passes, explicitly call ftruncate(fd, 0) to get the same truncate-on-legitimate-overwrite behavior, but only once safety is verified. Same fix applied to the test's own toctou_hardened_open() helper (a verbatim reproduction of the production sequence, used because a literal multi-threaded race against sched_record_start()'s few-instruction internal window would be flaky).",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-9.2",
|
||||
"recording.c",
|
||||
"toctou",
|
||||
"hardlink",
|
||||
"O_TRUNC",
|
||||
"check-after-side-effect",
|
||||
"sched_record_start"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-05T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1499",
|
||||
"timestamp": "2026-08-05T00:00:00Z",
|
||||
"error_message": "sched_driver_session_ordering_test TIMEOUT (30s, killed by SIGTERM) after adding task 9.2's recording-write hooks to xce_sched_driver_run_normal_mode_round() -- picker thread, allocator thread, and the main round all blocked forever.",
|
||||
"file": "src/scheduler/src/driver.c",
|
||||
"root_cause": "While inserting the new 'if (sched->recording) { rec_ctx = sched_record_session_begin(...); }' block between plugin_begin_session()/sched_account_jobs() and the decision-drain loop, the pre-existing `xce_core_signal_go(&(sched->core));` call was accidentally dropped from the Edit's old_string/new_string pair -- it was the last line of the block being replaced but was not included in the replacement text. Without xce_core_signal_go(), the picker thread's cbuf_get(&core->wbuf) rendezvous never unblocks, which cascades: the stand-in allocator thread never receives a jobid from cbuf_get(&core->jbuf), so it never pushes the end-of-session NULL decision, so the main thread's own cbuf_get(&core->decisions) drain loop blocks forever -- a 3-way deadlock across all three threads.",
|
||||
"fix": "Restored `xce_core_signal_go(&(sched->core));` immediately after the new recording session-begin block (matching requirements.md's own annotated pseudocode order: build ReplaySession{beginRound} THEN sched_go()). Caught by re-running the full scheduler test suite after the recording hooks were added -- test_driver_session_ordering.c is the ONLY test in the suite that drives a real picker/allocator thread pair through this exact function, so it was the only test capable of catching a signal_go() regression.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-9.2",
|
||||
"driver.c",
|
||||
"deadlock",
|
||||
"xce_core_signal_go",
|
||||
"accidental-deletion-in-edit",
|
||||
"xce_sched_driver_run_normal_mode_round"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-05T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1500",
|
||||
"timestamp": "2026-08-06T00:00:00Z",
|
||||
"error_message": "test_run_processes_multiple_sessions_in_order_and_no_longer_returns_enosys FAIL: Expected 5000 Was 743938186628040 (a raw-looking CLOCK_MONOTONIC nanosecond value, not the fixture's own beginRound).",
|
||||
"file": "src/scheduler/testsuite/test_sim_event_loop.c",
|
||||
"root_cause": "The test's fixture-building helper called the real sched_record_start() THEN sched_record_stop() to produce a valid header+snapshot prefix (matching test_sim_startup.c's own write_valid_fixture() precedent), then re-opened the same path in append mode ('a') to hand-write additional ReplayEvent/ReplaySession messages after it. But sched_record_stop() (recording.c) unconditionally writes its OWN ReplayEvent{endSession} marker immediately after the snapshot as part of every completed recording (Req 5 AC 20) -- so the fixture already contained a premature end-of-stream marker BEFORE any of the test's own appended events/sessions. sched_sim_driver_run()'s event loop (sim_loop.c, task 11.2) correctly treats ReplayEvent{endSession} as the whole-stream termination signal and stopped reading right there, leaving sim->core's clock at its xce_sched_core_init() default (virtual_clock_base=0, speed_num=1, speed_den=1 -- AC 3's normal-mode row), so xce_core_now() returned an unfrozen, elapsed-wall-clock-driven value instead of the intended frozen 5000.",
|
||||
"fix": "Changed the fixture helper to call ONLY sched_record_start() (never sched_record_stop()), and return the STILL-OPEN FILE* (sched.record_file) so the test appends its own hand-built events/sessions directly onto the same open stream, ending with its own append_end_of_stream_marker() call, then fclose()s it directly -- bypassing sched_record_stop()'s real event/session-recording API entirely for this hand-built-fixture use case.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-11.2",
|
||||
"sim_loop.c",
|
||||
"test-fixture",
|
||||
"sched_record_stop",
|
||||
"ReplayEvent_body_endSession",
|
||||
"premature-termination-marker"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-06T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1501",
|
||||
"timestamp": "2026-08-07T00:00:00Z",
|
||||
"error_message": "gcc: 'expected =, ,, ;, asm or __attribute__ before is' / 'unknown type name is' / 'missing terminating character' cascade starting at sim_verify.c:59, plus 'implicit declaration of function sim_verify_str_eq' further down.",
|
||||
"file": "src/scheduler/src/sim_verify.c",
|
||||
"root_cause": "File header doc comment's Comparison-strategy paragraph wrote the literal text 'xce_job_alloc_t*/xce_pendreason_t*' to mean 'an xce_job_alloc_t pointer or an xce_pendreason_t pointer' -- the '*/' substring is C's block-comment CLOSE token, so the /* ... */ doc comment terminated early, right in the middle of prose. Everything from that point until the next real */ (the following function's own closing delimiter) was parsed as C code, producing a cascade of nonsense syntax errors and, once the compiler resynced, an implicit-declaration warning/error for a function whose real prototype/definition had been swallowed into the truncated comment. Exact recurrence of the bug-1493 pattern already logged for typemap.h (same 'accidental */ in a C block comment' root cause).",
|
||||
"fix": "Reworded to 'xce_job_alloc_t* or xce_pendreason_t*' (space before 'or' instead of a bare slash-star-slash-adjacent sequence), eliminating the literal '*/' substring. Ran a repo grep for the same '[A-Za-z_]*/[A-Za-z_]' pattern across both new/modified files (sim_verify.c, sim_loop.c) before rebuilding to rule out a second occurrence.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-11.4",
|
||||
"sim_verify.c",
|
||||
"block-comment",
|
||||
"accidental-comment-close",
|
||||
"bug-1493-recurrence",
|
||||
"build-failure"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1493"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-07T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1502",
|
||||
"timestamp": "2026-08-07T00:00:00Z",
|
||||
"error_message": "Task 11.5 review round 1 REJECTED: previous implementer's self-reported RED-phase evidence (git stash of 4 'behavioral production files' -> 3 tests failing under meson test) did not reproduce -- reviewer got a hard ninja build failure instead (test_sim_event_loop.c:467 calling sim_process_replay_event_message with 5 args against a stashed-back 4-arg sim_loop.c declaration).",
|
||||
"file": "src/scheduler/src/sim_loop.c",
|
||||
"root_cause": "git stash scoped to only 4 production files silently desyncs test files (which already call the NEW post-task-11.5 signatures/behavior) against the stashed-back OLD production code -- a stash-based RED probe is not a valid methodology whenever tests and production code changed together in the same commit-to-be. The previous implementer's reported failure counts were therefore not reproducible as described.",
|
||||
"fix": "Replaced the stash approach with a direct, targeted, in-place temporary edit of ONLY the behavioral logic inside sim_loop.c (never touching test files, sim_record.c, sim.c/sim.h, recording_session.c/recording.h, or either meson.build): hardcoded event_src to sim->replay_file (defeats AC16 what-if branching) and wrapped the 3 --sim-output write gates (header/snapshot, event pass-through, session-write via record_ctx) in 'if (0 && ...)' no-ops (defeats AC19). Rebuilt+ran `meson test -C xce.build --suite scheduler`: real RED = 30/32, sched_sim_alt_events_test and sched_sim_record_output_test FAIL with concrete assertion messages. Reverted all 4 edits using the exact original text; confirmed via `git diff src/scheduler/src/sim_loop.c` that only the original (pre-probe) task-11.5 diff remained, no probe residue (grep for 'TEMP RED-PHASE PROBE' and 'if (0 &&' both zero matches). Rebuilt+reran: 32/32 green again.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-11.5",
|
||||
"sim_loop.c",
|
||||
"red-phase-evidence",
|
||||
"review-rejection",
|
||||
"git-stash-pitfall",
|
||||
"methodology"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-07T00:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1503",
|
||||
"timestamp": "2026-08-07T03:11:00Z",
|
||||
"error_message": "AddressSanitizer: attempting free on address which was not malloc()-ed, inside free_QueueConf() <- free_QueueConf_ptr() <- release_translated_payload() (driver.c) <- xce_sched_driver_translate_pending_events(). Crashing pointer was 56 bytes inside a 61-byte region allocated by makeString()->makeInstance().",
|
||||
"file": "src/scheduler/testsuite/test_sim_equivalence.c",
|
||||
"root_cause": "makeString() (lib/xstring.c) is backed by the vendored rtti library makeInstance() allocator, which does NOT return a plain malloc()-compatible pointer at offset 0 of its allocation (it returns a pointer into a larger rtti-managed block). Elsewhere in the codebase makeString()-allocated strings are safe because they are only ever released by the SAME rtti-aware teardown path that created them (e.g. kill_sched_* killers, or never freed at all in white-box test fixtures). This new test constructed xce_queue_t/xce_job_t fixtures with q->queue/submit->queue/user/project_name/resreq set via makeString(), then drove them through the REAL production cleanup path release_translated_payload() (driver.c), whose free_QueueConf_ptr()/free_Job_ptr() (controller.capnp.c, capnp-generated) call plain free() on those fields directly -- exactly the mismatch: an rtti-backed allocation freed via plain free().",
|
||||
"fix": "Replaced every makeString() call used to populate xce_queue_t.queue / xce_job_t.submit->{queue,user,project_name,resreq} in test_sim_equivalence.c with strdup(), which returns a plain malloc()-compatible buffer safe for free_QueueConf_ptr()/free_Job_ptr() to free directly. Confirmed clean (no ASan bad-free) after the fix, full scheduler suite (33/33) green.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"makeString",
|
||||
"strdup",
|
||||
"asan",
|
||||
"bad-free",
|
||||
"driver.c",
|
||||
"task-11.6"
|
||||
],
|
||||
"related_bugs": [],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-07T03:11:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1504",
|
||||
"timestamp": "2026-08-07T08:00:00Z",
|
||||
"error_message": "sched_sim_startup_test SIGSEGV in write_SnapshotData (lib/controller.capnp.c) -- deterministic crash inside libCapnC_Runtime's read_ptr/capn_setp, discovered during scheduler-core task 12.1 review remediation (full clean rebuild needed for a libedit dependency swap).",
|
||||
"file": "src/schema/controller.capnp",
|
||||
"root_cause": "SnapshotData.internalState was declared `:Data` in the capnp schema. This codebase's capnpc-c backend silently no-ops Data-field encoding inside generated encode_XXX() composite helpers (documented, deliberate limitation -- see controller.capnp's own prior comment citing ReplayHeader.configHash). But encode_SnapshotData_ptr()'s local `struct SnapshotData d;` is never zero-initialized before the (partially no-op) encode_SnapshotData() populates it, so the skipped Data field's capn_ptr is left as UNINITIALIZED STACK GARBAGE. write_SnapshotData() then unconditionally capn_setp()'s that garbage onto the wire. This is genuine UB: whether it crashes depends entirely on what garbage bytes land in that stack slot at each call site -- ReplayHeader.configHash has the identical gap but never crashed (lucky garbage); SnapshotData.internalState's stack layout in this one test's call chain was unlucky. A prior 'rm -rf xce.build' full rebuild (needed to pick up a new nix dependency) perturbed the whole binary's layout enough to flip this from dormant to a deterministic, reproducible SIGSEGV.",
|
||||
"fix": "Changed SnapshotData.internalState from `Data` to `List(UInt8)` in controller.capnp, with a new companion internalStateLen (@11, UInt32) wire field for the maplistcount -- following the exact working precedent of FetchFileReply.content/bytesRead, which this same capnpc-c backend DOES correctly auto-generate real (non-no-op) encode/decode for (capn_new_list8/capn_set8 loop, not a placeholder comment). Updated typemap.h's sched_snapshot_data_t.internal_state_len from size_t to uint32_t to match the wire type, and updated test_e2e_capnp.c's test_snapshot_data_round_trip (which previously worked around the Data no-op with a manual low-level SnapshotData_get/set_internalState accessor) to rely on the now-working ordinary codec bridge instead. No hand-written low-level accessor code needed any more for this field.",
|
||||
"tags": [
|
||||
"scheduler-core",
|
||||
"task-12.1",
|
||||
"capnp-schema",
|
||||
"codegen-limitation",
|
||||
"sigsegv",
|
||||
"uninitialized-memory",
|
||||
"controller.capnp",
|
||||
"snapshot"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1493",
|
||||
"bug-1505"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-07T08:00:00Z"
|
||||
},
|
||||
{
|
||||
"id": "bug-1505",
|
||||
"timestamp": "2026-08-07T08:10:00Z",
|
||||
"error_message": "controller_unit_test SIGSEGV in write_WalCatchupEntry (lib/controller.capnp.c), test_wal_catchup_append_term_fields_round_trip -- discovered incidentally while chasing bug-1504; NOT fixed, logged for its own follow-up.",
|
||||
"file": "src/schema/controller.capnp",
|
||||
"root_cause": "Same root cause as bug-1504 (Data-field codegen no-op leaves an uninitialized stack capn_ptr in the generated encode_XXX_ptr()'s local struct), a DIFFERENT field: WalCatchupEntry.payload (@2: Data), in the controller's HA/WAL-catchup subsystem (bug-847's own territory -- see controller.capnp's comment there), entirely unrelated to scheduler-core. Confirms this is a systemic, project-wide bug class affecting every `:Data` field in controller.capnp whose bytes are populated via the generic encode_XXX()/write_XXX() composite pipeline rather than fully hand-written low-level accessor code -- not a one-off. Known/suspected other Data fields with the same latent gap: ReplayHeader.configHash (dormant so far, no real consumer), CtrlHelloRequest.key, and possibly others not yet audited.",
|
||||
"fix": "NOT FIXED. Deliberately deferred -- out of scheduler-core task 12.1's boundary (this is the controller's HA/WAL-replication subsystem), and a proper fix requires a project-wide audit of every `:Data` field in controller.capnp to decide case-by-case: convert to List(UInt8) (bug-1504's fix, when the field is genuinely a flat byte blob with no cross-segment/per-element concerns) vs. keep Data with real hand-written low-level accessor code at every encode/decode call site (matching the batchPayloads precedent, when List(UInt8) risks bug-847's per-element far-pointer loss for repeated/batch structures). Needs its own scoped task, not a one-off patch.",
|
||||
"tags": [
|
||||
"controller",
|
||||
"ha",
|
||||
"wal-catchup",
|
||||
"capnp-schema",
|
||||
"codegen-limitation",
|
||||
"sigsegv",
|
||||
"uninitialized-memory",
|
||||
"controller.capnp",
|
||||
"deferred",
|
||||
"systemic"
|
||||
],
|
||||
"related_bugs": [
|
||||
"bug-1504",
|
||||
"bug-847"
|
||||
],
|
||||
"occurrences": 1,
|
||||
"last_seen": "2026-08-07T08:10:00Z"
|
||||
}
|
||||
]
|
||||
]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue