Skip to content

sweep --archive: two-file archive move has no durable rename ordering (power loss can leave stubs without bodies) #715

Description

@pbean

Status: analysis, not reproduced

Nobody has demonstrated this empirically. It is a reading of the write path
raised by codex on the PR #711 review (thread comment 3848352410, rated P1
there) and re-derived while resolving that review. Filed so it is not lost;
the claim below should be confirmed before it is fixed, and the P2 label is a
guess a maintainer should feel free to re-rate.

Claim

deferredwork.archive_closed is the repo's first two-file ledger
transaction
: it appends the moved bodies to deferred-work-archive.md and
rewrites deferred-work.md with the stubs that replaced them. The order is
deliberate and commented — the archive is written first, so a crash between
the two leaves the archive with extra content (harmless, it is append-only)
and the ledger unchanged (safe, the bodies are still live).

That reasoning holds for process death. It does not obviously hold for
host power loss, because ordering two atomic_write_text calls does not
order the two renames on disk:

  • atomic_write_text fsyncs the temp file's contents before os.replace
    publishes them, so a caller never sees a torn or zero-length file.
  • It deliberately does not fsync the parent directory afterwards. The
    documented rationale (platform_util.py, atomic_write_text docstring) is
    that syncing the directory would make the rename durable, and losing a
    rename just leaves the old contents in place — stale, never corrupt.

That rationale is stated for a single-file writer, where it is true: lose
the rename, keep the old file, nothing is inconsistent. Across two files it no
longer follows. Directory entries can reach the platter in either order, so a
power loss after both os.replace calls returned can plausibly recover with:

  • the ledger rename durable (entries replaced by stubs), and
  • the archive rename lost (the appended bodies gone).

The result is stubs pointing at bodies that no longer exist anywhere — the one
outcome the write order was chosen to prevent.

Why a re-run does not repair it

archive_closed skips entries matching _is_stub. After the loss above, the
ledger's entries are stubs, so a second sweep --archive pass considers them
already moved and re-appends nothing. The crash-recovery path in the function
handles the opposite direction only (archive written, ledger not), which is the
order the code can control. There is no path that notices a stub whose archive
block is missing.

Reproduction reasoning

No power-loss harness exists here, so a check would have to be indirect —
something like: instrument or wrap the two atomic_write_text calls, confirm
that neither issues an fsync on the parent directory descriptor after
os.replace (a strace/ltrace count, or a monkeypatched os.fsync
recorder around a real archive_closed call, is enough to establish the
mechanism); then argue the interleaving from filesystem semantics rather
than trying to stage it. On ext4 with the default data=ordered, an
un-fsynced rename is only guaranteed by the next journal commit, so two
independent renames issued microseconds apart have no ordering promise between
them. A stronger demonstration would need a fault-injection layer (dm-flakey,
or a VM killed at the hypervisor) — worth deciding whether that is in scope
before anyone attempts it.

Where a fix would live

Not in deferredwork.py. Making a rename durable is a platform seam:
POSIX wants the parent directory opened and fsynced after os.replace (and,
when follow_symlinks=True resolved the target elsewhere, the resolved
parent, which is the directory the rename actually happened in), while Windows
has no directory-fsync equivalent and would degrade to a documented no-op.
Per the quarantine invariant that belongs in platform_util.py, as an opt-in
variant of atomic_write_text — opt-in specifically because the existing
default is a compatibility contract for every other writer and the extra fsync
is not free.

Only the caller knows it is publishing a multi-file transaction, so the choice
of who asks for durability is part of the design, not a detail.

Relation to the atomic-write hardening work

This is a different axis from the hardening that landed in #712 (for
#710). That work was about confinement and permission semantics — where a
write may land and whose target it may replace — and it deliberately left every
deferredwork writer on the unconfined atomic_write_text; docs/FEATURES.md
scopes the confinement claim accordingly. Nothing here contradicts that
position: durability of the rename is orthogonal to confinement of the path,
and this issue does not ask for those writers to be confined.

Provenance

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Real defect - workaround exists or blast radius is narrowarea:deferred-workdeferred-work.md ledger and sweepbugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions