fix: do not hold cs_map_quorums for too long (#5370)

## Issue being fixed or feature implemented
`cs_map_quorums` was introduced to protect `mapQuorumsCache` only. We
shouldn't hold it for too long or require it to be held in
`BuildQuorumFromCommitment`.

## What was done?
limit the scope of `cs_map_quorums`

## How Has This Been Tested?
build and run tests locally and in gitlab ci

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
This commit is contained in:
UdjinM6 2023-05-12 04:20:33 +03:00 committed by GitHub
parent 490f32c8d1
commit bfccd1e732
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 5 additions and 7 deletions

View File

@ -363,7 +363,6 @@ void CQuorumManager::CheckQuorumConnections(const Consensus::LLMQParams& llmqPar
CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const
{ {
AssertLockHeld(cs_map_quorums);
assert(pQuorumBaseBlockIndex); assert(pQuorumBaseBlockIndex);
const uint256& quorumHash{pQuorumBaseBlockIndex->GetBlockHash()}; const uint256& quorumHash{pQuorumBaseBlockIndex->GetBlockHash()};
@ -400,7 +399,8 @@ CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType l
// sessions if the shares would be calculated on-demand // sessions if the shares would be calculated on-demand
StartCachePopulatorThread(quorum); StartCachePopulatorThread(quorum);
} }
mapQuorumsCache[llmqType].insert(quorumHash, quorum);
WITH_LOCK(cs_map_quorums, mapQuorumsCache[llmqType].insert(quorumHash, quorum));
return quorum; return quorum;
} }
@ -582,9 +582,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const CBlock
return nullptr; return nullptr;
} }
LOCK(cs_map_quorums);
CQuorumPtr pQuorum; CQuorumPtr pQuorum;
if (mapQuorumsCache[llmqType].get(quorumHash, pQuorum)) { if (LOCK(cs_map_quorums); mapQuorumsCache[llmqType].get(quorumHash, pQuorum)) {
return pQuorum; return pQuorum;
} }
@ -756,8 +755,7 @@ void CQuorumManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, C
CQuorumPtr pQuorum; CQuorumPtr pQuorum;
{ {
LOCK(cs_map_quorums); if (LOCK(cs_map_quorums); !mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash(), pQuorum)) {
if (!mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash(), pQuorum)) {
errorHandler("Quorum not found", 0); // Don't bump score because we asked for it errorHandler("Quorum not found", 0); // Don't bump score because we asked for it
return; return;
} }

View File

@ -259,7 +259,7 @@ private:
// all private methods here are cs_main-free // all private methods here are cs_main-free
void CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const; void CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const;
CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const EXCLUSIVE_LOCKS_REQUIRED(cs_map_quorums); CQuorumPtr BuildQuorumFromCommitment(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex) const;
bool BuildQuorumContributions(const CFinalCommitmentPtr& fqc, const std::shared_ptr<CQuorum>& quorum) const; bool BuildQuorumContributions(const CFinalCommitmentPtr& fqc, const std::shared_ptr<CQuorum>& quorum) const;
CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindex) const; CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindex) const;