Fix cross-lock clobber that could reissue an in-flight nonce - #273
Draft
damilolaedwards wants to merge 1 commit into
Draft
Fix cross-lock clobber that could reissue an in-flight nonce#273damilolaedwards wants to merge 1 commit into
damilolaedwards wants to merge 1 commit into
Conversation
GetNextNonce advances pendingTxCount with an atomic Add under nonceMutex. The confirmation path advances the same counter under a different lock (txNonceMutex) with a plain Load-then-Store, which is not atomic as a whole: a Store computed from a stale Load could clobber a concurrent Add and roll the counter backwards, handing a later caller a nonce that was already allocated to an in-flight transaction. Only one of the two ever lands on chain, and the tool's own record of what it sent versus confirmed goes quietly out of sync. Replaced the Load-then-Store with a CompareAndSwap retry loop so the update only ever lands against the value it was actually computed from, extracted into its own method since it is one half of a race with GetNextNonce in the same file. This bug's trigger window sits between two adjacent atomic operations with no blocking call in between, which turns out to be too narrow for a goroutine stress test to reproduce reliably even with the bug still present (confirmed while writing the tests). The fix is verified with direct, deterministic checks of the update logic instead, which is what CompareAndSwap's own atomicity guarantee makes sufficient here, plus a general concurrent-access smoke test under the race detector.
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.
Summary
Test plan
-racefor general memory safety.