refactor: Rework database lock management

* Follow-up to XRPLF#4989

src/xrpld/nodestore/DatabaseRotating.h

@@ -45,6 +45,10 @@ class DatabaseRotating : public Database
     /** Rotates the backends.
 
         @param f A function executed before the rotation and under the same lock
+
+        This function only has one caller in SHAMapStoreImp::run. The locking
+        for this function will need to be rethought another caller is ever
+        added.
     */
     virtual void
     rotateWithLock(std::function<std::unique_ptr<NodeStore::Backend>(

src/xrpld/nodestore/detail/DatabaseRotatingImp.cpp

@@ -45,9 +45,23 @@ DatabaseRotatingImp::rotateWithLock(
     std::function<std::unique_ptr<NodeStore::Backend>(
         std::string const& writableBackendName)> const& f)
 {
+    auto const writableBackend = [&] {
+        std::lock_guard lock(mutex_);
+        return writableBackend_;
+    }();
+
+    auto newBackend = f(writableBackend->getName());
+
     std::lock_guard lock(mutex_);
 
-    auto newBackend = f(writableBackend_->getName());
+    // This function only has one caller, which is syncronous, and this is the
+    // only place other than the ctor where writableBackend_ can change. Even
+    // without a lock, it should be impossible for writableBackend_ to have
+    // changed.
+    assert(writableBackend_ == writableBackend);
+    if (writableBackend_ != writableBackend)
+        LogicError("Backend changed in the middle of a rotation");
+
     archiveBackend_->setDeletePath();
     archiveBackend_ = std::move(writableBackend_);
     writableBackend_ = std::move(newBackend);

src/xrpld/nodestore/detail/DatabaseRotatingImp.h

@@ -48,6 +48,8 @@ class DatabaseRotatingImp : public DatabaseRotating
         stop();
     }
 
+    // This function only has one caller in SHAMapStoreImp::run. The locking for
+    // this function will need to be rethought another caller is ever added.
     void
     rotateWithLock(
         std::function<std::unique_ptr<NodeStore::Backend>(
@@ -82,13 +84,7 @@ class DatabaseRotatingImp : public DatabaseRotating
 private:
     std::shared_ptr<Backend> writableBackend_;
     std::shared_ptr<Backend> archiveBackend_;
-    // This needs to be a recursive mutex because callbacks in `rotateWithLock`
-    // can call function that also lock the mutex. A current example of this is
-    // a callback from SHAMapStoreImp, which calls `clearCaches`. This
-    // `clearCaches` call eventually calls `fetchNodeObject` which tries to
-    // relock the mutex. It would be desirable to rewrite the code so the lock
-    // was not held during a callback.
-    mutable std::recursive_mutex mutex_;
+    mutable std::mutex mutex_;
 
     std::shared_ptr<NodeObject>
     fetchNodeObject(