fix(#603): store appendVersion error path — no poisoned files entry on failed path dupe#604
Merged
Merged
Conversation
… files entry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n the path dupe fails Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Benchmark Regression ReportThresholds: 10.00% and 50,000 ns absolute delta
|
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.
What
Fixes #603 (note: merging into the release branch will not auto-close it — add it to the ship closes-list).
appendVersion's first-touch block inserted the map slot viagetOrPutand only then duped the path. A failed dupe (OOM) left a poisoned entry behind: key = the caller's transient slice, value = undefinedFileVersions. A retry appended into undefined memory (UB),deinitinvalid-freed the borrowed key, and once the caller freed its buffer the map held a dangling key. Same error-path class as #594, in the store.The fix scopes an
errdefer self.files.removeByPtr(entry.key_ptr)to the first-touch block, so the dupe failing rolls the insert back.FileVersions.initis infallible, so the errdefer guards exactly the dupe; a laterversions.appendfailure still leaves a valid initialized entry, which is fine.Test plan
test(#603)commit: failing test (expected 0, found 1+ leak) on the release tipzig build test— 23/23 steps, 808/808 tests passed🤖 Generated with Claude Code