Merge #950: [Consensus] Add locking on the chain index vector.

3cf8acf Add locking on the chain index vector. (Zannick)

Pull request description:

  ### Problem
  tsan identified a data race on CChain, particularly concurrent access to the underlying vector's `operator[]`, `size`, and `resize` member functions.

  ### Solution
  Create a new lock `cs_vchain` and mark `vchain` as guarded by it, and ensure that the lock is taken prior to any use.
  For the `operator==` member function, grab both the locks in a prescribed order to avoid a deadlock.

  ### Tested
  Configured with --with-sanitizers=tsan, built with clang, run on regtest with 2 SHA mining threads enabled.

Tree-SHA512: eac3c382c0afb8107865dbe041e8a0991e976cbccd602a80925d0fb1cc3c3d4e758c7a97605a7ba14547c420042d00ff7ba0744d20d28dc77b5928eade41b517

src/chain.cpp

@@ -16,6 +16,7 @@
  * CChain implementation
  */
 void CChain::SetTip(CBlockIndex *pindex) {
+    LOCK(cs_vchain);
     if (pindex == nullptr) {
         vChain.clear();
         return;
@@ -68,6 +69,7 @@ const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {
 
 CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime) const
 {
+    LOCK(cs_vchain);
     std::vector<CBlockIndex*>::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), nTime,
         [](CBlockIndex* pBlock, const int64_t& time) -> bool { return pBlock->GetBlockTimeMax() < time; });
     return (lower == vChain.end() ? nullptr : *lower);

src/chain.h

@@ -725,28 +725,36 @@ class CDiskBlockIndex : public CBlockIndex
 /** An in-memory indexed chain of blocks. */
 class CChain {
 private:
-    std::vector<CBlockIndex*> vChain;
+    mutable CCriticalSection cs_vchain;
+    std::vector<CBlockIndex*> vChain GUARDED_BY(cs_vchain);
 
 public:
     /** Returns the index entry for the genesis block of this chain, or nullptr if none. */
     CBlockIndex *Genesis() const {
+        LOCK(cs_vchain);
         return vChain.size() > 0 ? vChain[0] : nullptr;
     }
 
     /** Returns the index entry for the tip of this chain, or nullptr if none. */
     CBlockIndex *Tip() const {
+        LOCK(cs_vchain);
         return vChain.size() > 0 ? vChain[vChain.size() - 1] : nullptr;
     }
 
     /** Returns the index entry at a particular height in this chain, or nullptr if no such height exists. */
     CBlockIndex *operator[](int nHeight) const {
+        LOCK(cs_vchain);
         if (nHeight < 0 || nHeight >= (int)vChain.size())
             return nullptr;
         return vChain[nHeight];
     }
 
     /** Compare two chains efficiently. */
     friend bool operator==(const CChain &a, const CChain &b) {
+        // Maintain consistent lock order.
+        if (&b < &a) return b == a;
+
+        LOCK2(a.cs_vchain, b.cs_vchain);
         return a.vChain.size() == b.vChain.size() &&
                a.vChain[a.vChain.size() - 1] == b.vChain[b.vChain.size() - 1];
     }
@@ -758,6 +766,7 @@ class CChain {
 
     /** Find the successor of a block in this chain, or nullptr if the given index is not found or is the tip. */
     CBlockIndex *Next(const CBlockIndex *pindex) const {
+        LOCK(cs_vchain);
         if (Contains(pindex))
             return (*this)[pindex->nHeight + 1];
         else
@@ -766,6 +775,7 @@ class CChain {
 
     /** Return the maximal height in the chain. Is equal to chain.Tip() ? chain.Tip()->nHeight : -1. */
     int Height() const {
+        LOCK(cs_vchain);
         return vChain.size() - 1;
     }