From a506cf47add829f72224531f173526e84af4826b Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 5 Apr 2018 11:42:45 +0200 Subject: [PATCH] protect Record* peerStateStats functions by mutex Fixes #1414 DATA RACE: ``` Read at 0x00c4214ee940 by goroutine 146: github.com/tendermint/tendermint/consensus.(*peerStateStats).String() :1 +0x57 fmt.(*pp).handleMethods() /usr/local/go/src/fmt/print.go:596 +0x3f4 fmt.(*pp).printArg() /usr/local/go/src/fmt/print.go:679 +0x11f fmt.(*pp).doPrintf() /usr/local/go/src/fmt/print.go:996 +0x319 fmt.Sprintf() /usr/local/go/src/fmt/print.go:196 +0x73 github.com/tendermint/tendermint/consensus.(*PeerState).StringIndented() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:1426 +0x573 github.com/tendermint/tendermint/consensus.(*PeerState).String() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:1419 +0x66 github.com/go-logfmt/logfmt.safeString() /home/ubuntu/go/src/github.com/go-logfmt/logfmt/encode.go:299 +0x9d github.com/go-logfmt/logfmt.writeValue() /home/ubuntu/go/src/github.com/go-logfmt/logfmt/encode.go:217 +0x5a0 github.com/go-logfmt/logfmt.(*Encoder).EncodeKeyval() /home/ubuntu/go/src/github.com/go-logfmt/logfmt/encode.go:61 +0x1dd github.com/tendermint/tmlibs/log.tmfmtLogger.Log() /home/ubuntu/go/src/github.com/tendermint/tmlibs/log/tmfmt_logger.go:107 +0x1001 github.com/tendermint/tmlibs/log.(*tmfmtLogger).Log() :1 +0x93 github.com/go-kit/kit/log.(*context).Log() /home/ubuntu/go/src/github.com/go-kit/kit/log/log.go:124 +0x248 github.com/tendermint/tmlibs/log.(*tmLogger).Debug() /home/ubuntu/go/src/github.com/tendermint/tmlibs/log/tm_logger.go:64 +0x1d0 github.com/tendermint/tendermint/consensus.(*PeerState).PickSendVote() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:1059 +0x242 github.com/tendermint/tendermint/consensus.(*ConsensusReactor).gossipVotesForHeight() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:789 +0x6ef github.com/tendermint/tendermint/consensus.(*ConsensusReactor).gossipVotesRoutine() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:723 +0x1039 Previous write at 0x00c4214ee940 by goroutine 21: github.com/tendermint/tendermint/consensus.(*PeerState).RecordVote() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:1242 +0x15a github.com/tendermint/tendermint/consensus.(*ConsensusReactor).Receive() github.com/tendermint/tendermint/consensus/_test/_obj_test/reactor.go:309 +0x32e6 github.com/tendermint/tendermint/p2p.createMConnection.func1() /home/ubuntu/go/src/github.com/tendermint/tendermint/p2p/peer.go:365 +0xea github.com/tendermint/tendermint/p2p/conn.(*MConnection).recvRoutine() /home/ubuntu/go/src/github.com/tendermint/tendermint/p2p/conn/connection.go:531 +0x779 ``` --- consensus/reactor.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/consensus/reactor.go b/consensus/reactor.go index c71f9e0c4..70a79d86a 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1087,6 +1087,9 @@ func (ps *PeerState) ensureVoteBitArrays(height int64, numValidators int) { // It returns the total number of votes (1 per block). This essentially means // the number of blocks for which peer has been sending us votes. func (ps *PeerState) RecordVote(vote *types.Vote) int { + ps.mtx.Lock() + defer ps.mtx.Unlock() + if ps.stats.lastVoteHeight >= vote.Height { return ps.stats.votes } @@ -1098,13 +1101,20 @@ func (ps *PeerState) RecordVote(vote *types.Vote) int { // VotesSent returns the number of blocks for which peer has been sending us // votes. func (ps *PeerState) VotesSent() int { + ps.mtx.Lock() + defer ps.mtx.Unlock() + return ps.stats.votes } -// RecordVote updates internal statistics for this peer by recording the block part. -// It returns the total number of block parts (1 per block). This essentially means -// the number of blocks for which peer has been sending us block parts. +// RecordBlockPart updates internal statistics for this peer by recording the +// block part. It returns the total number of block parts (1 per block). This +// essentially means the number of blocks for which peer has been sending us +// block parts. func (ps *PeerState) RecordBlockPart(bp *BlockPartMessage) int { + ps.mtx.Lock() + defer ps.mtx.Unlock() + if ps.stats.lastBlockPartHeight >= bp.Height { return ps.stats.blockParts } @@ -1117,6 +1127,9 @@ func (ps *PeerState) RecordBlockPart(bp *BlockPartMessage) int { // BlockPartsSent returns the number of blocks for which peer has been sending // us block parts. func (ps *PeerState) BlockPartsSent() int { + ps.mtx.Lock() + defer ps.mtx.Unlock() + return ps.stats.blockParts }