jcp-wire: one writer per journal, or the replay spends twice #346
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/journal-single-writer"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
SPEC §5 requires an enforcement point that can restart to record every allowed decision durably, and to re-establish spend by replaying that record. Nothing said the record may have only one writer, and
FileJournal::openwas:with no lock at all.
The failure is a double-spend, not a corrupt file
Two endpoints sharing one journal each replay it, each derive the same remaining budget, each decide against it, and each append. The budget is spent twice, every record still verifies, and nothing in the journal shows it. The durability requirement is defeated one layer beneath itself — the same shape as the refill-on-restart defects fixed in #305 and #310, except the replay that fixed those is what is being defeated.
openalso truncates a torn trailing record, so a concurrent opener can cut the file out from under an appender.The fix
opentakes an exclusive lock, before the read and the truncation. An opener refused afterwards would already have cut the holder's file — the ordering is the property, not merely the presence of a lock.The lock is
std::fs::File::try_lock, stable since Rust 1.89: nounsafe, no new dependency, no lockfile change in any workspace, and no platform caveat, because std covers Windows as well as unix.Verified on the toolchain rather than assumed — a second
try_lockon a held file returnsWouldBlock, and the lock releases when the file is dropped. It is also released when the process dies, so a crash leaves no stale lock to recover; a lock-file sidecar would need a story for that case, and that story is where such schemes go wrong.Where a platform cannot lock at all,
openfails rather than proceeding. Deciding against a record another endpoint may also be writing is the overspend this exists to stop, and a platform limit is not a licence for it.SPEC §5 gains the requirement
The gap was in the spec before it was in the code, so prose alone would not stop a reimplementation repeating it. §5 now requires a single writer per record, failing to start over proceeding beside another writer, prefers an exclusive claim the OS releases on death, and requires failing to open where a platform cannot provide exclusivity at all.
No conformance clause (
C…) is added: the pack has no vector for it, and numbering a clause nothing tests would claim coverage that does not exist. That vector is the follow-up.Falsification
the_single_writer_claim_matches_what_open_does+ 1 FAILEDa_refused_open_does_not_truncate_the_holders_journalFAILEDthe_single_writer_claim_matches_what_open_does+ 1 FAILEDthe_single_writer_claim_matches_what_open_does+ 2 FAILEDa_platform_that_cannot_lock_fails_closedFAILEDM5 survived the first round, and that was the finding. Swallowing
TryLockError::Errorbroke no test, because that arm is unreachable on a platform whose locks work — and an unreachable arm is exactly where a fail-open sits unnoticed.lock_outcomeis split out from the I/O so all three outcomes are reachable from a test, and M5 now fails only its own.M2 is the other one worth reading: the lock is still present, just in the wrong place, and only the truncation test notices.
A blocking-lock mutation was deliberately not used: it would hang the suite rather than fail a test, and a mutation that hangs reports nothing.
Verification
cargo check --workspace --all-targets --locked(jcp-rs) — exit 0cargo test --workspace --locked— exit 0, 64 test binaries green, no failurescargo clippy -p jcp-wire --all-targets --no-deps --locked -- -D warnings— cleancheck-lockfiles— 42 lockfiles, all current (unchanged);check-spec-refs1673 citations / 16 standardsThe sibling
mesh-core's journal had the identical defect and is fixed the same way in #347. Mesh's journal is hash-chained, so there the chain detects a second writer — but only after both stewards have already served against the same spend, and at the cost of making the journal unreplayable. The two standards deliberately keep their own copies of this machinery, asmesh-receiptalready vendors JCR-1 rather than depending onjcp-schema, so each gets its own change rather than a shared crate.🤖 Generated with Claude Code
SPEC §5 requires an enforcement point that can restart to record every allowed decision durably and to re-establish spend by **replaying** the record. Nothing said the record may have only one writer, and `FileJournal::open` was OpenOptions::new().read(true).append(true).create(true) with no lock at all. **The failure is a double-spend, not a corrupt file.** Two endpoints sharing one journal each replay it, each derive the *same* remaining budget, each decide against it and each append. The budget is spent twice, every record still verifies, and nothing in the journal shows it — the durability requirement is defeated one layer beneath itself. `open` also truncates a torn trailing record, so a concurrent opener can cut the file out from under an appender. `open` now takes an exclusive advisory lock, and takes it **before** the read and the truncation: an opener refused afterwards would already have cut the holder's file. `flock` is the choice rather than a lock-file sidecar because the kernel releases it when the descriptor closes *or the process dies*, so a crash leaves no stale lock and there is no recovery story to get wrong. `libc` is added under `[target.'cfg(unix)'.dependencies]` — already a dependency of `jcp-gateway` and `jcp-intercept` at the same version, so no new external crate enters the tree. `flock` is POSIX. Rather than let the type imply a property it cannot hold everywhere, `FileJournal::SINGLE_WRITER_ENFORCED` states it as a constant a caller can **check** at run time, and the test asserts the constant against the behaviour in *both* directions — where it claims enforcement a second open must be refused, and where it does not a second open must succeed. A branch that asserted nothing would leave the constant free to understate as well as over-claim. SPEC §5 gains the requirement, since the gap was in the spec before it was in the code: a single writer per record is MUST, failing to start beats proceeding beside another writer, an exclusive claim the OS releases on death is preferred, and a platform that cannot enforce it MUST say so checkably. Falsification, four mutations, all killed by their named test: - no lock at all (the pre-fix behaviour), and a shared lock instead of an exclusive one, and a contended lock reported as success — each fails the claim-matches-behaviour test; - the lock taken *after* the read and truncation fails **only** the no-truncation test, which is what isolates the ordering rather than the presence of the lock. Clippy also caught a flaw in my first version of that test: an `assert!(!SINGLE_WRITER_ENFORCED)` in the else branch was a constant assertion and vacuous. It now asserts behaviour instead. **`mesh-core`'s journal has the identical defect** and is not touched here; there is no exclusive-lock precedent anywhere else in open-standards, so this introduces the pattern. Co-Authored-By: Claude Opus 5 <[email protected]>f246a411df8fb6588e63