SERVER-54066 Retain stashed CollectionCatalog in the case of out-of-order destruction of AutoGetCollectionForRead*LockFree helpers

5 files changed, 38 insertions, 14 deletions

diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index e9edcce2b16..14440ef18b0 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -1085,21 +1085,29 @@ void CollectionCatalog::addResource(const ResourceId& rid, const std::string& en
 
 CollectionCatalogStasher::CollectionCatalogStasher(OperationContext* opCtx)
     : _opCtx(opCtx), _stashed(false) {}
+
 CollectionCatalogStasher::CollectionCatalogStasher(OperationContext* opCtx,
                                                    std::shared_ptr<const CollectionCatalog> catalog)
     : _opCtx(opCtx), _stashed(true) {
     invariant(catalog);
     CollectionCatalog::stash(_opCtx, std::move(catalog));
 }
-CollectionCatalogStasher::~CollectionCatalogStasher() {
-    reset();
-}
 
 CollectionCatalogStasher::CollectionCatalogStasher(CollectionCatalogStasher&& other)
     : _opCtx(other._opCtx), _stashed(other._stashed) {
     other._stashed = false;
 }
 
+CollectionCatalogStasher::~CollectionCatalogStasher() {
+    if (_opCtx->isLockFreeReadsOp()) {
+        // Leave the catalog stashed on the opCtx because there is another Stasher instance still
+        // using it.
+        return;
+    }
+
+    reset();
+}
+
 void CollectionCatalogStasher::stash(std::shared_ptr<const CollectionCatalog> catalog) {
     CollectionCatalog::stash(_opCtx, std::move(catalog));
     _stashed = true;
diff --git a/src/mongo/db/catalog/collection_catalog.h b/src/mongo/db/catalog/collection_catalog.h
index 750b95bd2f5..00a80372821 100644
--- a/src/mongo/db/catalog/collection_catalog.h
+++ b/src/mongo/db/catalog/collection_catalog.h
@@ -412,25 +412,31 @@ private:
  * RAII style object to stash a versioned CollectionCatalog on the OperationContext.
  * Calls to CollectionCatalog::get(OperationContext*) will return this instance.
  *
- * Unstashes the CollectionCatalog at destruction.
- *
- * It is not safe to nest usages of this type.
+ * Unstashes the CollectionCatalog at destruction if the OperationContext::isLockFreeReadsOp()
+ * flag is no longer set. This is handling for the nested Stasher use case.
  */
 class CollectionCatalogStasher {
 public:
     CollectionCatalogStasher(OperationContext* opCtx);
     CollectionCatalogStasher(OperationContext* opCtx,
                              std::shared_ptr<const CollectionCatalog> catalog);
+
+    /**
+     * Unstashes the catalog if _opCtx->isLockFreeReadsOp() is no longer set.
+     */
     ~CollectionCatalogStasher();
 
-    CollectionCatalogStasher(const CollectionCatalogStasher&) = delete;
+    /**
+     * Moves ownership of the stash to the new instance, and marks the old one unstashed.
+     */
     CollectionCatalogStasher(CollectionCatalogStasher&& other);
 
+    CollectionCatalogStasher(const CollectionCatalogStasher&) = delete;
     CollectionCatalogStasher& operator=(const CollectionCatalogStasher&) = delete;
     CollectionCatalogStasher& operator=(CollectionCatalogStasher&&) = delete;
 
     /**
-     * Stashes a new catalog on OperationContext
+     * Stashes 'catalog' on the _opCtx.
      */
     void stash(std::shared_ptr<const CollectionCatalog> catalog);
 
diff --git a/src/mongo/db/catalog_raii.h b/src/mongo/db/catalog_raii.h
index eddb77fc384..e675dc663cf 100644
--- a/src/mongo/db/catalog_raii.h
+++ b/src/mongo/db/catalog_raii.h
@@ -277,7 +277,7 @@ private:
     // Indicate that we are lock-free on code paths that can run either lock-free or locked for
     // different kinds of operations. Note: this class member is currently declared first so that it
     // destructs last, as a safety measure, but not because it is currently depended upon behavior.
-    boost::optional<LockFreeReadsBlock> _lockFreeReadsBlock;
+    LockFreeReadsBlock _lockFreeReadsBlock;
 
     Lock::GlobalLock _globalLock;
 
diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp
index db8d0c205e9..27d783e40e2 100644
--- a/src/mongo/db/db_raii.cpp
+++ b/src/mongo/db/db_raii.cpp
@@ -112,6 +112,10 @@ auto acquireCollectionAndConsistentSnapshot(
         // If this is a nested lock acquisition, then we already have a consistent stashed catalog
         // and snapshot from which to read and we can skip the below logic.
         if (isLockFreeReadSubOperation) {
+            // A consistent in-memory and on-disk state is already set up by a higher level AutoGet*
+            // instance. Save the catalog on this instance, to retain it against out-of-order
+            // AutoGet* destruction, and return early.
+            catalogStasher.stash(catalog);
             return collection;
         }
 
@@ -597,10 +601,10 @@ const NamespaceString& AutoGetCollectionForReadCommandMaybeLockFree::getNss() co
 AutoGetDbForReadLockFree::AutoGetDbForReadLockFree(OperationContext* opCtx,
                                                    StringData dbName,
                                                    Date_t deadline)
-    : _lockFreeReadsBlock(opCtx),
+    : _catalogStash(opCtx),
+      _lockFreeReadsBlock(opCtx),
       _globalLock(
-          opCtx, MODE_IS, deadline, Lock::InterruptBehavior::kThrow, true /* skipRSTLLock */),
-      _catalogStash(opCtx) {
+          opCtx, MODE_IS, deadline, Lock::InterruptBehavior::kThrow, true /* skipRSTLLock */) {
 
     // Type that pretends to be a Collection. It implements the minimal interface used by
     // acquireCollectionAndConsistentSnapshot(). We are tricking
diff --git a/src/mongo/db/db_raii.h b/src/mongo/db/db_raii.h
index cda4f145141..0fa2d3526ed 100644
--- a/src/mongo/db/db_raii.h
+++ b/src/mongo/db/db_raii.h
@@ -244,9 +244,12 @@ private:
         bool _isLockFreeReadSubOperation;
     };
 
+    // The CollectionCatalogStasher must outlive the LockFreeReadsBlock in the AutoGet* below.
+    // ~LockFreeReadsBlock clears a flag that the ~CollectionCatalogStasher checks.
+    CollectionCatalogStasher _catalogStash;
+
     boost::optional<AutoGetCollectionForReadBase<AutoGetCollectionLockFree, EmplaceHelper>>
         _autoGetCollectionForReadBase;
-    CollectionCatalogStasher _catalogStash;
 };
 
 /**
@@ -412,11 +415,14 @@ public:
                              Date_t deadline = Date_t::max());
 
 private:
+    // The CollectionCatalogStasher must outlive the LockFreeReadsBlock below. ~LockFreeReadsBlock
+    // clears a flag that the ~CollectionCatalogStasher checks.
+    CollectionCatalogStasher _catalogStash;
+
     // Sets a flag on the opCtx to inform subsequent code that the operation is running lock-free.
     LockFreeReadsBlock _lockFreeReadsBlock;
 
     Lock::GlobalLock _globalLock;
-    CollectionCatalogStasher _catalogStash;
 };
 
 /**