Add fix

BedrockServer.cpp

@@ -1721,6 +1721,7 @@ void BedrockServer::_status(unique_ptr<BedrockCommand>& command) {
         // Coalesce all of the peer data into one value to return or return
         // an error message if we timed out getting the peerList data.
         list<string> peerList;
+        // This blocks during state change
         list<STable> peerData = getPeerInfo();
         for (const STable& peerTable : peerData) {
             peerList.push_back(SComposeJSONObject(peerTable));

sqlitecluster/SQLite.cpp

@@ -669,13 +669,20 @@ bool SQLite::prepare(uint64_t* transactionID, string* transactionhash) {
     static const size_t deleteLimit = 10;
     if (minJournalEntry < oldestCommitToKeep) {
         auto startUS = STimeNow();
+        shared_lock<shared_mutex> lock(_sharedData.writeLock);
         string query = "DELETE FROM " + _journalName + " WHERE id < " + SQ(oldestCommitToKeep) + " LIMIT " + SQ(deleteLimit);
         SASSERT(!SQuery(_db, "Deleting oldest journal rows", query));
         size_t deletedCount = sqlite3_changes(_db);
         SINFO("Removed " << deletedCount << " rows from journal " << _journalName << ", oldestToKeep: " << oldestCommitToKeep << ", count:"
                << commitCount << ", limit: " << _maxJournalSize << ", in " << (STimeNow() - startUS) << "us.");
     }
 
+    // So let's say that a replicate thread is running the above. Neither the write or commit lock is held.
+
+    // Let's say another thread calls `exclusiveLockDB`. It grabs the commit lock.
+    // We grab the write lock above. Other thread waits. We release writeLock, it acquires it. Writes are now blocked.
+    // We attempot to grab commitLock below. We block.
+
     // We lock this here, so that we can guarantee the order in which commits show up in the database.
     if (!_mutexLocked) {
         auto start = STimeNow();

sqlitecluster/SQLiteNode.cpp

@@ -493,7 +493,12 @@ void SQLiteNode::_sendOutstandingTransactions(const set<uint64_t>& commitOnlyIDs
 }
 
 list<STable> SQLiteNode::getPeerInfo() const {
-    shared_lock<decltype(_stateMutex)> sharedLock(_stateMutex);
+    // This does not lock _stateMutex. It follows the rule in `SQLiteNode.h` that says:
+    //  * Alternatively, a public `const` method that is a simple getter for an atomic property can skip the lock.
+    // peer->getData is atomic internally, so we can treat `peer->getData()` as a simple getter for an atomic property.
+    // _peerList is also `const` and so we can iterate this list safely regardless of the lock.
+    // This makes this function a slightly more complex getter for an atomic property, but it's still safe to skip
+    // The state lock here.
     list<STable> peerData;
     for (SQLitePeer* peer : _peerList) {
         peerData.emplace_back(peer->getData());

sqlitecluster/SQLiteNode.h

@@ -25,16 +25,16 @@
  * Rules for maintaining SQLiteNode methods so that atomicity works as intended.
  *
  * No non-const members should be publicly exposed.
- * Any public method that is `const` must shared_lock<>(nodeMutex).
+ * Any public method that is `const` must shared_lock<>(_stateMutex).
  * Alternatively, a public `const` method that is a simple getter for an atomic property can skip the lock.
- * Any public method that is non-const must unique_lock<>(nodeMutex) before changing any internal state, and must hold
+ * Any public method that is non-const must unique_lock<>(_stateMutex) before changing any internal state, and must hold
  * this lock until it is done changing state to make this method's changes atomic.
  * Any private methods must not call public methods.
- * Any private methods must not lock nodeMutex (for recursion reasons).
+ * Any private methods must not lock _stateMutex (for recursion reasons).
  * Any public methods must not call other public methods.
  *
  * `_replicate` is a special exception because it runs in multiple threads internally. It needs to handle locking if it
- * changes any internal state (and it calls `changeState`, which does).
+ * changes any internal state (and it calls `changeState`, which it does).
  *
  */
 

sqlitecluster/SQLitePeer.cpp

@@ -225,6 +225,7 @@ void SQLitePeer::getCommit(uint64_t& count, string& hashString) const {
 }
 
 STable SQLitePeer::getData() const {
+    lock_guard<decltype(peerMutex)> lock(peerMutex);
     // Add all of our standard stuff.
     STable result({
         {"name", name},