Skip to content

Protect exactly what the last scan saw when materializing - #188

Merged
myobie merged 2 commits into
mainfrom
fix/sync-rename-atomic
Sep 5, 2026
Merged

Protect exactly what the last scan saw when materializing#188
myobie merged 2 commits into
mainfrom
fix/sync-rename-atomic

Conversation

@myobie

@myobie myobie commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

A local rename could leave both the old and the new path present on every peer. Fixes #175.

What went wrong

A sync pass has two guarded halves around an unguarded peer step. The first half scanned, materialized, and captured a disk view. The second half scanned again and materialized with that older view.

During the peer step the guard is free. An inbound session from the peer can run then, and it materializes the peer's file onto local disk. The older view does not contain that file. If a rename of that file lands between the second scan and its materialization, the materialization sees a Present manifest entry, a missing file, and no record that the file was ever here. That combination means "remote-only file, write it", so the old path was written back. The next scan found both paths present and recorded the new one. The delete was never recorded, so it could never propagate.

The daemon's sync reload runs a pass for every entry while the entry loop may already be in one, so two passes on one entry can interleave through the guard. That is what produced the overlap in the integration test.

The same shape existed in the first half of a pass and in the completion of an inbound session: a file created inside the window and removed between the scan and its materialization came back.

The rule

Every materialization protects exactly what the scan immediately before it saw, under the same hold of the operation guard.

  • A path the scan saw on disk and that is gone at materialization is a local delete. It becomes a tombstone. It is never written back.
  • A path no scan in that guarded section saw is remote-only. It is materialized.
  • A path whose bytes changed after the scan is a local edit. Its bytes win, as before.

materialize_entry_state now takes that view for itself at the start of each attempt. Its protected parameter is removed, so no caller can pass a stale view again. The baselines the callers carried across the peer step and the wire session now decide only whether there is anything to persist.

Proof

Five new tests pin events to test-only seams between each scan and the materialization that follows it. Three reproduce the defect and two are controls.

Test Before (30 runs, 22:51:05Z to 22:51:16Z) After (30 runs, 23:01:13Z to 23:01:24Z) Fix reverted alone (30 runs, 23:02:05Z to 23:02:16Z)
Rename between the post-peer scan and its materialization failed 30 passed 30 failed 30
File created and removed inside one inbound session failed 30 passed 30 failed 30
File created and removed around the pre-peer scan failed 30 passed 30 failed 30
Same rename one seam earlier (control) passed 30 passed 30 passed 30
Remote-only path adopted during the peer step still materialized (control) passed 30 passed 30 passed 30

The first test asserts, before it looks at the outcome, that the seam fired while the file was on disk and Present in the manifest. The last control fails if a fix protects every manifest path instead of every scanned path, which would stop remote-only files from being written.

The full library suite passed 538 tests with 5 ignored on the final head, 23:01:24Z to 23:01:55Z.

The seams do not exist in production builds

The seam enum, the hook slot, set_seam_hook, at_seam, and every call to at_seam are under cfg(test). A production reference to any of them cannot compile, and a non-test cargo check --lib passes. The release binary built from this branch holds 0 symbols matching the seam names against 61 symbols for the surrounding functions and 62,805 symbols in total.

Out of scope

A remote edit of the source at a higher version, concurrent with the local rename, brings the edited source back beside the new path. Present wins over a tombstone at an equal version and a higher version wins outright. That is the designed newer-wins rule for a real concurrent edit and rename, not this defect.

A crash between the scan and the persist leaves the durable state at the pre-rename version with the source still in the observed receipts. The first scan after restart finds the source absent and observed, records the tombstone, and records the destination as new. Nothing about this change alters persistence.

A local rename can end with both the old and the new path present on
every peer. The sync pass materializes with a disk view it captured
before the peer step. A file that an inbound session materialized
during the peer step is missing from that view, so a rename that
lands between the post-peer scan and its materialization looks like
a file this machine never had, and the old path is written back.

This adds test-only seams between each scan and the materialization
that follows it, and five tests that pin events to those seams:

- a rename at the post-peer seam splits the file (fails today),
- a file created and removed inside one inbound session comes back
  (fails today),
- a file created and removed around the pre-peer scan comes back
  (fails today),
- the same rename one seam earlier is recorded by the scan (passes),
- a remote-only path adopted during the peer step is still written
  (passes).

No behavior changes. Refs #175.
A local rename could leave both the old and the new path present on
every peer. The sync pass captured a disk view before its peer step
and materialized with that view after it. An inbound session that ran
during the peer step had already written the peer's file to disk, so
the view lacked it. A rename landing between the post-peer scan and
its materialization then read as a file this machine never had, and
the old path was written back. The delete was never recorded.

Every materialization now takes its protected view for itself, under
the caller's operation guard, at the start of each attempt. That view
is what the scan immediately before it published. The function no
longer accepts a caller's view, so no caller can hand it a stale one.
The same rule stops a file created and removed inside one pass or one
inbound session from coming back.

The seam hooks and the hook slot are compiled only in test builds.
A non-test `cargo check --lib` passes, and the release binary holds
no seam symbol while it holds 61 symbols for the surrounding
functions.

Fixes #175.
@myobie
myobie merged commit 931b77d into main Sep 5, 2026
3 checks passed
@myobie

myobie commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Integration samples for the record. These are reported, not used as proof. The test bus_update_beats_equal_version_delete_then_archive_survives_restart is schedule-dependent, and issue #175 was filed after it passed four of four times with the defect present. The seam tests in this pull request are the proof.

Binary built at Runs Passed Window (UTC, 2026-09-05) Per-run time
9c34ce0 (seams and tests, no fix) 10 10 22:55:14Z to 23:16:47Z 128 to 130 s
aa94381 (with the fix) 10 10 23:16:58Z to 23:38:32Z 128.55 to 131.18 s

Both series ran serially on one macOS machine with --test-threads=1, each from a copy of the test binary taken before the other series started. The before-fix series ran while the library test suite and a release build were compiling on the same machine. The after-fix series ran on an otherwise idle machine.

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.

Make sync rename atomic against concurrent inbound state

1 participant