From 2f1a7b3aef6bcae28fb856d305667fd153c5fe96 Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Mon, 12 Aug 2024 12:17:39 +0000
Subject: [PATCH] fix: protect `m_wallet_manager_map` with
 `cs_wallet_manager_map`

Avoid TSan-reported data race

```
WARNING: ThreadSanitizer: data race (pid=374820)
  Read of size 8 at 0x7b140002ce10 by thread T12:
    #0 _M_ptr /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:154:42 (dashd+0xb58e08) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408)
    #1 get /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:361:21 (dashd+0xb58e08)
    #2 operator-> /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:355:9 (dashd+0xb58e08)
    #3 CoinJoinWalletManager::DoMaintenance() /src/dash/src/coinjoin/client.cpp:1907:9 (dashd+0xb58e08)
    [...]
  Previous write of size 8 at 0x7b140002ce10 by thread T17 (mutexes: write M0):
    #0 operator new(unsigned long) <null> (dashd+0x162657) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408)
    #1 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:114:27 (dashd+0xb772b4) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408)
    #2 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:443:20 (dashd+0xb772b4)
    #3 _M_get_node /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:580:16 (dashd+0xb772b4)
    [...]
    #8 CoinJoinWalletManager::Add(CWallet&) /src/dash/src/coinjoin/client.cpp:1898:26 (dashd+0xb58c73) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408)
[...]
SUMMARY: ThreadSanitizer: data race [...]
```
---
 src/coinjoin/client.cpp | 23 +++++++++++++++--------
 src/coinjoin/client.h   | 10 ++++++++--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
index 1675a6408e831..131a03d1b60b1 100644
--- a/src/coinjoin/client.cpp
+++ b/src/coinjoin/client.cpp
@@ -1895,34 +1895,41 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
 }
 
 void CoinJoinWalletManager::Add(CWallet& wallet) {
-    m_wallet_manager_map.try_emplace(
-        wallet.GetName(),
-        std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, m_is_masternode)
-    );
+    {
+        LOCK(cs_wallet_manager_map);
+        m_wallet_manager_map.try_emplace(
+            wallet.GetName(),
+            std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync,
+                                                     m_queueman, m_is_masternode)
+        );
+    }
     g_wallet_init_interface.InitCoinJoinSettings(*this);
 }
 
 void CoinJoinWalletManager::DoMaintenance() {
-    for (auto& [wallet_str, walletman] : m_wallet_manager_map) {
+    for (auto& [wallet_str, walletman] : raw()) {
         walletman->DoMaintenance(m_chainstate, m_connman, m_mempool);
     }
 }
 
 void CoinJoinWalletManager::Remove(const std::string& name) {
-    m_wallet_manager_map.erase(name);
+    {
+        LOCK(cs_wallet_manager_map);
+        m_wallet_manager_map.erase(name);
+    }
     g_wallet_init_interface.InitCoinJoinSettings(*this);
 }
 
 void CoinJoinWalletManager::Flush(const std::string& name)
 {
-    auto clientman = Get(name);
-    assert(clientman != nullptr);
+    auto clientman = Assert(Get(name));
     clientman->ResetPool();
     clientman->StopMixing();
 }
 
 CCoinJoinClientManager* CoinJoinWalletManager::Get(const std::string& name) const
 {
+    LOCK(cs_wallet_manager_map);
     auto it = m_wallet_manager_map.find(name);
     return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr;
 }
diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h
index ba77e565cf264..e6e5046f71306 100644
--- a/src/coinjoin/client.h
+++ b/src/coinjoin/client.h
@@ -93,7 +93,11 @@ class CoinJoinWalletManager {
 
     CCoinJoinClientManager* Get(const std::string& name) const;
 
-    const wallet_name_cjman_map& raw() const { return m_wallet_manager_map; }
+    const wallet_name_cjman_map& raw() const
+    {
+        LOCK(cs_wallet_manager_map);
+        return m_wallet_manager_map;
+    }
 
 private:
     CChainState& m_chainstate;
@@ -105,7 +109,9 @@ class CoinJoinWalletManager {
     const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
 
     const bool m_is_masternode;
-    wallet_name_cjman_map m_wallet_manager_map;
+
+    mutable RecursiveMutex cs_wallet_manager_map;
+    wallet_name_cjman_map m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map);
 };
 
 class CCoinJoinClientSession : public CCoinJoinBaseSession