eth/protocols/eth: prevent hanging dispatch (ethereum#30918)

holiman · rjl493456442 · web-flow · commit 92980746339d · 2025-01-06T16:31:53.000+01:00
This PR attempts to fix a strange test-failure (timeout) observed on a windows-32 platform. https://ci.appveyor.com/project/ethereum/go-ethereum/builds/51174391/job/d8ascanwwltrlqd5 A goroutine is stuck trying to deliver a response: ``` goroutine 9632 [select, 29 minutes]: github.com/ethereum/go-ethereum/eth/protocols/eth.(*Peer).dispatchResponse(0x314f100, 0x3e5f6d0, 0x3acbb84) C:/projects/go-ethereum/eth/protocols/eth/dispatcher.go:172 +0x2a5 github.com/ethereum/go-ethereum/eth/protocols/eth.handleBlockHeaders({0x12abe68, 0x30021b8}, {0x12a815c, 0x40b41c0}, 0x314f100) C:/projects/go-ethereum/eth/protocols/eth/handlers.go:301 +0x173 github.com/ethereum/go-ethereum/eth/protocols/eth.handleMessage({0x12abe68, 0x30021b8}, 0x314f100) C:/projects/go-ethereum/eth/protocols/eth/handler.go:205 +0x4f6 github.com/ethereum/go-ethereum/eth/protocols/eth.Handle({0x12abe68, 0x30021b8}, 0x314f100) C:/projects/go-ethereum/eth/protocols/eth/handler.go:149 +0x33 github.com/ethereum/go-ethereum/eth.testSnapSyncDisabling.func1(0x314f100) C:/projects/go-ethereum/eth/sync_test.go:65 +0x33 github.com/ethereum/go-ethereum/eth.(*handler).runEthPeer(0x30021b8, 0x314f100, 0x427f648) C:/projects/go-ethereum/eth/handler.go:355 +0xe65 created by github.com/ethereum/go-ethereum/eth.testSnapSyncDisabling in goroutine 11 C:/projects/go-ethereum/eth/sync_test.go:64 +0x54f FAIL github.com/ethereum/go-ethereum/eth 1800.138s ``` --------- Co-authored-by: Gary Rong <garyrong0905@gmail.com>
diff --git a/eth/protocols/eth/dispatcher.go b/eth/protocols/eth/dispatcher.go
@@ -174,6 +174,8 @@ func (p *Peer) dispatchResponse(res *Response, metadata func() interface{}) erro
 				return <-res.Done // Response delivered, return any errors
 			case <-res.Req.cancel:
 				return nil // Request cancelled, silently discard response
+			case <-p.term:
+				return errDisconnected
 			}
 		}
 
diff --git a/eth/sync_test.go b/eth/sync_test.go
@@ -88,7 +88,7 @@ func testSnapSyncDisabling(t *testing.T, ethVer uint, snapVer uint) {
 	if err := empty.handler.downloader.BeaconSync(ethconfig.SnapSync, full.chain.CurrentBlock(), nil); err != nil {
 		t.Fatal("sync failed:", err)
 	}
-	empty.handler.enableSyncedFeatures()
+	time.Sleep(time.Second * 5) // Downloader internally has to wait a timer (3s) to be expired before exiting
 
 	if empty.handler.snapSync.Load() {
 		t.Fatalf("snap sync not disabled after successful synchronisation")

Original file line number	Diff line number	Diff line change
`@@ -174,6 +174,8 @@ func (p Peer) dispatchResponse(res Response, metadata func() interface{}) erro`
`174`	`174`	`return <-res.Done // Response delivered, return any errors`
`175`	`175`	`case <-res.Req.cancel:`
`176`	`176`	`return nil // Request cancelled, silently discard response`
	`177`	`+ case <-p.term:`
	`178`	`+ return errDisconnected`
`177`	`179`	`}`
`178`	`180`	`}`
`179`	`181`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ func testSnapSyncDisabling(t *testing.T, ethVer uint, snapVer uint) {`
`88`	`88`	`if err := empty.handler.downloader.BeaconSync(ethconfig.SnapSync, full.chain.CurrentBlock(), nil); err != nil {`
`89`	`89`	`t.Fatal("sync failed:", err)`
`90`	`90`	`}`
`91`		`- empty.handler.enableSyncedFeatures()`
	`91`	`+ time.Sleep(time.Second * 5) // Downloader internally has to wait a timer (3s) to be expired before exiting`
`92`	`92`
`93`	`93`	`if empty.handler.snapSync.Load() {`
`94`	`94`	`t.Fatalf("snap sync not disabled after successful synchronisation")`