Repository navigation
fix(dashboard): keep a save that lands while a refresh is reading the store - #569
Merged
Merged
Conversation
BryanFRD
enabled auto-merge (squash)
October 10, 2026 15:52
Contributor
There was a problem hiding this comment.
The fix is correct. set_access bumps the revision while it holds the write lock, and refresh_access checks it under the same lock, so the check and the swap are atomic. seen is read before read_meta. A dashboard write or reset runs write_meta/delete_meta and then set_access, so any save that overlaps a refresh moves the counter and the refresh's result is dropped. If the refresh already read the new file, dropping it changes nothing. write and reset are the only callers of set_access, and both are explicit changes. Nothing blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #568
refresh_onereads the saved file, awaits, then applies the result. A save (or reset) from the dashboard that completes in that gap was overwritten by the refresh's older read.Authorizernow counts explicit changes (revision, bumped byset_access). A refresh records the revision before it reads and applies throughrefresh_access(seen, access), which does nothing if the counter moved. A refresh's own apply does not bump it, so one refresh never makes the next give up.Tests are deterministic and need no timing: a refresh nobody overtook applies; a save between the read and the apply is kept; a refresh does not move the counter; each explicit change does; with authentication disabled there is nothing to refresh.
I could not reproduce the race on Windows (the integration test passes 60 of 60 here), so the existing
forge_accesstest is the check that matters, and it runs in this PR's CI. Full workspace tests pass locally.