Skip to content

Make Rust/Python atomic-file lock ownership and stale recovery safe #149

Closed

Description

@NOirBRight

Problem

The cross-language atomic-write helpers introduced by #23 use the same timestamp-based lock-file protocol in Python and Rust, but they do not preserve lock ownership when reclaiming or releasing a stale path.

Both src-python/atomic_io.py and src-tauri/src/safe_file.rs treat a lock as stale after 60 seconds, remove the lock path, and let another writer create a replacement. The original owner later removes that same path unconditionally during cleanup. This creates a concrete A/B/C race:

  1. Writer A still owns a long-running operation when its timestamp reaches 60 seconds.
  2. Writer B removes A's path as stale and creates a replacement lock.
  3. Writer A finishes and unconditionally removes B's lock path.
  4. Writer C can now enter while B is still writing.

The same failure can occur across Rust and Python because both use the shared target.lock path and compatible timestamp metadata. Age alone also allows a valid long-running owner to be reclaimed.

Separately, Python atomic_read_or_create_text catches every OSError while reading and then creates/replaces the value. Permission, disk, and other I/O failures can therefore be misclassified as a missing/empty file.

Outcome

Adopt one ownership-safe cross-language lock contract so stale recovery and release can never remove or overlap a different live owner. Surface non-absence read failures from atomic_read_or_create_text instead of converting them into create behavior.

Scope and invariants

  • Define one versioned lock protocol used by Rust and Python for every shared atomic-write target.
  • Prefer a held OS advisory lock whose ownership is released by closing the owning handle and whose cross-language behavior is proven on supported platforms.
  • If a platform fallback is required, use an unguessable owner token plus an atomic ownership/reclamation protocol. A read-token-then-unlink sequence is insufficient unless tests prove replacement cannot occur between validation and removal.
  • A writer may release or reclaim only the lock instance it owns. An old owner must be unable to delete a replacement owner's lock.
  • Do not reclaim a live owner merely because wall-clock age exceeds 60 seconds. Use OS ownership/liveness or a lease/renewal protocol with a proven handoff.
  • Preserve bounded acquisition timeout and sanitized error reporting.
  • Crash recovery must eventually permit a new owner without allowing concurrent critical sections.
  • Make Rust and Python metadata parsing/versioning mutually compatible, including legacy lock files that may remain after an older process crashes. Unknown/malformed state must fail safely rather than immediately authorize an overlapping writer.
  • In Python atomic_read_or_create_text, create only for the defined absent/empty case. Propagate permission, disk, path-type, decoding, and other non-absence failures with their causal error.
  • Audit callers of the two helpers only to update protocol plumbing and error expectations; do not change target file formats or unrelated persistence behavior.

Non-goals

Acceptance criteria

  • A deterministic same-language A/B/C process test proves that after B replaces or legitimately follows A, A's cleanup cannot remove B's ownership and C cannot overlap B.
  • The same choreography passes with Python A/Rust B and Rust A/Python B, using a real shared target path and real processes.
  • A valid critical section held longer than the former 60-second threshold remains exclusive, or renews its lease without a gap; no second writer enters based on age alone.
  • A process killed while holding the lock leaves a state that a later Rust or Python writer can recover within the documented bound.
  • Release is idempotent for the owner and harmless after external crash cleanup; it never removes another owner's lock instance.
  • Simultaneous Rust/Python acquisition produces exactly one critical-section owner and the loser waits or times out with a sanitized causal error.
  • Legacy timestamp-only lock metadata has an explicit, tested compatibility/recovery rule that cannot overlap a still-live old binary.
  • Python atomic_read_or_create_text returns existing non-empty content, creates for the defined absent/empty case, and propagates injected permission/disk/path/decoding failures without invoking create_text.
  • Existing config/catalog/overlay/global-state/history/update-marker atomic-write tests remain green.
  • Lock artifacts, owner tokens, PIDs, and local paths are not exposed in user-facing diagnostics beyond the repository's existing sanitized error boundary.

Expected hotset

  • src-python/atomic_io.py
  • src-tauri/src/safe_file.rs
  • tests/test_atomic_io.py
  • Focused cross-language subprocess fixtures/tests
  • Only directly affected atomic-write callers whose error handling must stop swallowing non-absence failures

Dependencies and relationships

No hard blocker.

Related, not duplicate: #23 created the atomic-write/lock helpers and #30 audited remaining runtime writers. This issue closes a newly confirmed ownership race in that shared protocol and the broad Python read-error classification gap.

Verification

  • python -m pytest -q tests/test_atomic_io.py
  • cd src-tauri; cargo test --locked safe_file
  • Focused real-process Rust/Python interoperability tests for acquisition, long hold, crash recovery, replacement, and release
  • python -m pytest -q
  • cd src-tauri; cargo test --locked
  • cd src-tauri; cargo clippy --locked --all-targets -- -D warnings
  • python scripts/report_quality_gates.py (report-only; non-blocking)

Execution contract

Execution-Contract: v2
Verification-Class: strict
Verification-Commands: python -m pytest -q tests/test_atomic_io.py; cd src-tauri; cargo test --locked safe_file; focused real-process Rust/Python interoperability tests; python -m pytest -q; cd src-tauri; cargo test --locked; cd src-tauri; cargo clippy --locked --all-targets -- -D warnings; python scripts/report_quality_gates.py (report-only)
Manual-Evidence: none
Architecture-Decision: resolved
Review-Owner: orchestrator

Activity

  1. added
    bugSomething isn't working
    ready-for-agentFully specified, ready for an AFK agent
    wayfinder:taskWayfinder implementation task
    on Jul 16, 2026
  2. self-assigned this
    on Jul 17, 2026
  3. NOirBRight commented on Jul 18, 2026

    @NOirBRight
    OwnerAuthor

    Fixed by #180 (merged into dev, b4157b3). All acceptance criteria covered: ownership-safe versioned protocol (no unlink, no age reclaim), OS advisory locks on both platforms, provably-dead-only legacy recovery, fail-closed unknown state, atomic_read_or_create_text error propagation, deterministic A/B/C + cross-language real-process + simultaneous-acquisition tests, sanitized errors, and green caller suites. New rust-safe-file-linux CI job guards the unix FFI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingwayfinder:taskWayfinder implementation task

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions