From fa60d8120ecfd02b2027b0cd053389d5434d69fd Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 14 Nov 2017 17:41:30 -0600 Subject: [PATCH 1/2] fix TestFullRound1 race (Refs #846) ``` ================== WARNING: DATA RACE Write at 0x00c42d7605f0 by goroutine 844: github.com/tendermint/tendermint/consensus.(*ConsensusState).updateToState() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:465 +0x59e I[11-14|22:37:28.781] Added to prevote vote="Vote{0:646753DCE124 1/02/1(Prevote) E9B19636DCDB {/CAD5FA805E8C.../}}" prevotes="VoteSet{H:1 R:2 T:1 +2/3: BA{2:X_} map[]}" github.com/tendermint/tendermint/consensus.(*ConsensusState).finalizeCommit() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1229 +0x16a9 github.com/tendermint/tendermint/consensus.(*ConsensusState).tryFinalizeCommit() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1135 +0x721 github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit.func1() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1087 +0x153 github.com/tendermint/tendermint/consensus.(*ConsensusState).enterCommit() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1114 +0xa34 github.com/tendermint/tendermint/consensus.(*ConsensusState).addVote() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1423 +0xdd6 github.com/tendermint/tendermint/consensus.(*ConsensusState).tryAddVote() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:1317 +0x77 github.com/tendermint/tendermint/consensus.(*ConsensusState).handleMsg() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:565 +0x7a9 github.com/tendermint/tendermint/consensus.(*ConsensusState).receiveRoutine() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:523 +0x6d2 Previous read at 0x00c42d7605f0 by goroutine 654: github.com/tendermint/tendermint/consensus.validatePrevote() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/common_test.go:149 +0x57 github.com/tendermint/tendermint/consensus.TestFullRound1() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state_test.go:256 +0x3c5 testing.tRunner() /usr/local/go/src/testing/testing.go:746 +0x16c Goroutine 844 (running) created at: github.com/tendermint/tendermint/consensus.(*ConsensusState).startRoutines() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state.go:258 +0x8c github.com/tendermint/tendermint/consensus.startTestRound() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/common_test.go:118 +0x63 github.com/tendermint/tendermint/consensus.TestFullRound1() /home/vagrant/go/src/github.com/tendermint/tendermint/consensus/state_test.go:247 +0x1fb testing.tRunner() /usr/local/go/src/testing/testing.go:746 +0x16c Goroutine 654 (running) created at: testing.(*T).Run() /usr/local/go/src/testing/testing.go:789 +0x568 testing.runTests.func1() /usr/local/go/src/testing/testing.go:1004 +0xa7 testing.tRunner() /usr/local/go/src/testing/testing.go:746 +0x16c testing.runTests() /usr/local/go/src/testing/testing.go:1002 +0x521 testing.(*M).Run() /usr/local/go/src/testing/testing.go:921 +0x206 main.main() github.com/tendermint/tendermint/consensus/_test/_testmain.go:106 +0x1d3 ================== ``` --- consensus/state_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/consensus/state_test.go b/consensus/state_test.go index 290eb026..49ec1185 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -10,6 +10,7 @@ import ( cstypes "github.com/tendermint/tendermint/consensus/types" "github.com/tendermint/tendermint/types" cmn "github.com/tendermint/tmlibs/common" + "github.com/tendermint/tmlibs/log" tmpubsub "github.com/tendermint/tmlibs/pubsub" ) @@ -240,6 +241,14 @@ func TestFullRound1(t *testing.T) { cs, vss := randConsensusState(1) height, round := cs.Height, cs.Round + // NOTE: buffer capacity of 0 ensures we can validate prevote and last commit + // before consensus can move to the next height (and cause a race condition) + cs.eventBus.Stop() + eventBus := types.NewEventBusWithBufferCapacity(0) + eventBus.SetLogger(log.TestingLogger().With("module", "events")) + cs.SetEventBus(eventBus) + eventBus.Start() + voteCh := subscribe(cs.eventBus, types.EventQueryVote) propCh := subscribe(cs.eventBus, types.EventQueryCompleteProposal) newRoundCh := subscribe(cs.eventBus, types.EventQueryNewRound) From fe3c92ecce1a4811eb7d20e3c22156ef362d60c8 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 14 Nov 2017 20:56:39 -0600 Subject: [PATCH 2/2] unescape $NODE_FLAGS (see comment) --- test/p2p/peer.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/p2p/peer.sh b/test/p2p/peer.sh index 3b8322b6..d5ede231 100644 --- a/test/p2p/peer.sh +++ b/test/p2p/peer.sh @@ -14,6 +14,8 @@ set +eu echo "starting tendermint peer ID=$ID" # start tendermint container on the network +# NOTE: $NODE_FLAGS should be unescaped (no quotes). otherwise it will be +# treated as one flag. set -u docker run -d \ --net="$NETWORK_NAME" \ @@ -25,4 +27,4 @@ docker run -d \ --log-opt syslog-address=udp://127.0.0.1:5514 \ --log-opt syslog-facility=daemon \ --log-opt tag="{{.Name}}" \ - "$DOCKER_IMAGE" node "$NODE_FLAGS" --log_level=debug --proxy_app="$APP_PROXY" + "$DOCKER_IMAGE" node $NODE_FLAGS --log_level=debug --proxy_app="$APP_PROXY"