eth: move chainSync.forced state type to atomic instead of bool

@ziogaschr found a(nother) race condition in the tests, which I have had a hard time dupliating with env GOMAXPROCS=1 go test -count 1 -race ./eth But this should, hopefully, fix the issue. Date: 2023-02-07 08:47:44-08:00 Signed-off-by: meows <[email protected]>
etclabscore · Feb 7, 2023 · ec625f6 · ec625f6
1 parent 51da9a4
commit ec625f6
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 9 deletions.
diff --git a/eth/sync.go b/eth/sync.go
@@ -107,7 +107,7 @@ func (h *handler) syncTransactions(p *eth.Peer) {
 type chainSyncer struct {
 	handler     *handler
 	force       *time.Timer
-	forced      bool // true when force timer fired
+	forced      uint32 // true when force timer fired
 	warned      time.Time
 	peerEventCh chan struct{}
 	doneCh      chan error // non-nil when sync is running
@@ -166,7 +166,7 @@ func (cs *chainSyncer) loop() {
 		case err := <-cs.doneCh:
 			cs.doneCh = nil
 			cs.force.Reset(forceSyncCycle)
-			cs.forced = false
+			atomic.StoreUint32(&cs.forced, 0)
 
 			// If we've reached the merge transition but no beacon client is available, or
 			// it has not yet switched us over, keep warning the user that their infra is
@@ -176,7 +176,7 @@ func (cs *chainSyncer) loop() {
 				cs.warned = time.Now()
 			}
 		case <-cs.force.C:
-			cs.forced = true
+			atomic.StoreUint32(&cs.forced, 1)
 
 		case <-cs.handler.quitSync:
 			// Disable all insertion on the blockchain. This needs to happen before
@@ -210,7 +210,7 @@ func (cs *chainSyncer) nextSyncOp() *chainSyncOp {
 	}
 	// Ensure we're at minimum peer count.
 	minPeers := defaultMinSyncPeers
-	if cs.forced {
+	if atomic.LoadUint32(&cs.forced) == 1 {
 		minPeers = 1
 	} else if minPeers > cs.handler.maxPeers {
 		minPeers = cs.handler.maxPeers

diff --git a/eth/sync_test.go b/eth/sync_test.go
@@ -219,7 +219,8 @@ func TestArtificialFinalityFeatureEnablingDisabling_AbleDisable(t *testing.T) {
 		t.Fatalf("sync failed: %v", err)
 	}
 
-	b.handler.chainSync.forced = true
+	atomic.StoreUint32(&b.handler.chainSync.forced, 1)
+
 	next := b.handler.chainSync.nextSyncOp()
 	if next != nil {
 		t.Fatal("non-nil next sync op")
@@ -235,7 +236,7 @@ func TestArtificialFinalityFeatureEnablingDisabling_AbleDisable(t *testing.T) {
 	minArtificialFinalityPeers = oMinAFPeers
 
 	// Next sync op will unset AF because manager only has 1 peer.
-	b.handler.chainSync.forced = true
+	atomic.StoreUint32(&b.handler.chainSync.forced, 1)
 	next = b.handler.chainSync.nextSyncOp()
 	if next != nil {
 		t.Fatal("non-nil next sync op")
@@ -306,7 +307,7 @@ func TestArtificialFinalityFeatureEnablingDisabling_NoDisable(t *testing.T) {
 		t.Fatalf("sync failed: %v", err)
 	}
 
-	b.handler.chainSync.forced = true
+	atomic.StoreUint32(&b.handler.chainSync.forced, 1)
 	next := b.handler.chainSync.nextSyncOp()
 
 	// Revert safety condition overrides to default values.
@@ -324,7 +325,7 @@ func TestArtificialFinalityFeatureEnablingDisabling_NoDisable(t *testing.T) {
 	}
 
 	// Next sync op will unset AF because manager only has 1 peer.
-	b.handler.chainSync.forced = true
+	atomic.StoreUint32(&b.handler.chainSync.forced, 1)
 	next = b.handler.chainSync.nextSyncOp()
 	if next != nil {
 		t.Fatal("non-nil next sync op")
@@ -393,7 +394,7 @@ func TestArtificialFinalityFeatureEnablingDisabling_StaleHead(t *testing.T) {
 		t.Fatalf("sync failed: %v", err)
 	}
 
-	b.handler.chainSync.forced = true
+	atomic.StoreUint32(&b.handler.chainSync.forced, 1)
 	next := b.handler.chainSync.nextSyncOp()
 
 	// Revert safety condition overrides to default values.