eth/downloader: deflake TestDeliverHeadersHang

This commit is contained in:
Felix Lange 2018-10-21 23:39:15 +02:00
parent d6300d114d
commit 029b9c5055

View file

@ -20,7 +20,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"math/big" "math/big"
"runtime"
"sync" "sync"
"sync/atomic" "sync/atomic"
"testing" "testing"
@ -1385,9 +1384,9 @@ func testFakedSyncProgress(t *testing.T, protocol int, mode SyncMode) {
// This test reproduces an issue where unexpected deliveries would // This test reproduces an issue where unexpected deliveries would
// block indefinitely if they arrived at the right time. // block indefinitely if they arrived at the right time.
// We use data driven subtests to manage this so that it will be parallel on its own
// and not with the other tests, avoiding intermittent failures.
func TestDeliverHeadersHang(t *testing.T) { func TestDeliverHeadersHang(t *testing.T) {
t.Parallel()
testCases := []struct { testCases := []struct {
protocol int protocol int
syncMode SyncMode syncMode SyncMode
@ -1401,14 +1400,13 @@ func TestDeliverHeadersHang(t *testing.T) {
} }
for _, tc := range testCases { for _, tc := range testCases {
t.Run(fmt.Sprintf("protocol %d mode %v", tc.protocol, tc.syncMode), func(t *testing.T) { t.Run(fmt.Sprintf("protocol %d mode %v", tc.protocol, tc.syncMode), func(t *testing.T) {
t.Parallel()
testDeliverHeadersHang(t, tc.protocol, tc.syncMode) testDeliverHeadersHang(t, tc.protocol, tc.syncMode)
}) })
} }
} }
func testDeliverHeadersHang(t *testing.T, protocol int, mode SyncMode) { func testDeliverHeadersHang(t *testing.T, protocol int, mode SyncMode) {
t.Parallel()
master := newTester() master := newTester()
defer master.terminate() defer master.terminate()
chain := testChainBase.shorten(15) chain := testChainBase.shorten(15)
@ -1428,16 +1426,12 @@ func testDeliverHeadersHang(t *testing.T, protocol int, mode SyncMode) {
t.Errorf("test %d: sync failed: %v", i, err) t.Errorf("test %d: sync failed: %v", i, err)
} }
tester.terminate() tester.terminate()
// Flush all goroutines to prevent messing with subsequent tests
tester.downloader.peers.peers["peer"].peer.(*floodingTestPeer).pend.Wait()
} }
} }
type floodingTestPeer struct { type floodingTestPeer struct {
peer Peer peer Peer
tester *downloadTester tester *downloadTester
pend sync.WaitGroup
} }
func (ftp *floodingTestPeer) Head() (common.Hash, *big.Int) { return ftp.peer.Head() } func (ftp *floodingTestPeer) Head() (common.Hash, *big.Int) { return ftp.peer.Head() }
@ -1456,25 +1450,29 @@ func (ftp *floodingTestPeer) RequestNodeData(hashes []common.Hash) error {
func (ftp *floodingTestPeer) RequestHeadersByNumber(from uint64, count, skip int, reverse bool) error { func (ftp *floodingTestPeer) RequestHeadersByNumber(from uint64, count, skip int, reverse bool) error {
deliveriesDone := make(chan struct{}, 500) deliveriesDone := make(chan struct{}, 500)
for i := 0; i < cap(deliveriesDone); i++ { for i := 0; i < cap(deliveriesDone)-1; i++ {
peer := fmt.Sprintf("fake-peer%d", i) peer := fmt.Sprintf("fake-peer%d", i)
ftp.pend.Add(1)
go func() { go func() {
ftp.tester.downloader.DeliverHeaders(peer, []*types.Header{{}, {}, {}, {}}) ftp.tester.downloader.DeliverHeaders(peer, []*types.Header{{}, {}, {}, {}})
deliveriesDone <- struct{}{} deliveriesDone <- struct{}{}
ftp.pend.Done()
}() }()
} }
// Deliver the actual requested headers. runtime.Gosched is needed to ensure some of
// the flooding delivery goroutines run before we deliver the real answer.
runtime.Gosched()
ftp.peer.RequestHeadersByNumber(from, count, skip, reverse)
// None of the extra deliveries should block. // None of the extra deliveries should block.
timeout := time.After(60 * time.Second) timeout := time.After(60 * time.Second)
launched := false
for i := 0; i < cap(deliveriesDone); i++ { for i := 0; i < cap(deliveriesDone); i++ {
select { select {
case <-deliveriesDone: case <-deliveriesDone:
if !launched {
// Start delivering the requested headers
// after one of the flooding responses has arrived.
go func() {
ftp.peer.RequestHeadersByNumber(from, count, skip, reverse)
deliveriesDone <- struct{}{}
}()
launched = true
}
case <-timeout: case <-timeout:
panic("blocked") panic("blocked")
} }