Skip to content

Commit 2f1a7b3

Browse files
committed
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) dashpay#1 get /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:361:21 (dashd+0xb58e08) dashpay#2 operator-> /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:355:9 (dashd+0xb58e08) dashpay#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) dashpay#1 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:114:27 (dashd+0xb772b4) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) dashpay#2 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:443:20 (dashd+0xb772b4) dashpay#3 _M_get_node /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:580:16 (dashd+0xb772b4) [...] dashpay#8 CoinJoinWalletManager::Add(CWallet&) /src/dash/src/coinjoin/client.cpp:1898:26 (dashd+0xb58c73) (BuildId: c3fdce9f7e778985a4fb0968ff4506d9ad24d408) [...] SUMMARY: ThreadSanitizer: data race [...] ```
1 parent 25eef1e commit 2f1a7b3

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

src/coinjoin/client.cpp

+15-8
Original file line numberDiff line numberDiff line change
@@ -1895,34 +1895,41 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
18951895
}
18961896

18971897
void CoinJoinWalletManager::Add(CWallet& wallet) {
1898-
m_wallet_manager_map.try_emplace(
1899-
wallet.GetName(),
1900-
std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman, m_is_masternode)
1901-
);
1898+
{
1899+
LOCK(cs_wallet_manager_map);
1900+
m_wallet_manager_map.try_emplace(
1901+
wallet.GetName(),
1902+
std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman, m_mn_sync,
1903+
m_queueman, m_is_masternode)
1904+
);
1905+
}
19021906
g_wallet_init_interface.InitCoinJoinSettings(*this);
19031907
}
19041908

19051909
void CoinJoinWalletManager::DoMaintenance() {
1906-
for (auto& [wallet_str, walletman] : m_wallet_manager_map) {
1910+
for (auto& [wallet_str, walletman] : raw()) {
19071911
walletman->DoMaintenance(m_chainstate, m_connman, m_mempool);
19081912
}
19091913
}
19101914

19111915
void CoinJoinWalletManager::Remove(const std::string& name) {
1912-
m_wallet_manager_map.erase(name);
1916+
{
1917+
LOCK(cs_wallet_manager_map);
1918+
m_wallet_manager_map.erase(name);
1919+
}
19131920
g_wallet_init_interface.InitCoinJoinSettings(*this);
19141921
}
19151922

19161923
void CoinJoinWalletManager::Flush(const std::string& name)
19171924
{
1918-
auto clientman = Get(name);
1919-
assert(clientman != nullptr);
1925+
auto clientman = Assert(Get(name));
19201926
clientman->ResetPool();
19211927
clientman->StopMixing();
19221928
}
19231929

19241930
CCoinJoinClientManager* CoinJoinWalletManager::Get(const std::string& name) const
19251931
{
1932+
LOCK(cs_wallet_manager_map);
19261933
auto it = m_wallet_manager_map.find(name);
19271934
return (it != m_wallet_manager_map.end()) ? it->second.get() : nullptr;
19281935
}

src/coinjoin/client.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,11 @@ class CoinJoinWalletManager {
9393

9494
CCoinJoinClientManager* Get(const std::string& name) const;
9595

96-
const wallet_name_cjman_map& raw() const { return m_wallet_manager_map; }
96+
const wallet_name_cjman_map& raw() const
97+
{
98+
LOCK(cs_wallet_manager_map);
99+
return m_wallet_manager_map;
100+
}
97101

98102
private:
99103
CChainState& m_chainstate;
@@ -105,7 +109,9 @@ class CoinJoinWalletManager {
105109
const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
106110

107111
const bool m_is_masternode;
108-
wallet_name_cjman_map m_wallet_manager_map;
112+
113+
mutable RecursiveMutex cs_wallet_manager_map;
114+
wallet_name_cjman_map m_wallet_manager_map GUARDED_BY(cs_wallet_manager_map);
109115
};
110116

111117
class CCoinJoinClientSession : public CCoinJoinBaseSession

0 commit comments

Comments
 (0)