From a0a6a638bb1b66edcdfc1ada3d3b9594fa8c8549 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Mon, 3 Jun 2024 16:55:42 +0900 Subject: [PATCH] perf(consensus): Fix some peer messages taking write locks on the cs mtx (#3159) Some clear, read-only spots in receive routine took Write locks instead of read locks. Seems like a bug. This PR fixes it. --- #### PR checklist - [x] Tests written/updated - N/A imo? - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --- .../3159-fix-taking-wlocks-instead-of-rlocks.md | 2 ++ consensus/reactor.go | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 .changelog/unreleased/improvements/3159-fix-taking-wlocks-instead-of-rlocks.md diff --git a/.changelog/unreleased/improvements/3159-fix-taking-wlocks-instead-of-rlocks.md b/.changelog/unreleased/improvements/3159-fix-taking-wlocks-instead-of-rlocks.md new file mode 100644 index 0000000000..093ecc8fbb --- /dev/null +++ b/.changelog/unreleased/improvements/3159-fix-taking-wlocks-instead-of-rlocks.md @@ -0,0 +1,2 @@ +- [`consensus`] Fix some reactor messages taking write locks instead of read locks. + ([\#3159](https://github.com/cometbft/cometbft/issues/3159) diff --git a/consensus/reactor.go b/consensus/reactor.go index c7d0cf563f..47114b8eb8 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -262,9 +262,9 @@ func (conR *Reactor) ReceiveEnvelope(e p2p.Envelope) { case StateChannel: switch msg := msg.(type) { case *NewRoundStepMessage: - conR.conS.mtx.Lock() + conR.conS.mtx.RLock() initialHeight := conR.conS.state.InitialHeight - conR.conS.mtx.Unlock() + conR.conS.mtx.RUnlock() if err = msg.ValidateHeight(initialHeight); err != nil { conR.Logger.Error("Peer sent us invalid msg", "peer", e.Src, "msg", msg, "err", err) conR.Switch.StopPeerForError(e.Src, err) @@ -277,9 +277,9 @@ func (conR *Reactor) ReceiveEnvelope(e p2p.Envelope) { ps.ApplyHasVoteMessage(msg) case *VoteSetMaj23Message: cs := conR.conS - cs.mtx.Lock() + cs.mtx.RLock() height, votes := cs.Height, cs.Votes - cs.mtx.Unlock() + cs.mtx.RUnlock() if height != msg.Height { return } @@ -364,9 +364,9 @@ func (conR *Reactor) ReceiveEnvelope(e p2p.Envelope) { switch msg := msg.(type) { case *VoteSetBitsMessage: cs := conR.conS - cs.mtx.Lock() + cs.mtx.RLock() height, votes := cs.Height, cs.Votes - cs.mtx.Unlock() + cs.mtx.RUnlock() if height == msg.Height { var ourVotes *bits.BitArray