Merge #2923: [Bug] Fix wallet data race

f9a6ebd fix: solve data race by making nTimeBestReceived atomic (ale) 4331c30 refactor: move nTimeBestReceived to CWallet (ale) Pull request description: First commit is a small refactor: the global variable `nTimeBestReceived` is now a private member of the `CWallet` class, as it was used only there. Second commit: fix the following data race by making `nTimeBestReceived` atomic ``` WARNING: ThreadSanitizer: data race (pid=18270) Write of size 8 at 0x7b6800000ed8 by thread T16: #0 CWallet::UpdatedBlockTip(CBlockIndex const*, CBlockIndex const*, bool) wallet/wallet.cpp:2094 (pivxd+0x5058c9) #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2ee1c0) #2 std::__invoke_result<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>::type std::__invoke<void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*, CBlockIndex const*, bool>(void (CValidationInterface::*&)(CBlockIndex const*, CBlockIndex const*, bool), CValidationInterface*&, CBlockIndex const*&&, CBlockIndex const*&&, bool&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2ee25a) #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>, std::_Placeholder<2>, std::_Placeholder<3>))(CBlockIndex const*, CBlockIndex const*, bool)>::__call<void, CBlockIndex const*&&, CBlockIndex const*&&, bool&&, 0ul, 1ul, 2ul, 3ul>(std::tuple<CBlockIndex const*&&, CBlockIndex const*&&, bool&&>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul>) /usr/include/c++/12/functional:484 (pivxd+0x2ee25a) ... Previous read of size 8 at 0x7b6800000ed8 by thread T35 (mutexes: write M134298, write M132880): #0 CWallet::ResendWalletTransactions(CConnman*) wallet/wallet.cpp:2065 (pivxd+0x515b25) #1 void std::__invoke_impl<void, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(std::__invoke_memfun_deref, void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:74 (pivxd+0x2eef45) #2 std::__invoke_result<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>::type std::__invoke<void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*>(void (CValidationInterface::*&)(CConnman*), CValidationInterface*&, CConnman*&&) /usr/include/c++/12/bits/invoke.h:96 (pivxd+0x2eefb9) #3 void std::_Bind<void (CValidationInterface::*(CValidationInterface*, std::_Placeholder<1>))(CConnman*)>::__call<void, CConnman*&&, 0ul, 1ul>(std::tuple<CConnman*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/functional:484 (pivxd+0x2eefb9) ... ``` ACKs for top commit: f9a6ebd Duddino: utACK f9a6ebd Liquid369: tACK f9a6ebd Fuzzbawls: ACK f9a6ebd Tree-SHA512: c1de883956bf0958204b05c338831c27f694e6453e74b77e3347ae12a3cbd69df2fe1120bae3fca72dab14587c0c499c4f92d5c53943305855e66fc1275ab4fa
PIVX-Project · Apr 20, 2024 · 694eb59 · 694eb59
2 parents db78a7e + f9a6ebd
commit 694eb59
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 4 deletions.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -34,7 +34,6 @@
 
 using namespace std::chrono_literals;
 
-int64_t nTimeBestReceived = 0;  // Used only to inform the wallet of when we last received a block
 
 static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8]
 
@@ -757,8 +756,6 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex* pindexNew, const CB
             }
         });
     }
-
-    nTimeBestReceived = GetTime();
 }
 
 void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state)

diff --git a/src/validation.h b/src/validation.h
@@ -129,7 +129,6 @@ extern BlockMap mapBlockIndex;
 extern PrevBlockMap mapPrevBlockIndex;
 extern uint64_t nLastBlockTx;
 extern uint64_t nLastBlockSize;
-extern int64_t nTimeBestReceived;
 
 // Best block section
 extern Mutex g_best_block_mutex;

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -2088,6 +2088,11 @@ void CWallet::ResendWalletTransactions(CConnman* connman)
     }
 }
 
+void CWallet::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
+{
+    nTimeBestReceived = GetTime();
+}
+
 /** @} */ // end of mapWallet
 
 

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -628,6 +628,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
 
     int64_t nNextResend;
     int64_t nLastResend;
+    std::atomic<int64_t> nTimeBestReceived{0}; // Used only to inform the wallet of when we last received a block
 
     /**
      * Used to keep track of spent outpoints, and
@@ -1042,6 +1043,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
     void ReacceptWalletTransactions(bool fFirstLoad = false);
     void ResendWalletTransactions(CConnman* connman) override;
+    void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;
 
     struct Balance {
         CAmount m_mine_trusted{0};               //!< Trusted, at depth=GetBalance.min_depth or more
-Original file line number
+Diff line change
@@ Expand Up @@
         }
     }
+    void CWallet::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
+    {
+        nTimeBestReceived = GetTime();
+    }
     /** @} */ // end of mapWallet
@@ Expand Down @@