Skip to content

fix: durable free-list and CoW invariant violations causing silent store corruption#7

Merged
farhan-syah merged 9 commits into
NodeDB-Lab:mainfrom
mkhairi:fix/free-page-reuse-durability
Jul 19, 2026
Merged

fix: durable free-list and CoW invariant violations causing silent store corruption#7
farhan-syah merged 9 commits into
NodeDB-Lab:mainfrom
mkhairi:fix/free-page-reuse-durability

Conversation

@mkhairi

@mkhairi mkhairi commented Jul 19, 2026

Copy link
Copy Markdown
Member

Summary

Sustained write workloads (a few hundred to a few thousand puts with periodic
commits) could durably corrupt a store with no error reported at write time.
The store would later fail reads with checksum / AEAD tag verification failed
— often only after a clean shutdown and reopen — with no recovery path.

This PR fixes one root cause and four contributing defects found while
tracing it, and adds a cross-backend structural stress suite that catches
all of them.

Root cause: dangling free-list chain pointer

freelist::write_chain stopped writing pages as soon as all entries were
consumed — but the preceding page's next pointer was already set from the
supplied chain_pages. When a host carve in rewrite_chain shrank the entry
set across a page boundary (≈1 in chain_capacity commits under sustained
load), the durable chain ended in a pointer to a page whose bytes were never
rewritten.

Two failure modes, depending on the stale bytes:

  • They fail kind authentication → every subsequent begin_write's
    read_chain errors forever; the store is wedged and cannot reopen.
  • They still authenticate as an older chain generation (same page id,
    Free kind, valid seal) → read_chain silently resurrects ancient free
    entries naming pages that are live again → the allocator overwrites live
    tree pages. Reads stay green while the pages are warm in cache and fail
    only after eviction or reopen.

Verified on a damaged store produced by the reproduction workload: the chain
walk ends at a page whose bytes authenticate as Overflow, not Free.
tests/freelist_chain.rs hits the exact carve boundary and fails pre-fix.

Contributing defects

  • btree split path remap: on an internal split, propagate_split_up
    remapped every cached dirty-leaf parent path to the left half
    unconditionally; leaves under the right half never got their parent CoW'd
    at flush, leaving a durable internal pointing at a freed leaf page. Root
    growth also failed to prepend the new root to existing paths.
  • in-session recycling of durable-header pages: with no reader tracked,
    the reuse threshold was 0, so a page freed this session could be
    reallocated in the same session — but a CoW free does not unreference the
    page on disk until the next header lands. Threshold is now always the
    session-start next_page_id; cache-drawn pages (free per the durable
    header, below the reclamation floor) stay recyclable so file growth
    remains bounded.
  • pager flush + cache: flush_file cleared dirty flags even for pages
    replaced by a concurrent writer during the seal+write window (silent lost
    update); warm cache hits skipped the expected_kind check that cold reads
    enforce via AAD, hiding structural damage until reopen.
    Read-authentication failures now log page id / file / expected kind.
  • tokio VFS: len() raced in-flight background writes
    (File::metadata() is the only op that does not await them), the source
    of the truncate_and_len flake under parallel test load.

Tests

  • tests/freelist_chain.rs — chain round-trip regressions: the carve
    boundary (fails pre-fix), a capacity boundary sweep, single-entry host.
  • tests/structural_repro.rs — five scenarios, each on MemVfs, TokioVfs,
    and IouringVfs: sustained put/rewrite/overflow with per-commit read-back;
    close/reopen cycles; a commit-cancellation sweep dropping the commit
    future at every await point and demanding byte-exact pre- or post-commit
    state on reopen; readers racing a writer; seeded randomized structural
    stress with a deep-walk invariant check after every commit.

Full suite: 396 passed / 0 failed (41 suites), including the previously
flaky truncate_and_len and the deferred_free_reclaim growth bound.

External validation: a reproduction workload (1000–2000 mixed puts with
overflow values, commit every ~500 ms, SIGTERM, reopen) corrupted the store
on 100% of runs before this PR and survives 4/4 runs with it.

Known follow-ups (out of scope)

  • A repair path for already-damaged stores (rebuild the free-list from a
    deep walk's orphan set) — currently they remain unopenable.
  • overflow::release's Decremented contract frees the old root while
    remaining refholders point at it; dead code today (increment_ref has no
    in-tree callers) but the contract is corrupting if ever used.
  • Reader-vs-reuse read races under concurrent load now surface as clean
    authentication errors (previously silent wrong-kind reads); reader pinning
    could eliminate the noise.
  • PagedbError::Aborted's display string says "reader stall policy" for all
    origins (counter monotonicity, nonce anchor budget); splitting the variant
    would make field diagnosis much faster.
  • The default nonce anchor_budget (1024) self-aborts any commit dirtying
    more than 1024 pages; sizing it from the commit's dirty-page count would
    remove a hard ceiling on bulk writes.

mkhairi added 9 commits July 19, 2026 15:57
…inter

write_chain stopped writing pages as soon as all entries were consumed,
but the preceding page's next pointer was already set from chain_pages —
so when a host carve in rewrite_chain shrank the entry set across a page
boundary, the chain was durably linked to a page whose bytes were never
rewritten. Depending on what those stale bytes held, the next read_chain
either fails authentication forever (every subsequent commit wedges) or
decodes an older chain generation and silently resurrects free entries
for pages that are live again, letting the allocator overwrite live tree
pages. Trailing chain pages are now always written (zero entries,
properly linked), and rewrite_chain no longer carves a host when only a
single entry remains, which would orphan the host page.

read_chain also now rejects an on-disk entry count exceeding page
capacity as Corruption instead of panicking on the slice overrun — a
panic there poisons the pager lock and wedges every subsequent commit.

New tests cover the exact carve boundary (fails pre-fix), a capacity
boundary sweep, and the single-entry sole-host case.
… root

Two holes in propagate_split_up left durable internal nodes pointing at
freed leaf pages:

- On an internal split, every cached dirty-leaf parent path was remapped
  to the left half unconditionally (the comment claimed either half was
  acceptable). Flush's spine walk only CoWs internals named in the
  paths, so a dirty leaf whose true parent was the right half never got
  its parent's child pointer rewritten — the durable tree kept
  referencing the old leaf page while that page entered the free list.
  Paths are now remapped per-leaf to whichever half actually contains
  the path's next hop.

- When a split cascaded into root growth, existing cached paths were not
  prepended with the new root, so the new root's child pointers were
  never rewritten when the old halves were CoW'd at flush.

Either hole makes a freed-but-still-referenced page: once recycled under
another kind, reads of the stale reference fail authentication — or
return wrong data before that, as silent lost updates.

Covered by tests/structural_repro.rs (rewrites-then-appends ordering
triggers the splits after the dirty-path cache is populated).
The btree reuse threshold was 0 whenever no reader was tracked, allowing
a page freed this session to be reallocated in the same session. A
copy-on-write free does not unreference the page on disk until the next
header lands, so recycling it in-session lets a torn or failed commit
leave the durable header pointing at rewritten content. The threshold is
now always the session-start next_page_id.

Pages drawn from the shared free-page cache remain recyclable in-session
even below the threshold: they are free per the last durable header
regardless of what the session writes to them, and they sit below the
reclamation floor, so no header and no reader can observe their old
content. This keeps file growth bounded (the blanket threshold alone
regressed the deferred-free reclaim bound).
…warm reads

Two flush/cache hardenings:

- flush_file cleared every gathered page's dirty flag after the write,
  even when a concurrent writer had replaced the cached Arc during the
  (slow) seal+write window — silently discarding that write. The flag is
  now cleared only when the cache still holds the exact Arc that was
  flushed; a replaced page keeps its flag and flushes next cycle.

- The cache fast path served a page without checking expected_kind,
  while the cold path's AAD binds the kind byte. A structurally wrong
  reference (a pointer to a page recycled under another kind) therefore
  read fine while warm and only failed after eviction or reopen, hiding
  damage from the tests most likely to catch it. Warm hits now enforce
  expected_kind, matching cold-read semantics.

Also logs page id, file, and expected kind when a page fails
authentication on read, and traces each page written by flush — the
forensic trail that located the free-list chain defect.
tokio's File::metadata() is the only file op that does not await the
backend's in-flight background write, so len() could race a completed
write_at and return a stale length. Flush first. Root cause of the
truncate_and_len flake under parallel suite load.
Five scenarios, each run on MemVfs, TokioVfs, and IouringVfs: sustained
put/rewrite/overflow with full read-back per commit; close/reopen cycles
with a mixed workload and cold read-back; a commit-cancellation sweep
that polls the commit future to every await point, drops it, reopens,
and demands byte-exact pre- or post-commit state; concurrent readers
racing a writer then reopen; and seeded randomized structural stress
(cascade splits, inline/overflow flips, deletes, aborted txns, reopens)
with a deep-walk invariant check after every commit.
…iles

The Randomness error variant added with durable identities uses
#[from] getrandom::Error, but getrandom 0.2 only implements
std::error::Error behind its std feature — every native build of the
crate has failed to compile since. CI did not catch it because no run
has happened on main since the commit landed.

Also: rustfmt normalization on files touched by this branch, and
too_many_lines allowances on read_page/flush_file, which crossed the
threshold with the new forensic logging (matching the existing
allowance on commit).
…endency

manifest_validation imports snapshot::export, which is compiled out on
wasm32 — as is its only caller (txn/db/snapshot). The module itself was
not gated, so every wasm32 build failed with an unresolved import. Gate
it the same way.
cargo-deny takes --exclude-dev before the subcommand; the audit job
passed it after `check` and died on argument parsing, so the audit
never actually ran. With it running, two advisories fail the build:
RUSTSEC-2026-0204 (crossbeam-epoch 0.9.18 null-pointer deref in
fmt::Display) and RUSTSEC-2026-0186 (memmap2 0.9.10 unchecked pointer
offset). Lockfile bumped to crossbeam-epoch 0.9.20 and memmap2 0.9.11
per the advisories' guidance.
@farhan-syah
farhan-syah merged commit da042c3 into NodeDB-Lab:main Jul 19, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants