polygon/sync: fix race due to header copy #13480
Merged
Conversation
This file contains 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
shohamc1
approved these changes
Jan 17, 2025
taratorio
added a commit
that referenced
this pull request
Jan 18, 2025
relates to #13480 fully fixes #13474 Seems like `Header.hash` memoization changes from a few months ago have made `Block.Header()` (which makes a copy by calling `CopyHeader`) show race warnings. This is because we are copying over `atomic.Pointer` in `CopyHeader`. This PR fixes this by manually copy-ing all fields and skipping copy of the `hash atomic.Pointer` attribute (instead it will get set using its zero value - correct behaviour) and then the hash will be memoized using it as part of the copy later on. This is a bit clumsier as we can miss updating the `CopyHeader` function when adding new attributes to the `Header` struct - to address that I've added a reflection based test which will generate random values for all struct fields and as a result capture this pitfall and remind developers to update. Example race detected: ``` ================== WARNING: DATA RACE Read at 0x00c03acc0838 by goroutine 26498: github.com/erigontech/erigon/core/types.CopyHeader() /home/ubuntu/erigon/core/types/block.go:1098 +0x5c github.com/erigontech/erigon/core/types.(*Block).Header() /home/ubuntu/erigon/core/types/block.go:1278 +0x54 github.com/erigontech/erigon/turbo/execution/eth1/eth1_utils.ConvertBlockToRPC() /home/ubuntu/erigon/turbo/execution/eth1/eth1_utils/grpc.go:108 +0x55 github.com/erigontech/erigon/turbo/execution/eth1/eth1_utils.ConvertBlocksToRPC() /home/ubuntu/erigon/turbo/execution/eth1/eth1_utils/grpc.go:102 +0xc4 github.com/erigontech/erigon/polygon/sync.(*executionClient).InsertBlocks() /home/ubuntu/erigon/polygon/sync/execution_client.go:71 +0x4f github.com/erigontech/erigon/polygon/sync.(*ExecutionClientStore).insertBlocks() /home/ubuntu/erigon/polygon/sync/store.go:162 +0x14f github.com/erigontech/erigon/polygon/sync.(*ExecutionClientStore).Run() /home/ubuntu/erigon/polygon/sync/store.go:143 +0x1ab github.com/erigontech/erigon/polygon/sync.(*Service).Run.func2() /home/ubuntu/erigon/polygon/sync/service.go:125 +0x69 golang.org/x/sync/errgroup.(*Group).Go.func1() ================== ```
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 #13474