Skip to content

Session store has no per-session concurrency control #2

Description

@pathscale

Two runs targeting the same session name concurrently can interleave, and the store resolves it last-writer-wins.

What happens today

SessionStore reads a record, runs the agent, then writes the binding back. There is nothing holding the session between the read and the write, so:

  1. Run A resolves thread-42, gets token T1.
  2. Run B resolves thread-42, also gets T1.
  3. Both run, each producing a different continuation.
  4. Whichever finishes last overwrites the binding.

The losing turn is not lost from the provider's side (its conversation still exists) but the name no longer points at it, so it becomes unreachable through the store.

Writes themselves are safe: each goes to a per-process temp file and is renamed atomically, with the parent directory fsynced, so a reader never sees a torn record. The gap is ordering across the whole operation, not file integrity.

Why it is not fixed yet

Doing it properly needs one of:

  • A cross-process lease held for the full read-run-commit cycle, which means flock or O_EXCL lock files, plus a stale-lease policy for the case where a holder is killed mid-run. That last part is the awkward one: an agent turn can legitimately take minutes, so a lease timeout cannot be short.
  • Revision-based CAS, adding revision: u64 to SessionRecord and rejecting a write whose expected revision has moved, returning SessionConflict. Simpler, but rename-based writes give no atomic read-modify-write, so this still needs a lock to be airtight rather than merely narrow.

Why it is low priority

The current consumer is a single-user desktop GUI, where two simultaneous turns on one conversation is a UI defect rather than something the store should be arbitrating. The failure is also recoverable: the provider still holds both conversations, and the id can be recovered from Outcome::session if it was captured.

Worth revisiting if this crate is ever driven by a server handling concurrent requests, or by a queue that can dispatch the same session twice.

Acceptance

  • Concurrent runs on one session name either serialize or fail loudly with SessionConflict.
  • A holder killed mid-run does not deadlock the session forever.
  • A test spawning two runs against one name asserts exactly one binding survives and the other is reported, not silently discarded.

Raised in PR #1 review.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions