From 0e818fc0f451bdb2dd380925e5f432c5c475bda9 Mon Sep 17 00:00:00 2001
From: Richard Holland <richard.holland@starstone.co.nz>
Date: Sat, 12 Oct 2024 18:38:04 +1100
Subject: [PATCH] ensure peekMutex works correctly in the new setup

---
 src/ripple/app/ledger/LedgerHistory.cpp | 34 +++++++++++++++----------
 src/ripple/app/ledger/LedgerHistory.h   |  2 ++
 src/ripple/basics/TaggedCache.h         |  7 ++---
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/ripple/app/ledger/LedgerHistory.cpp b/src/ripple/app/ledger/LedgerHistory.cpp
index b23988985..1cc7c24f9 100644
--- a/src/ripple/app/ledger/LedgerHistory.cpp
+++ b/src/ripple/app/ledger/LedgerHistory.cpp
@@ -58,12 +58,15 @@ LedgerHistory::insert(std::shared_ptr<Ledger const> ledger, bool validated)
 
     assert(ledger->stateMap().getHash().isNonZero());
 
-    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
+    std::unique_lock sl(m_ledgers_by_hash.peekMutex(ledger->info().hash));
 
     const bool alreadyHad = m_ledgers_by_hash.canonicalize_replace_cache(
         ledger->info().hash, ledger);
     if (validated)
+    {
+        std::unique_lock<std::shared_mutex> lock(mLedgersByIndexMutex);
         mLedgersByIndex[ledger->info().seq] = ledger->info().hash;
+    }
 
     return alreadyHad;
 }
@@ -71,12 +74,12 @@ LedgerHistory::insert(std::shared_ptr<Ledger const> ledger, bool validated)
 LedgerHash
 LedgerHistory::getLedgerHash(LedgerIndex index)
 {
-    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
+    std::unique_lock<std::shared_mutex> lock(mLedgersByIndexMutex);
     auto it = mLedgersByIndex.find(index);
 
     if (it != mLedgersByIndex.end())
         return it->second;
-
+    
     return uint256();
 }
 
@@ -84,13 +87,13 @@ std::shared_ptr<Ledger const>
 LedgerHistory::getLedgerBySeq(LedgerIndex index)
 {
     {
-        std::unique_lock sl(m_ledgers_by_hash.peekMutex());
+        std::unique_lock<std::shared_mutex> lock(mLedgersByIndexMutex);
+
         auto it = mLedgersByIndex.find(index);
 
         if (it != mLedgersByIndex.end())
         {
             uint256 hash = it->second;
-            sl.unlock();
             return getLedgerByHash(hash);
         }
     }
@@ -104,11 +107,16 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index)
 
     {
         // Add this ledger to the local tracking by index
-        std::unique_lock sl(m_ledgers_by_hash.peekMutex());
-
-        assert(ret->isImmutable());
-        m_ledgers_by_hash.canonicalize_replace_client(ret->info().hash, ret);
-        mLedgersByIndex[ret->info().seq] = ret->info().hash;
+        {
+            std::unique_lock sl(m_ledgers_by_hash.peekMutex(ret->info().hash));
+            assert(ret->isImmutable());
+            m_ledgers_by_hash.canonicalize_replace_client(ret->info().hash, ret);
+        }
+    
+        {
+            std::unique_lock<std::shared_mutex> lock(mLedgersByIndexMutex);
+            mLedgersByIndex[ret->info().seq] = ret->info().hash;
+        }
         return (ret->info().seq == index) ? ret : nullptr;
     }
 }
@@ -440,7 +448,7 @@ LedgerHistory::builtLedger(
     LedgerHash hash = ledger->info().hash;
     assert(!hash.isZero());
 
-    std::unique_lock sl(m_consensus_validated.peekMutex());
+    std::unique_lock sl(m_consensus_validated.peekMutex(index));
 
     auto entry = std::make_shared<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
@@ -480,7 +488,7 @@ LedgerHistory::validatedLedger(
     LedgerHash hash = ledger->info().hash;
     assert(!hash.isZero());
 
-    std::unique_lock sl(m_consensus_validated.peekMutex());
+    std::unique_lock sl(m_consensus_validated.peekMutex(index));
 
     auto entry = std::make_shared<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
@@ -515,7 +523,7 @@ LedgerHistory::validatedLedger(
 bool
 LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash)
 {
-    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
+    std::unique_lock<std::shared_mutex> lock(mLedgersByIndexMutex);
     auto it = mLedgersByIndex.find(ledgerIndex);
 
     if ((it != mLedgersByIndex.end()) && (it->second != ledgerHash))
diff --git a/src/ripple/app/ledger/LedgerHistory.h b/src/ripple/app/ledger/LedgerHistory.h
index be5c559be..f2740ad41 100644
--- a/src/ripple/app/ledger/LedgerHistory.h
+++ b/src/ripple/app/ledger/LedgerHistory.h
@@ -27,6 +27,7 @@
 #include <ripple/protocol/RippleLedgerHash.h>
 
 #include <optional>
+#include <shared_mutex>
 
 namespace ripple {
 
@@ -150,6 +151,7 @@ private:
 
     // Maps ledger indexes to the corresponding hash.
     std::map<LedgerIndex, LedgerHash> mLedgersByIndex;  // validated ledgers
+    std::shared_mutex mLedgersByIndexMutex;
 
     beast::Journal j_;
 };
diff --git a/src/ripple/basics/TaggedCache.h b/src/ripple/basics/TaggedCache.h
index 09107aeb6..112d94038 100644
--- a/src/ripple/basics/TaggedCache.h
+++ b/src/ripple/basics/TaggedCache.h
@@ -981,12 +981,9 @@ public:
         return caches[getCacheIndex(key)]->retrieve(key, data);
     }
 
-    mutex_type& peekMutex()
+    mutex_type& peekMutex(key_type const& key)
     {
-        // This is tricky as we have multiple mutexes now.
-        // For simplicity, we'll return the mutex of the first cache,
-        // but this might not be the best approach in all scenarios.
-        return caches[0]->peekMutex();
+        return caches[getCacheIndex(key)]->peekMutex();
     }
 
     std::vector<key_type> getKeys() const