eth/protocols/snap: fix data race on testPeer counters (#34802)

The testPeer request counters (nAccountRequests, nStorageRequests,
nBytecodeRequests, nTrienodeRequests) were plain int fields incremented
with ++. These increments happen in Request* methods that are invoked
concurrently by the Syncer from multiple goroutines
(assignBytecodeTasks, assignStorageTasks, etc.), causing a data race
reliably detected by go test -race.

Change the counters to atomic.Int64 so increments and reads are
synchronized without introducing a mutex.

Fixes races detected in TestMultiSyncManyUseless,
TestMultiSyncManyUselessWithLowTimeout,
TestMultiSyncManyUnresponsive, TestSyncWithStorageAndOneCappedPeer,
TestSyncWithStorageAndCorruptPeer, and
TestSyncWithStorageAndNonProvingPeer.
This commit is contained in:
rayoo 2026-04-24 19:37:34 +08:00 committed by MariusVanDerWijden
parent a86eaceda0
commit a262af0430

View file

@ -25,6 +25,7 @@ import (
mrand "math/rand" mrand "math/rand"
"slices" "slices"
"sync" "sync"
"sync/atomic"
"testing" "testing"
"time" "time"
@ -142,10 +143,10 @@ type testPeer struct {
term func() term func()
// counters // counters
nAccountRequests int nAccountRequests atomic.Int64
nStorageRequests int nStorageRequests atomic.Int64
nBytecodeRequests int nBytecodeRequests atomic.Int64
nTrienodeRequests int nTrienodeRequests atomic.Int64
} }
func newTestPeer(id string, t *testing.T, term func()) *testPeer { func newTestPeer(id string, t *testing.T, term func()) *testPeer {
@ -179,25 +180,25 @@ func (t *testPeer) Stats() string {
Storage requests: %d Storage requests: %d
Bytecode requests: %d Bytecode requests: %d
Trienode requests: %d Trienode requests: %d
`, t.nAccountRequests, t.nStorageRequests, t.nBytecodeRequests, t.nTrienodeRequests) `, t.nAccountRequests.Load(), t.nStorageRequests.Load(), t.nBytecodeRequests.Load(), t.nTrienodeRequests.Load())
} }
func (t *testPeer) RequestAccountRange(id uint64, root, origin, limit common.Hash, bytes int) error { func (t *testPeer) RequestAccountRange(id uint64, root, origin, limit common.Hash, bytes int) error {
t.logger.Trace("Fetching range of accounts", "reqid", id, "root", root, "origin", origin, "limit", limit, "bytes", common.StorageSize(bytes)) t.logger.Trace("Fetching range of accounts", "reqid", id, "root", root, "origin", origin, "limit", limit, "bytes", common.StorageSize(bytes))
t.nAccountRequests++ t.nAccountRequests.Add(1)
go t.accountRequestHandler(t, id, root, origin, limit, bytes) go t.accountRequestHandler(t, id, root, origin, limit, bytes)
return nil return nil
} }
func (t *testPeer) RequestTrieNodes(id uint64, root common.Hash, count int, paths []TrieNodePathSet, bytes int) error { func (t *testPeer) RequestTrieNodes(id uint64, root common.Hash, count int, paths []TrieNodePathSet, bytes int) error {
t.logger.Trace("Fetching set of trie nodes", "reqid", id, "root", root, "pathsets", len(paths), "bytes", common.StorageSize(bytes)) t.logger.Trace("Fetching set of trie nodes", "reqid", id, "root", root, "pathsets", len(paths), "bytes", common.StorageSize(bytes))
t.nTrienodeRequests++ t.nTrienodeRequests.Add(1)
go t.trieRequestHandler(t, id, root, paths, bytes) go t.trieRequestHandler(t, id, root, paths, bytes)
return nil return nil
} }
func (t *testPeer) RequestStorageRanges(id uint64, root common.Hash, accounts []common.Hash, origin, limit []byte, bytes int) error { func (t *testPeer) RequestStorageRanges(id uint64, root common.Hash, accounts []common.Hash, origin, limit []byte, bytes int) error {
t.nStorageRequests++ t.nStorageRequests.Add(1)
if len(accounts) == 1 && origin != nil { if len(accounts) == 1 && origin != nil {
t.logger.Trace("Fetching range of large storage slots", "reqid", id, "root", root, "account", accounts[0], "origin", common.BytesToHash(origin), "limit", common.BytesToHash(limit), "bytes", common.StorageSize(bytes)) t.logger.Trace("Fetching range of large storage slots", "reqid", id, "root", root, "account", accounts[0], "origin", common.BytesToHash(origin), "limit", common.BytesToHash(limit), "bytes", common.StorageSize(bytes))
} else { } else {
@ -208,7 +209,7 @@ func (t *testPeer) RequestStorageRanges(id uint64, root common.Hash, accounts []
} }
func (t *testPeer) RequestByteCodes(id uint64, hashes []common.Hash, bytes int) error { func (t *testPeer) RequestByteCodes(id uint64, hashes []common.Hash, bytes int) error {
t.nBytecodeRequests++ t.nBytecodeRequests.Add(1)
t.logger.Trace("Fetching set of byte codes", "reqid", id, "hashes", len(hashes), "bytes", common.StorageSize(bytes)) t.logger.Trace("Fetching set of byte codes", "reqid", id, "hashes", len(hashes), "bytes", common.StorageSize(bytes))
go t.codeRequestHandler(t, id, hashes, bytes) go t.codeRequestHandler(t, id, hashes, bytes)
return nil return nil
@ -1901,7 +1902,7 @@ func testSyncAccountPerformance(t *testing.T, scheme string) {
// sync cycle starts. When popping the queue, we do not look it up again. // sync cycle starts. When popping the queue, we do not look it up again.
// Doing so would bring this number down to zero in this artificial testcase, // Doing so would bring this number down to zero in this artificial testcase,
// but only add extra IO for no reason in practice. // but only add extra IO for no reason in practice.
if have, want := src.nTrienodeRequests, 1; have != want { if have, want := src.nTrienodeRequests.Load(), int64(1); have != want {
fmt.Print(src.Stats()) fmt.Print(src.Stats())
t.Errorf("trie node heal requests wrong, want %d, have %d", want, have) t.Errorf("trie node heal requests wrong, want %d, have %d", want, have)
} }