Skip to content

bug: a WAL checkpoint is blind to same-process readers and can backfill past a live snapshot #728

Description

@dpsiderius

ADR-0047 (as amended on PR #723) now records this honestly; this is the
follow-up it names.

  • src/vfs/shm.rs::active_reader_marks probes slot occupancy with a
    non-blocking F_WRLCK. POSIX locks do not conflict within one process, so a
    reader mark held by this process is invisible to it — the exact bug class
    bug: two Connections on one file in one process do not lock against each other — a write that reports success is silently discarded #706 exists to fix, left unfixed for this one lock. With one in-process
    WalReadLock held, active_reader_marks() returns empty.
  • src/pager/checkpoint.rs folds an empty mark list into
    .unwrap_or(total_frames), so the bound becomes the whole WAL — a checkpoint
    can backfill past a live same-process reader's snapshot.
  • Two in-process readers racing claim_wal_read_lock can land on the same slot,
    the second clobbering the first's published mark.

Reachability, measured: Pager::switch_wal_to_journal calls
checkpoint_passive in a loop and is reachable from the public
Pager::set_journal_mode; PRAGMA journal_mode=WAL then =DELETE works from
the CLI today and removes the -wal file. So the checkpoint path is live now.
What is still missing for harm is a second in-process reader, which needs the
unmerged embedding API (PR #705) — so not a live data-loss bug today, and it
stops being dormant the moment that pool lands.

Scope: route claim_wal_read_lock and active_reader_marks through the
in-process arbiter #706 added (src/vfs/inode_registry.rs) so a same-process
reader mark is visible and two readers cannot share a slot; make the
empty-mark-list fallback in checkpoint.rs fail closed rather than open. Tests:
a same-process reader must bound a same-process checkpoint; two in-process
readers must get distinct slots. While in this file, add the direct unit tests
inode_registry.rs still lacks (357 lines covered only by 4 oracle-gated
integration tests that no-op without the pinned oracle; SHARED ref-counting and
mutex-poison recovery were both verified by hand during review and neither is
regression-tested).

Complexity

Estimate: medium
Reasoning: The arbiter already exists; this extends it to one more
lock, tightens one fallback, and backfills unit tests.

Refs: 013/Req-4, #491, #412, #706, #723, ADR-0047

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

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions