myxce/src/scheduler/include/recording.h
Rongsong Shen 9c02184fb4
fix(scheduler): record events before the core applies them (bug-1852)
The recorder encoded each controller event batch after
xce_core_push_events() had applied it, and applying moves payload fields
out of the events: a WORKERS_PUSH's resvals (core_apply_worker_resvals
MOVE_PTR), a JOBS_PUSH's allocs, a JOB_RUSAGE's rusages, and a
CONFIG_PUSH's shared resvals (sched_apply_pending_config recorded after
sched_init_config). Recordings silently lost exactly those fields -- a
worker's resval names were written with NULL values, so --sim-verify
replayed "no slots" and turned 3 live DISPATCHes into PENDs.

driver.c now records the batch before the push; the periodic in-stream
SnapshotEntry moves out of sched_record_write_events() into new
sched_record_checkpoint_after_apply(), called after the push so a
checkpoint still reflects the events written ahead of it. The config
push is likewise recorded before sched_init_config().

Regression test: test_recorded_workers_push_keeps_resval_values (RED:
recorded value NULL; GREEN after). Periodic-checkpoint tests updated to
the write-then-checkpoint contract. meson test 109/109; 25 live soak
recordings replay with 0 mismatches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-24 08:41:55 +00:00

421 lines
24 KiB
C

#if !defined(_RECORDING_H_)
#define _RECORDING_H_ 1
#include <stddef.h>
#include <stdint.h>
#include "scheduler.h"
/*
* recording.h - scheduler-core spec, tasks 9.1/9.2 (Req 5 AC 10, "Record
* file path validation" and "Runtime recording control -- detailed
* mechanism"). Task 9.1 added ONLY sched_validate_record_filename() below,
* the validation function every write must pass through first. Task 9.2
* (this revision) adds the actual record start/stop lifecycle
* (sched_record_start()/sched_record_stop()) plus the recording-write hooks
* driver.c's normal-mode loop calls into -- design.md's NormalDriver
* service interface, adapted to this codebase's single combined
* `xce_scheduler_t` driver-level struct (there is no separate
* `xce_sched_driver_t` -- see scheduler.h's field comment on `recording`).
*
* sched_validate_record_filename - validates a caller-supplied, admin-
* authenticated but UNTRUSTED record file name (controller.capnp's
* `SchedAdminRequest.recordFile` -- "filename (not a path)", per that
* field's own comment) and resolves it to a full path beneath `record_dir`
* BEFORE the caller (task 9.2) ever opens it for writing. `record_dir`
* itself is trusted: per design.md's NormalDriver component, it is
* provisioned EXACTLY ONE WAY, the scheduler's own `--record-dir` startup
* CLI flag (main.c), and is never settable from any request-path field --
* so only `filename` is adversarial input here, never `record_dir`.
*
* Two-phase check, per requirements.md Req 5 AC 10 "Record file path
* validation":
*
* 1. String-level rejection, BEFORE any filesystem call ever touches
* `filename`: reject if `filename` is NULL/empty, contains a path
* separator ('/'), or contains a ".." substring anywhere (leading,
* trailing, embedded, doubled -- e.g. "..", "../x", "x/..", "..x",
* "x..", "x..y", "...."). Since step 1 also bans '/' outright, a bare
* ".." substring check is both necessary and sufficient: with no
* slash permitted at all, `filename` can never have more than one
* path segment, so there is no "segment" for ".." to hide in other
* than the substring itself. C strings terminate at the first NUL
* byte, so none of these checks -- nor the eventual fopen() -- can
* ever observe bytes past an embedded NUL; there is no bypass there.
*
* 2. Canonical-path containment, AFTER the string check passes:
* `record_dir` (which must already exist -- provisioning/creating it
* on disk is explicitly out of this task's scope; see tasks.md 9.1's
* boundary) is resolved to its canonical form via realpath(), then
* the validated bare `filename` is joined onto that canonical form
* (never onto the possibly-non-canonical original `record_dir`
* string). Because `filename` has no '/' and no "..", this join
* cannot itself escape the resolved directory by construction -- the
* one remaining escape vector is a pre-existing filesystem entry
* already named `filename` inside `record_dir` that is itself a
* symlink pointing elsewhere. That case is checked explicitly: the
* symlink's target is independently canonicalized via realpath() and
* tested for containment with a true canonical-path comparison
* (resolved target must equal the resolved directory followed by
* '/') -- NEVER a textual prefix match, which would wrongly accept a
* sibling directory like "/var/log/xce-evil/..." against a
* `record_dir` of "/var/log/xce".
*
* As a defensive measure beyond the letter of AC 10 (still within its
* "before opening it for writing" intent): if an entry already named
* `filename` exists and is neither a regular file nor a (validated,
* contained) symlink -- e.g. a directory, device node, FIFO, or
* socket -- validation fails rather than handing back a path task
* 9.2 would blindly fopen().
*
* A second defensive measure, same rationale: an existing REGULAR
* file named `filename` with more than one hard link (st_nlink > 1)
* is also rejected. A hard link shares its target's inode -- unlike
* a symlink, there is no separate "link target" path for realpath()
* to resolve and contain-check, so a hard link planted inside
* record_dir by anything with write access there (pointing at a
* file OUTSIDE record_dir) passes S_ISREG and looks canonically
* contained even though writing through it would alias a file
* outside record_dir. Same threat class as the symlink-escape case
* above (CWE-59 link-following), just via inode aliasing instead of
* path resolution -- rejected with the same -EACCES. This st_nlink
* check applies to the FULLY-RESOLVED target reached after
* realpath() has followed any number of symlink hops -- not just to
* an entry named `filename` directly: a symlink inside record_dir
* may itself point at a hard-linked alias (whether that alias lives
* directly inside record_dir or is reached through further
* intermediate symlinks), and containment alone does not detect
* that the resolved target's inode is also reachable from outside
* record_dir. Both the direct-entry case and every symlink-hop case
* are checked against the same resolved target's st_nlink.
*
* Returns 0 on success; `out_path` holds the resolved, validated full path
* (record_dir's canonical form + '/' + the validated bare filename).
* Returns a negative errno on failure:
* -EINVAL `record_dir`/`filename`/`out_path` is NULL,
* `out_path_len` is 0, or `filename` is empty, contains
* '/', or contains "..".
* -ENAMETOOLONG the resolved path does not fit in `out_path_len`.
* -ENOENT `record_dir` does not exist / cannot be resolved.
* -ENOTDIR `record_dir` resolves to something other than a
* directory.
* -EACCES an existing entry named `filename` inside `record_dir`
* either (a) resolves (via realpath()) to a location
* outside `record_dir` -- the symlink-escape case -- or
* (b) is, or (via any number of symlink hops) resolves
* to, a regular file with more than one hard link
* (st_nlink > 1) -- the hard-link inode-aliasing case,
* checked against the fully-resolved target either way;
* this function exists to catch both.
* -EEXIST an existing entry named `filename` inside `record_dir`
* is neither a regular file nor a symlink (see the
* defensive measure above).
*
* No filesystem call is made on `filename` itself until step 2 passes --
* and even then, only `record_dir` and (if present) the pre-existing
* `record_dir/filename` entry are ever touched. Any malformed-string
* rejection in step 1 returns -EINVAL without a single filesystem call,
* even if `record_dir` itself does not exist.
*
* TOCTOU note (closed by sched_record_start() below, task 9.2): this
* function's containment/aliasing checks are only valid up to the moment it
* returns -- a TOCTOU race could still swap a symlink or hard link into
* `out_path` between this call returning and the eventual fopen()/open().
* sched_record_start() closes that window by (a) passing O_NOFOLLOW so a
* symlink swapped in after validation is refused at open() rather than
* followed, and (b) fstat()-ing the resulting fd and rejecting unless
* st_nlink == 1, since O_NOFOLLOW alone does not defend against a hard link
* swapped in after validation (no symlink is involved in that race, so
* O_NOFOLLOW never triggers).
*/
extern int sched_validate_record_filename(const char* record_dir,
const char* filename, char* out_path,
size_t out_path_len);
/*
* sched_record_start - Req 5 AC 10 (scheduler-core spec, task 9.2,
* "Recording lifecycle -- On record start"). Begins a new recording:
*
* 1. Resolves/validates `record_file` (a bare filename, or NULL/"" to
* auto-generate one) via sched_validate_record_filename() above,
* against `sched->record_dir`.
* 2. Opens the resolved path with the TOCTOU-hardened sequence this
* header's own doc comment describes (O_NOFOLLOW + fstat()+
* st_nlink==1 check) -- closes the race window
* sched_validate_record_filename() itself flags as still-open after
* its own return.
* 3. Takes a full core snapshot (xce_sched_core_save_snapshot(),
* core.h) and writes `ReplayHeader` followed by the initial
* `SnapshotEntry` as the first two messages of the file (Req 5 AC 20:
* "the first data message after ReplayHeader MUST be a
* SnapshotEntry").
* 4. On success, sets `sched->recording = true`, stores the resolved
* path in `sched->record_file_path` (strdup'd), resets
* `sched->record_event_count = 0`, and stamps
* `sched->record_start_ts = xce_core_now(&sched->core)`.
*
* Preconditions: fails with -EALREADY if `sched->recording` is already
* true (design.md's NormalDriver contract) -- no file is touched. Fails
* with -EINVAL if `sched` is NULL.
*
* Postconditions: on success, `ReplayHeader` + the initial `SnapshotEntry`
* are durably on disk (fwrite completed) before this function returns.
* On any failure after opening the file (write error, snapshot failure),
* the partially-written file is closed and this function returns a
* negative errno WITHOUT setting `sched->recording` -- the caller may
* retry with a fresh sched_record_start() call. Never leaves `recording`
* true with a NULL `record_file`, or vice versa (scheduler.h's own
* invariant comment on `xce_scheduler_t.recording`).
*
* `record_file` may be NULL or empty to auto-generate a timestamped name
* (`scheduler-replay-<UTC timestamp>.replay`) beneath `sched->record_dir`.
*
* Concurrency (task 9.4 fix round): step 4's field commit takes
* `sched->record_lock` (scheduler.h's field comments) -- see
* sched_record_status()'s doc comment below for the cross-thread reader
* this protects against. The lock is held only around the five assignments
* themselves, never across steps 1-3's file I/O.
*/
extern int sched_record_start(xce_scheduler_t* sched, const char* record_file);
/*
* sched_record_stop - Req 5 AC 10 (scheduler-core spec, task 9.2,
* "Recording lifecycle -- On record stop"). Ends the current recording:
* writes a `ReplayEvent{body=endSession}` message (the end-of-recording
* marker -- there is no separate top-level footer message type, see
* requirements.md Req 5 AC 20), then fflush()+fclose()s `sched->record_file`
* and clears `sched->recording`/`sched->record_file`/`sched->record_file_path`
* (freeing the latter) regardless of whether the marker write succeeded
* (matches Req 5 AC 10's "Error handling" clause: a failed record write
* disables recording rather than leaving it in a half-broken active state).
*
* Preconditions: fails with -EALREADY, no state touched, if
* `sched->recording` is already false. Fails with -EINVAL if `sched` is
* NULL.
*
* Returns 0 on a clean stop; a negative errno if the end-of-recording
* marker failed to write (state is still fully cleared either way, per the
* error-handling clause above -- the negative return is diagnostic only,
* not a signal that recording is still active).
*
* Concurrency (task 9.4 fix round): the fflush()+fclose()+state-clear
* sequence above takes `sched->record_lock` (scheduler.h's field
* comments) around the state-clear portion only -- see
* sched_record_status()'s doc comment below for the cross-thread reader
* this protects against. In particular, freeing `record_file_path` and
* setting it NULL happens atomically from that reader's point of view.
*/
extern int sched_record_stop(xce_scheduler_t* sched);
/*
* sched_record_write_events - the per-event-batch recording-write hook
* (Req 5 AC 10, "On each core_push_events() call ... For each event in the
* batch, serialize and write the corresponding ReplayEvent message").
* Called from xce_sched_driver_translate_pending_events() (driver.c) once
* per drained batch, immediately after xce_core_push_events() and BEFORE
* that function's own cleanup loop frees any translated payload (so every
* pointer inside `events` is still guaranteed live).
*
* Boolean-gated: a NULL `sched`, a NULL/empty `events`, or
* `sched->recording == false` is a zero-cost no-op (Req 5 AC 10,
* "Performance"). On a write failure mid-batch, recording is disabled
* (mirrors sched_record_stop()'s cleanup, minus the end-of-recording
* marker -- the file is presumably broken already) and a negative errno is
* returned; the caller (driver.c) does not need to react further -- normal
* scheduling continues either way (Req 5 AC 10, "Error handling").
*
* Call BEFORE the batch is applied to the core (bug-1852): applying moves
* payload fields out of the events (core_apply_worker_resvals()'s
* MOVE_PTR of each resval, a JOBS_PUSH's allocs, a JOB_RUSAGE's rusages,
* a CONFIG_PUSH's shared resvals), so an event encoded afterwards is
* missing exactly those fields and replays differently. The periodic
* checkpoint is a separate step, sched_record_checkpoint_after_apply()
* below, because a SnapshotEntry must reflect the events written ahead
* of it.
*
* Concurrency (task 9.4 fix round): the per-event increment of
* `sched->record_event_count` takes `sched->record_lock` (scheduler.h's
* field comments), scoped to just the increment -- see
* sched_record_status()'s doc comment below for the cross-thread reader
* this protects against. `record_snapshot_interval` is driver-thread-only
* and not protected by this lock (scheduler.h's field comment).
*/
extern int sched_record_write_events(xce_scheduler_t* sched,
xce_core_event_t* events, int n);
/*
* sched_record_checkpoint_after_apply - periodic checkpoint (Req 5 AC 11,
* task 9.3), called AFTER the batch just recorded by
* sched_record_write_events() has been applied to the core. If
* `sched->record_snapshot_interval > 0` and the (cumulative, since
* recording started) `sched->record_event_count` has just landed exactly
* on a multiple of it, a fresh `SnapshotEntry` message is appended (via
* recording.c's shared write_snapshot_entry(), the SAME builder
* sched_record_start() uses for the initial one). `record_snapshot_interval
* == 0` disables this entirely. Checked ONCE per call against the
* post-batch count, not once per multiple crossed within one large batch
* -- see recording_events.c's own comment for the full boundary-semantics
* rationale. A checkpoint write failure disables recording via
* sched_record_abort_on_error(), the exact same path as any other
* recording write failure. Same zero-cost no-op gate as
* sched_record_write_events().
*/
extern int sched_record_checkpoint_after_apply(xce_scheduler_t* sched);
/*
* sched_record_session_ctx_t - opaque per-round accumulator for the
* decisions a recording session collects between
* sched_record_session_begin()/sched_record_session_end() (Req 5 AC 10,
* "On core_go(): begin building a ReplaySession message" /
* "In the decision drain loop: append a ReplayDecision" /
* "On core_end_session(): finalize ... write the complete message").
* Defined privately in recording.c; driver.c only ever holds a pointer.
*/
typedef struct sched_record_session_ctx_s sched_record_session_ctx_t;
/*
* sched_record_session_begin - starts accumulating a ReplaySession for one
* normal-mode round. Returns NULL (zero allocation) when `sched` is NULL,
* `sched->recording` is false, or `sched->record_file` is NULL -- the
* caller (xce_sched_driver_run_normal_mode_round(), driver.c) then simply
* never calls sched_record_session_add_decision()/sched_record_session_end(),
* satisfying Req 5 AC 10's "zero extra allocation when recording == false".
* On success, returns a heap-allocated context stamped with
* `begin_round_ts` (the ReplaySession.beginRound field).
*/
extern sched_record_session_ctx_t*
sched_record_session_begin(xce_scheduler_t* sched, uint64_t begin_round_ts);
/*
* sched_record_session_ctx_new - scheduler_t-free half of
* sched_record_session_begin() above (scheduler-core spec, task 11.5, Req 5
* AC 19): allocates and stamps a fresh sched_record_session_ctx_t with
* `begin_round_ts`, with no `sched->recording`/`sched->record_file` gate at
* all. sched_record_session_begin() is a thin wrapper around this function
* (gate check, then this call) -- driver.c keeps calling
* sched_record_session_begin() unchanged; this entry point exists so a
* caller with NO xce_scheduler_t* (the simulation driver, sim_record.c,
* Req 5 AC 17) can still build the identical accumulator, gating on
* whatever it has instead (sim->record_file != NULL). Returns NULL only on
* allocation failure.
*/
extern sched_record_session_ctx_t*
sched_record_session_ctx_new(uint64_t begin_round_ts);
/*
* sched_record_session_add_decision - appends one drained decision to
* `ctx`'s in-progress ReplaySession.decisions list, tagged with `ts`. A
* NULL `ctx` (recording inactive -- see sched_record_session_begin() above)
* or NULL `decision` is a no-op. XCE_DECISION_PREEMPT is not yet supported
* (mirrors driver.c's xce_sched_driver_on_decision() -- no live allocator
* path produces one, and this is therefore currently unreachable, but is
* handled loudly (ERROR log) rather than silently dropped or crashing) --
* see driver.c for the shared rationale. XCE_DECISION_NONE/
* XCE_DECISION_NEW_SESSION are likewise skipped (nothing to record).
*
* bug-1843 fix: if growing `ctx`'s internal decisions array fails
* (realloc() returns NULL), this decision -- and every one still to come
* this round -- goes unrecorded; `ctx` is marked internally so
* sched_record_session_finalize() below refuses to write the resulting
* incomplete ReplaySession as though it were a genuine, smaller round (see
* that function's own doc comment). The live scheduling round itself is
* entirely unaffected either way -- only this round's .replay FILE
* representation is dropped.
*/
extern void sched_record_session_add_decision(sched_record_session_ctx_t* ctx,
uint64_t ts,
xce_decision_t* decision);
/*
* sched_record_session_end - finalizes and writes the accumulated
* ReplaySession message (begin_round + decisions + a SessionSummary
* computed from `end_ts` and the tallies sched_record_session_add_decision()
* collected) to `sched->record_file`, then frees `ctx` and everything it
* owns. A NULL `ctx` is a no-op (0 is returned; nothing to free). On a
* write failure -- including bug-1843's incomplete-`ctx` case, see
* sched_record_session_finalize()'s own doc comment below -- recording is
* disabled the same way sched_record_write_events() does. Always frees
* `ctx` before returning,
* even on failure or when `sched`'s recording state changed out from under
* the caller between sched_record_session_begin() and this call.
*/
extern int sched_record_session_end(xce_scheduler_t* sched,
sched_record_session_ctx_t* ctx,
uint64_t end_ts);
/*
* sched_record_session_finalize - scheduler_t-free half of
* sched_record_session_end() above (scheduler-core spec, task 11.5, Req 5
* AC 19): builds the ReplaySession wire message from `ctx`'s accumulated
* decisions and `end_ts`, writes it to `f` directly, then frees `ctx` and
* everything it owns. sched_record_session_end() is a thin wrapper around
* this function (its own gate check plus the same
* sched_record_abort_on_error() call on failure); this entry point exists
* so a caller with no xce_scheduler_t* (the simulation driver,
* sim_record.c, Req 5 AC 17) can finalize+write its own accumulated
* session against its own FILE* (sim->record_file) without that gate.
* Returns 0 on success, a negative errno on failure -- either from
* write_capnp_message(), or -ENOMEM (bug-1843 fix), without writing
* anything, when `ctx` was marked truncated by a decision-batch growth
* failure inside sched_record_session_add_decision() -- writing `ctx`'s
* necessarily-incomplete `decisions` prefix would otherwise produce a
* ReplaySession wire message indistinguishable from a genuine, smaller
* live round, corrupting the recording in a way --sim-verify has no way to
* detect (bug-1843's own "expected: <none>" spurious mismatches). Same
* "always frees ctx" contract either way -- same contract
* sched_record_session_end() itself documents.
* `ctx` must be non-NULL (unlike sched_record_session_end(), this function
* has no NULL-ctx no-op case -- the caller already knows it has a live ctx
* by construction, since it only ever calls this after a successful
* sched_record_session_ctx_new()).
*/
extern int sched_record_session_finalize(FILE* f,
sched_record_session_ctx_t* ctx,
uint64_t end_ts);
/*
* sched_record_status - Req 5 AC 10 (scheduler-core spec, task 9.4,
* "Reply field semantics by action" -- the `recordStatus` column). A pure
* read of `sched`'s CURRENT recording-state fields into a
* `sched_admin_reply_t` the caller (handler.c's scheduler_req_handler())
* sends back over the wire immediately -- no file I/O, no state mutation,
* and (unlike sched_record_start()/sched_record_stop()) no session_active
* dependency, so it is safe to answer directly from the request-handling
* thread rather than deferring through the driver-thread pend_updates
* queue the way recordStart/recordStop must.
*
* Concurrency (task 9.4 fix round): this function is called from a vtask
* worker pthread (handler.c's scheduler_req_handler(), invoked with no
* lock held -- lib/xcectx.c), which runs concurrently with the
* single-threaded driver loop's sched_record_start()/sched_record_stop()/
* sched_record_write_events()/sched_record_abort_on_error() -- all of
* which mutate the same fields this function reads, with no other
* synchronization. This function therefore takes `sched->record_lock`
* (scheduler.h's field comments) around every read of those fields,
* INCLUDING the copy of `record_file_path` described next.
*
* `reply->file` is a fresh, caller-owned heap copy of
* `sched->record_file_path` (via strdup(), taken while still holding
* `sched->record_lock` -- copying the bytes before the lock is released is
* what makes this safe), NOT a borrowed alias of the live pointer. Unlike
* every other capnp encode call in this codebase, this reply crosses a
* function-return boundary before it is ever encoded, so a borrowed
* pointer into `sched`'s live, mutable state cannot be handed back safely
* -- the driver thread could free() it the instant `record_lock` is
* released, well before the caller's eventual xcecli_reply_schedadmin()
* call. The caller MUST free(reply->file) once it is done (NULL when not
* recording -- nothing to free).
*
* Returns 0 on success, populating every field of `*reply`
* (`reply->ok = true` unconditionally). Returns -EINVAL if `sched` or
* `reply` is NULL; `*reply` is left untouched in that case. Returns
* -ENOMEM if the internal strdup() of `record_file_path` fails; `*reply`
* is left zeroed (via the initial memset()) in that case -- this is the
* one way reading status can now itself fail, a direct consequence of no
* longer handing back a borrowed pointer.
*/
extern int sched_record_status(xce_scheduler_t* sched,
sched_admin_reply_t* reply);
#endif