From b9a0d47f1467f1b8b8a589ee0da68374af0ab7bb Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 26 May 2020 13:51:37 +0200 Subject: [PATCH] test/blockchain/v0: mitigate test data race (#4886) Mitigates the below data race. The proper fix involves not fiddling with reactor internals, which needs a rewrite of the test and possible additional reactor infrastructure. ``` ================== WARNING: DATA RACE Write at 0x00c001118e78 by goroutine 187: github.com/tendermint/tendermint/blockchain/v0.TestBadBlockStopsPeer() /go/src/github.com/tendermint/tendermint/blockchain/v0/reactor_test.go:234 +0x9d7 testing.tRunner() /usr/local/go/src/testing/testing.go:992 +0x1eb Previous read at 0x00c001118e78 by goroutine 326: [failed to restore the stack] Goroutine 187 (running) created at: testing.(*T).Run() /usr/local/go/src/testing/testing.go:1043 +0x660 testing.runTests.func1() /usr/local/go/src/testing/testing.go:1285 +0xa6 testing.tRunner() /usr/local/go/src/testing/testing.go:992 +0x1eb testing.runTests() /usr/local/go/src/testing/testing.go:1283 +0x527 testing.(*M).Run() /usr/local/go/src/testing/testing.go:1200 +0x2ff main.main() _testmain.go:112 +0x337 Goroutine 326 (running) created at: github.com/tendermint/tendermint/blockchain/v0.(*BlockchainReactor).OnStart() /go/src/github.com/tendermint/tendermint/blockchain/v0/reactor.go:118 +0x12c github.com/tendermint/tendermint/libs/service.(*BaseService).Start() /go/src/github.com/tendermint/tendermint/libs/service/service.go:140 +0x504 github.com/tendermint/tendermint/blockchain/v0.(*BlockchainReactor).Start() :1 +0x43 github.com/tendermint/tendermint/p2p.(*Switch).OnStart() /go/src/github.com/tendermint/tendermint/p2p/switch.go:225 +0x120 github.com/tendermint/tendermint/libs/service.(*BaseService).Start() /go/src/github.com/tendermint/tendermint/libs/service/service.go:140 +0x504 github.com/tendermint/tendermint/p2p.StartSwitches() /go/src/github.com/tendermint/tendermint/p2p/test_util.go:168 +0x75 github.com/tendermint/tendermint/p2p.MakeConnectedSwitches() /go/src/github.com/tendermint/tendermint/p2p/test_util.go:89 +0x17d github.com/tendermint/tendermint/blockchain/v0.TestBadBlockStopsPeer() /go/src/github.com/tendermint/tendermint/blockchain/v0/reactor_test.go:209 +0x768 testing.tRunner() /usr/local/go/src/testing/testing.go:992 +0x1eb ================== panic: BlockStore can only save contiguous blocks. Wanted 149, got 147 goroutine 1259 [running]: github.com/tendermint/tendermint/store.(*BlockStore).SaveBlock(0xc000ff9cc0, 0xc001997180, 0xc0010c6a00, 0xc0013b3000) /go/src/github.com/tendermint/tendermint/store/store.go:276 +0xbc4 github.com/tendermint/tendermint/blockchain/v0.(*BlockchainReactor).poolRoutine(0xc001118d00, 0x107c000) /go/src/github.com/tendermint/tendermint/blockchain/v0/reactor.go:355 +0xe90 created by github.com/tendermint/tendermint/blockchain/v0.(*BlockchainReactor).OnStart /go/src/github.com/tendermint/tendermint/blockchain/v0/reactor.go:118 +0x12d FAIL github.com/tendermint/tendermint/blockchain/v0 11.447s FAIL ``` --- blockchain/v0/reactor_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/blockchain/v0/reactor_test.go b/blockchain/v0/reactor_test.go index 4d1a6dabd..5acf01431 100644 --- a/blockchain/v0/reactor_test.go +++ b/blockchain/v0/reactor_test.go @@ -220,17 +220,23 @@ func TestBadBlockStopsPeer(t *testing.T) { }() for { - if reactorPairs[3].reactor.pool.IsCaughtUp() { + time.Sleep(1 * time.Second) + caughtUp := true + for _, r := range reactorPairs { + if !r.reactor.pool.IsCaughtUp() { + caughtUp = false + } + } + if caughtUp { break } - - time.Sleep(1 * time.Second) } //at this time, reactors[0-3] is the newest assert.Equal(t, 3, reactorPairs[1].reactor.Switch.Peers().Size()) - //mark reactorPairs[3] is an invalid peer + // Mark reactorPairs[3] as an invalid peer. Fiddling with .store without a mutex is a data + // race, but can't be easily avoided. reactorPairs[3].reactor.store = otherChain.reactor.store lastReactorPair := newBlockchainReactor(log.TestingLogger(), genDoc, privVals, 0)