refactor: streamline CSigSharesManager's message handling and improve thread safety

PastaPastaPasta · PastaPastaPasta · commit 228e568d6f6f · 2025-11-22T09:21:55.000-06:00
- Removed unnecessary lastSendTime variable and simplified message sending in HousekeepingThreadMain.
- Enhanced DispatchPendingSigns by swapping the entire vector of pending signs to reduce lock contention and improve performance.
- Updated ActiveContext to start and stop the share manager more efficiently.
diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp
@@ -1613,16 +1613,9 @@ void CSigSharesManager::BanNode(NodeId nodeId)
 
 void CSigSharesManager::HousekeepingThreadMain()
 {
-    int64_t lastSendTime = 0;
-
     while (!workInterrupt) {
         RemoveBannedNodeStates();
-
-        if (TicksSinceEpoch<std::chrono::milliseconds>(SystemClock::now()) - lastSendTime > 100) {
-            SendMessages();
-            lastSendTime = TicksSinceEpoch<std::chrono::milliseconds>(SystemClock::now());
-        }
-
+        SendMessages();
         Cleanup();
 
         workInterrupt.sleep_for(std::chrono::milliseconds(100));
@@ -1645,18 +1638,18 @@ void CSigSharesManager::WorkDispatcherThreadMain()
 
 void CSigSharesManager::DispatchPendingSigns()
 {
-    // Pop and dispatch ALL pending signs until queue is empty
-    while (!workInterrupt) {
-        std::optional<PendingSignatureData> work;
-        {
-            LOCK(cs_pendingSigns);
-            if (pendingSigns.empty()) break;
-            // Move the data out of the vector
-            work.emplace(std::move(pendingSigns.back()));
-            pendingSigns.pop_back();
-        }
+    // Swap out entire vector to avoid lock thrashing
+    std::vector<PendingSignatureData> signs;
+    {
+        LOCK(cs_pendingSigns);
+        signs.swap(pendingSigns);
+    }
+
+    // Dispatch all signs to worker pool
+    for (auto& work : signs) {
+        if (workInterrupt) break;
 
-        workerPool.push([this, work = std::move(*work)](int) {
+        workerPool.push([this, work = std::move(work)](int) {
             SignAndProcessSingleShare(std::move(work));
         });
     }
diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp
@@ -55,12 +55,12 @@ void ActiveContext::Start(CConnman& connman, PeerManager& peerman)
 {
     m_llmq_ctx.qdkgsman->StartThreads(connman, peerman);
     shareman->RegisterAsRecoveredSigsListener();
-    shareman->StartWorkerThread();
+    shareman->Start();
 }
 
 void ActiveContext::Stop()
 {
-    shareman->StopWorkerThread();
+    shareman->Stop();
     shareman->UnregisterAsRecoveredSigsListener();
     m_llmq_ctx.qdkgsman->StopThreads();
 }

Original file line number	Diff line number	Diff line change
`@@ -55,12 +55,12 @@ void ActiveContext::Start(CConnman& connman, PeerManager& peerman)`
`55`	`55`	`{`
`56`	`56`	`m_llmq_ctx.qdkgsman->StartThreads(connman, peerman);`
`57`	`57`	`shareman->RegisterAsRecoveredSigsListener();`
`58`		`- shareman->StartWorkerThread();`
	`58`	`+ shareman->Start();`
`59`	`59`	`}`
`60`	`60`
`61`	`61`	`void ActiveContext::Stop()`
`62`	`62`	`{`
`63`		`- shareman->StopWorkerThread();`
	`63`	`+ shareman->Stop();`
`64`	`64`	`shareman->UnregisterAsRecoveredSigsListener();`
`65`	`65`	`m_llmq_ctx.qdkgsman->StopThreads();`
`66`	`66`	`}`