Skip to content

Commit e9c2fea

Browse files
feat: convert CSigSharesManager to use Mutex instead of RecursiveMutex
update lock requirements for various methods. This change enhances thread safety and simplifies locking semantics throughout the class.
1 parent d56bff5 commit e9c2fea

File tree

1 file changed

+33
-36
lines changed

1 file changed

+33
-36
lines changed

src/llmq/signing_shares.h

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ class CSigSharesManager : public CRecoveredSigsListener
376376
static constexpr int64_t MAX_SEND_FOR_RECOVERY_TIMEOUT{10000};
377377
static constexpr size_t MAX_MSGS_SIG_SHARES{32};
378378

379-
RecursiveMutex cs;
379+
Mutex cs;
380380

381381
std::thread workThread;
382382
CThreadInterrupt workInterrupt;
@@ -424,73 +424,70 @@ class CSigSharesManager : public CRecoveredSigsListener
424424
const CQuorumManager& _qman, const CSporkManager& sporkman);
425425
~CSigSharesManager() override;
426426

427-
void StartWorkerThread();
428-
void StopWorkerThread();
429-
void RegisterAsRecoveredSigsListener();
430-
void UnregisterAsRecoveredSigsListener();
431-
void InterruptWorkerThread();
427+
void StartWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);
428+
void StopWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);
429+
void RegisterAsRecoveredSigsListener() EXCLUSIVE_LOCKS_REQUIRED(!cs);
430+
void UnregisterAsRecoveredSigsListener() EXCLUSIVE_LOCKS_REQUIRED(!cs);
431+
void InterruptWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);
432432

433-
void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv);
433+
void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs);
434434

435435
void AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash)
436-
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
437-
std::optional<CSigShare> CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const;
438-
void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
436+
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
437+
std::optional<CSigShare> CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
438+
void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
439439

440-
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
440+
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs);
441441

442442
static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, int attempt);
443443

444444
bool AsyncSignIfMember(Consensus::LLMQType llmqType, CSigningManager& sigman, const uint256& id,
445445
const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false,
446446
bool allowDiffMsgHashSigning = false)
447-
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
447+
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
448448

449-
void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const;
449+
void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
450450

451451
private:
452452
// all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages)
453-
bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann);
454-
bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv);
455-
bool ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv);
456-
bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares);
457-
void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare);
453+
bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs);
454+
bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs);
455+
bool ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs);
456+
bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) EXCLUSIVE_LOCKS_REQUIRED(!cs);
457+
void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) EXCLUSIVE_LOCKS_REQUIRED(!cs);
458458

459459
static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv);
460460
static bool PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
461461
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan);
462462

463463
bool CollectPendingSigSharesToVerify(
464464
size_t maxUniqueSessions, std::unordered_map<NodeId, std::vector<CSigShare>>& retSigShares,
465-
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& retQuorums);
466-
bool ProcessPendingSigShares();
465+
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& retQuorums) EXCLUSIVE_LOCKS_REQUIRED(!cs);
466+
bool ProcessPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs);
467467

468468
void ProcessPendingSigShares(
469469
const std::vector<CSigShare>& sigSharesToProcess,
470-
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums);
470+
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums) EXCLUSIVE_LOCKS_REQUIRED(!cs);
471471

472-
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum);
473-
void TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash);
472+
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs);
473+
void TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
474474

475-
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo);
475+
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) EXCLUSIVE_LOCKS_REQUIRED(!cs);
476476
static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in);
477477

478-
void Cleanup();
478+
void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs);
479479
void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs);
480-
void RemoveBannedNodeStates();
480+
void RemoveBannedNodeStates() EXCLUSIVE_LOCKS_REQUIRED(!cs);
481481

482-
void BanNode(NodeId nodeId);
482+
void BanNode(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(!cs);
483483

484-
bool SendMessages();
485-
void CollectSigSharesToRequest(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToRequest)
486-
EXCLUSIVE_LOCKS_REQUIRED(cs);
487-
void CollectSigSharesToSend(std::unordered_map<NodeId, Uint256HashMap<CBatchedSigShares>>& sigSharesToSend)
488-
EXCLUSIVE_LOCKS_REQUIRED(cs);
484+
bool SendMessages() EXCLUSIVE_LOCKS_REQUIRED(!cs);
485+
void CollectSigSharesToRequest(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToRequest) EXCLUSIVE_LOCKS_REQUIRED(cs);
486+
void CollectSigSharesToSend(std::unordered_map<NodeId, Uint256HashMap<CBatchedSigShares>>& sigSharesToSend) EXCLUSIVE_LOCKS_REQUIRED(cs);
489487
void CollectSigSharesToSendConcentrated(std::unordered_map<NodeId, std::vector<CSigShare>>& sigSharesToSend, const std::vector<CNode*>& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs);
490-
void CollectSigSharesToAnnounce(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToAnnounce)
491-
EXCLUSIVE_LOCKS_REQUIRED(cs);
492-
void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
493-
void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
488+
void CollectSigSharesToAnnounce(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToAnnounce) EXCLUSIVE_LOCKS_REQUIRED(cs);
489+
void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
490+
void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
494491
};
495492
} // namespace llmq
496493

0 commit comments

Comments
 (0)