Fix race conditions in shard:

ThreadSafetyAnalysis was used to identify race conditions in this file.
This analysis was modivated by a (rare) crash while running unit tests.

Add locks to Shard flagged by ThreadSafetyAnalysis

src/ripple/nodestore/impl/Shard.cpp

@@ -223,6 +223,7 @@ Shard::storeNodeObject(std::shared_ptr<NodeObject> const& nodeObject)
 
     try
     {
+        std::lock_guard lock(mutex_);
         backend_->store(nodeObject);
     }
     catch (std::exception const& e)
@@ -249,6 +250,7 @@ Shard::fetchNodeObject(uint256 const& hash, FetchReport& fetchReport)
     Status status;
     try
     {
+        std::lock_guard lock(mutex_);
         status = backend_->fetch(hash.data(), &nodeObject);
     }
     catch (std::exception const& e)
@@ -331,6 +333,7 @@ Shard::storeLedger(
 
         try
         {
+            std::lock_guard lock(mutex_);
             backend_->storeBatch(batch);
         }
         catch (std::exception const& e)
@@ -538,6 +541,7 @@ Shard::getWriteLoad()
     auto const scopedCount{makeBackendCount()};
     if (!scopedCount)
         return 0;
+    std::lock_guard lock(mutex_);
     return backend_->getWriteLoad();
 }
 
@@ -572,6 +576,8 @@ Shard::finalize(bool writeSQLite, std::optional<uint256> const& referenceHash)
 
     try
     {
+        std::lock_guard lock(mutex_);
+
         state_ = ShardState::finalizing;
         progress_ = 0;
 
@@ -759,8 +765,11 @@ Shard::finalize(bool writeSQLite, std::optional<uint256> const& referenceHash)
 
     try
     {
-        // Store final key's value, may already be stored
-        backend_->store(nodeObject);
+        {
+            // Store final key's value, may already be stored
+            std::lock_guard lock(mutex_);
+            backend_->store(nodeObject);
+        }
 
         // Do not allow all other threads work with the shard
         busy_ = true;
@@ -819,7 +828,7 @@ Shard::open(std::lock_guard<std::mutex> const& lock)
     using namespace boost::filesystem;
     Config const& config{app_.config()};
     auto preexist{false};
-    auto fail = [this, &preexist](std::string const& msg) {
+    auto fail = [this, &preexist](std::string const& msg) REQUIRES(mutex_) {
         backend_->close();
         lgrSQLiteDB_.reset();
         txSQLiteDB_.reset();
@@ -837,7 +846,7 @@ Shard::open(std::lock_guard<std::mutex> const& lock)
         }
         return false;
     };
-    auto createAcquireInfo = [this, &config]() {
+    auto createAcquireInfo = [this, &config]() REQUIRES(mutex_) {
         DatabaseCon::Setup setup;
         setup.startUp = config.standalone() ? config.LOAD : config.START_UP;
         setup.standAlone = config.standalone();
@@ -1024,6 +1033,8 @@ Shard::storeSQLite(std::shared_ptr<Ledger const> const& ledger)
 
     try
     {
+        std::lock_guard lock(mutex_);
+
         auto res = updateLedgerDBs(
             *txSQLiteDB_->checkoutDb(),
             *lgrSQLiteDB_->checkoutDb(),
@@ -1186,6 +1197,7 @@ Shard::verifyFetch(uint256 const& hash) const
 
     try
     {
+        std::lock_guard lock(mutex_);
         switch (backend_->fetch(hash.data(), &nodeObject))
         {
             case ok:

src/ripple/nodestore/impl/Shard.h

@@ -26,6 +26,7 @@
 #include <ripple/basics/KeyCache.h>
 #include <ripple/basics/MathUtilities.h>
 #include <ripple/basics/RangeSet.h>
+#include <ripple/basics/ThreadSafetyAnalysis.h>
 #include <ripple/core/DatabaseCon.h>
 #include <ripple/nodestore/NodeObject.h>
 #include <ripple/nodestore/Scheduler.h>
@@ -210,6 +211,7 @@ class Shard final
     std::string
     getStoredSeqs()
     {
+        std::lock_guard lock(mutex_);
         if (!acquireInfo_)
             return "";
 
@@ -316,29 +318,30 @@ class Shard final
     boost::filesystem::path const dir_;
 
     // Storage space utilized by the shard
-    std::uint64_t fileSz_{0};
+    GUARDED_BY(mutex_) std::uint64_t fileSz_{0};
 
     // Number of file descriptors required by the shard
-    std::uint32_t fdRequired_{0};
+    GUARDED_BY(mutex_) std::uint32_t fdRequired_{0};
 
     // NuDB key/value store for node objects
-    std::unique_ptr<Backend> backend_;
+    GUARDED_BY(mutex_) std::unique_ptr<Backend> backend_;
 
     std::atomic<std::uint32_t> backendCount_{0};
 
     // Ledger SQLite database used for indexes
-    std::unique_ptr<DatabaseCon> lgrSQLiteDB_;
+    std::unique_ptr<DatabaseCon> lgrSQLiteDB_ GUARDED_BY(mutex_);
 
     // Transaction SQLite database used for indexes
-    std::unique_ptr<DatabaseCon> txSQLiteDB_;
+    std::unique_ptr<DatabaseCon> txSQLiteDB_ GUARDED_BY(mutex_);
 
     // Tracking information used only when acquiring a shard from the network.
     // If the shard is finalized, this member will be null.
-    std::unique_ptr<AcquireInfo> acquireInfo_;
+    std::unique_ptr<AcquireInfo> acquireInfo_ GUARDED_BY(mutex_);
+    ;
 
     // Older shard without an acquire database or final key
     // Eventually there will be no need for this and should be removed
-    bool legacy_{false};
+    GUARDED_BY(mutex_) bool legacy_{false};
 
     // Determines if the shard needs to stop processing for shutdown
     std::atomic<bool> stop_{false};
@@ -356,16 +359,17 @@ class Shard final
     std::atomic<bool> removeOnDestroy_{false};
 
     // The time of the last access of a shard with a finalized state
-    std::chrono::steady_clock::time_point lastAccess_;
+    std::chrono::steady_clock::time_point lastAccess_ GUARDED_BY(mutex_);
+    ;
 
     // Open shard databases
     [[nodiscard]] bool
-    open(std::lock_guard<std::mutex> const& lock);
+    open(std::lock_guard<std::mutex> const& lock) REQUIRES(mutex_);
 
     // Open/Create SQLite databases
     // Lock over mutex_ required
     [[nodiscard]] bool
-    initSQLite(std::lock_guard<std::mutex> const&);
+    initSQLite(std::lock_guard<std::mutex> const&) REQUIRES(mutex_);
 
     // Write SQLite entries for this ledger
     [[nodiscard]] bool
@@ -374,7 +378,7 @@ class Shard final
     // Set storage and file descriptor usage stats
     // Lock over mutex_ required
     void
-    setFileStats(std::lock_guard<std::mutex> const&);
+    setFileStats(std::lock_guard<std::mutex> const&) REQUIRES(mutex_);
 
     // Verify this ledger by walking its SHAMaps and verifying its Merkle trees
     // Every node object verified will be stored in the deterministic shard