write_lock_request writes a second request when the engine takes the first - #1750
Merged
Merged
Conversation
…first On drvfs a write can be refused and land all the same. #1694 handled that by asking, after a refused write, whether the request is there and newer than hts-in_progress.lock. The engine polls for that same file and deletes it, so the check can run after the engine already took the request, see nothing, and write a second copy. Write the request under <name>.staged and move it into place instead. The staging file going away says the move happened, and the engine never reads that name, so nothing can take it first. A directory sitting where the request goes is left to the retry, since mv would put the staged file inside it. Test 258 drops the case built on the old check and gains two: a staged file a refused write left behind is moved rather than written again, and a request an earlier run left behind is replaced. Both die to a mutation of their own clause. Reds the Win32 leg of #1743, where the log records the second write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The first commit lost the clause that handles a refused write which landed: a restore in my own mutation run wrote to the build directory instead of the worktree, so the mutant went in. Restored, and the matrix re-run from a known state. Three findings from the review: The staged file now carries a marker and the case asserts the request carries it too. Without that, a helper that deleted the staging file and wrote the request again passed the case named for refusing exactly that. The stale-request case makes its leftover read-only. It passed against master's helper before, so it pinned behaviour master already had. The cleanup after the retry loop is gone. Every path out of that loop calls fail, so no mutation of it turns any test red. Also drop the header comment of lock_request_landed, which this branch deletes, and cut two comment blocks that repeated the body of the change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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
1 participant
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.
On WSL2's drvfs a write can be refused and land all the same. For that case #1694 made
write_lock_requestask, after a refused write, whether the request is there and newer thanhts-in_progress.lock. That question loses a race. The engine polls for the request and deletes it, so the check sees nothing and writes a second copy. Test 459 then findshts-stop.lockstill in place after the engine paused, which is what reds the Win32 leg of #1743.The request is now written as
hts-stop.lock.stagedand moved onto its real name. The staging file going away is what says the move happened, and the engine never reads that name, so nothing can take it first.Test 258 loses the case built on the old check and gains two. A staged file a refused write left behind is moved rather than written again, and a request an earlier run left behind is replaced.