Fix deadlock in CSigSharesManager::SendMessages (#2757)

* Fix deadlock in CSigSharesManager::SendMessages

Locking "cs" at this location caused a (potential) deadlock due to changed
order of cs and cs_vNodes locking. This changes the method to not require
the session object anymore which removes the need for locking.

* Pass size of LLMQ instead of llmqType into CSigSharesInv::Init

This allows use of sizes which are not supported in chainparams.
This commit is contained in:
Alexander Block 2019-03-11 14:31:51 +01:00 committed by UdjinM6
parent 7b24f9b8b3
commit 588eb30b86
2 changed files with 20 additions and 22 deletions

View File

@ -65,10 +65,9 @@ std::string CSigSharesInv::ToString() const
return str;
}
void CSigSharesInv::Init(Consensus::LLMQType _llmqType)
void CSigSharesInv::Init(size_t size)
{
size_t llmqSize = (size_t)(Params().GetConsensus().llmqs.at(_llmqType).size);
inv.resize(llmqSize, false);
inv.resize(size, false);
}
bool CSigSharesInv::IsSet(uint16_t quorumMember) const
@ -83,27 +82,30 @@ void CSigSharesInv::Set(uint16_t quorumMember, bool v)
inv[quorumMember] = v;
}
CSigSharesInv CBatchedSigShares::ToInv(Consensus::LLMQType llmqType) const
std::string CBatchedSigShares::ToInvString() const
{
CSigSharesInv inv;
inv.Init(llmqType);
// we use 400 here no matter what the real size is. We don't really care about that size as we just want to call ToString()
inv.Init(400);
for (size_t i = 0; i < sigShares.size(); i++) {
inv.inv[sigShares[i].first] = true;
}
return inv;
return inv.ToString();
}
template<typename T>
static void InitSession(CSigSharesNodeState::Session& s, const uint256& signHash, T& from)
{
const auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)from.llmqType);
s.llmqType = (Consensus::LLMQType)from.llmqType;
s.quorumHash = from.quorumHash;
s.id = from.id;
s.msgHash = from.msgHash;
s.signHash = signHash;
s.announced.Init((Consensus::LLMQType)from.llmqType);
s.requested.Init((Consensus::LLMQType)from.llmqType);
s.knows.Init((Consensus::LLMQType)from.llmqType);
s.announced.Init((size_t)params.size);
s.requested.Init((size_t)params.size);
s.knows.Init((size_t)params.size);
}
CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromShare(const llmq::CSigShare& sigShare)
@ -443,7 +445,7 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(CNode* pfrom, const CBatc
}
LogPrint("llmq", "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__,
sessionInfo.signHash.ToString(), batchedSigShares.sigShares.size(), sigShares.size(), batchedSigShares.ToInv(sessionInfo.llmqType).ToString(), pfrom->id);
sessionInfo.signHash.ToString(), batchedSigShares.sigShares.size(), sigShares.size(), batchedSigShares.ToInvString(), pfrom->id);
if (sigShares.empty()) {
return true;
@ -871,7 +873,8 @@ void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map<NodeId, std
}
auto& inv = (*invMap)[signHash];
if (inv.inv.empty()) {
inv.Init(session.llmqType);
const auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)session.llmqType);
inv.Init((size_t)params.size);
}
inv.inv[k.second] = true;
@ -982,7 +985,8 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map<NodeId, st
auto& inv = sigSharesToAnnounce[nodeId][signHash];
if (inv.inv.empty()) {
inv.Init((Consensus::LLMQType)sigShare->llmqType);
const auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)sigShare->llmqType);
inv.Init((size_t)params.size);
}
inv.inv[quorumMember] = true;
session.knows.inv[quorumMember] = true;
@ -1101,14 +1105,8 @@ bool CSigSharesManager::SendMessages()
std::vector<CBatchedSigShares> msgs;
for (auto& p : jt->second) {
assert(!p.second.sigShares.empty());
if (LogAcceptCategory("llmq")) {
LOCK(cs);
auto session = nodeStates[pnode->id].GetSessionBySignHash(p.first);
assert(session);
LogPrint("llmq", "CSigSharesManager::SendMessages -- QBSIGSHARES signHash=%s, inv={%s}, node=%d\n",
p.first.ToString(), p.second.ToInv(session->llmqType).ToString(), pnode->id);
}
LogPrint("llmq", "CSigSharesManager::SendMessages -- QBSIGSHARES signHash=%s, inv={%s}, node=%d\n",
p.first.ToString(), p.second.ToInvString(), pnode->id);
if (totalSigsCount + p.second.sigShares.size() > MAX_MSGS_TOTAL_BATCHED_SIGS) {
g_connman->PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, msgs), false);
msgs.clear();

View File

@ -101,7 +101,7 @@ public:
READWRITE(AUTOBITSET(inv, (size_t)invSize));
}
void Init(Consensus::LLMQType _llmqType);
void Init(size_t size);
bool IsSet(uint16_t quorumMember) const;
void Set(uint16_t quorumMember, bool v);
void Merge(const CSigSharesInv& inv2);
@ -127,7 +127,7 @@ public:
READWRITE(sigShares);
}
CSigSharesInv ToInv(Consensus::LLMQType llmqType) const;
std::string ToInvString() const;
};
template<typename T>