SERVER-46865 Make collMod not take database MODE_X lock

(cherry picked from commit e2e5505c2975bfd8a355229c8e85cd0c560d3a0e)

6 files changed, 26 insertions, 18 deletions

diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index 8d0b0f286bf..afb64d12476 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -272,10 +272,12 @@ Status _collModInternal(OperationContext* opCtx,
                         const BSONObj& cmdObj,
                         BSONObjBuilder* result) {
     StringData dbName = nss.db();
-    AutoGetDb autoDb(opCtx, dbName, MODE_X);
-    Database* const db = autoDb.getDb();
-    Collection* coll =
-        db ? CollectionCatalog::get(opCtx).lookupCollectionByNamespace(opCtx, nss) : nullptr;
+    AutoGetCollection autoColl(opCtx, nss, MODE_X, AutoGetCollection::ViewMode::kViewsPermitted);
+    Lock::CollectionLock systemViewsLock(
+        opCtx, NamespaceString(dbName, NamespaceString::kSystemDotViewsCollectionName), MODE_X);
+
+    Database* const db = autoColl.getDb();
+    Collection* coll = autoColl.getCollection();
 
     CurOpFailpointHelpers::waitWhileFailPointEnabled(
         &hangAfterDatabaseLock, opCtx, "hangAfterDatabaseLock", []() {}, false, nss);
diff --git a/src/mongo/db/commands/run_aggregate.cpp b/src/mongo/db/commands/run_aggregate.cpp
index 104c5da8cba..e70e57fe4da 100644
--- a/src/mongo/db/commands/run_aggregate.cpp
+++ b/src/mongo/db/commands/run_aggregate.cpp
@@ -268,12 +268,16 @@ StatusWith<StringMap<ExpressionContext::ResolvedNamespace>> resolveInvolvedNames
         return {StringMap<ExpressionContext::ResolvedNamespace>()};
     }
 
-    // We intentionally do not drop and reacquire our DB lock after resolving the view definition in
-    // order to prevent the definition for any view namespaces we've already resolved from changing.
-    // This is necessary to prevent a cycle from being formed among the view definitions cached in
-    // 'resolvedNamespaces' because we won't re-resolve a view namespace we've already encountered.
-    AutoGetDb autoDb(opCtx, request.getNamespaceString().db(), MODE_IS);
-    Database* const db = autoDb.getDb();
+    // We intentionally do not drop and reacquire our system.views collection lock after resolving
+    // the view definition in order to prevent the definition for any view namespaces we've already
+    // resolved from changing. This is necessary to prevent a cycle from being formed among the view
+    // definitions cached in 'resolvedNamespaces' because we won't re-resolve a view namespace we've
+    // already encountered.
+    AutoGetCollection autoColl(opCtx,
+                               NamespaceString(request.getNamespaceString().db(),
+                                               NamespaceString::kSystemDotViewsCollectionName),
+                               MODE_IS);
+    Database* const db = autoColl.getDb();
     ViewCatalog* viewCatalog = db ? ViewCatalog::get(db) : nullptr;
 
     std::deque<NamespaceString> involvedNamespacesQueue(pipelineInvolvedNamespaces.begin(),
diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp
index c42dee1d8a4..6b9ee596ba7 100644
--- a/src/mongo/db/op_observer_impl.cpp
+++ b/src/mongo/db/op_observer_impl.cpp
@@ -686,7 +686,7 @@ void OpObserverImpl::onCollMod(OperationContext* opCtx,
 
     // Make sure the UUID values in the Collection metadata, the Collection object, and the UUID
     // catalog are all present and equal.
-    invariant(opCtx->lockState()->isDbLockedForMode(nss.db(), MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(nss, MODE_X));
     auto databaseHolder = DatabaseHolder::get(opCtx);
     auto db = databaseHolder->getDb(opCtx, nss.db());
     // Some unit tests call the op observer on an unregistered Database.
diff --git a/src/mongo/db/op_observer_impl_test.cpp b/src/mongo/db/op_observer_impl_test.cpp
index b66303045a6..4408c7494c4 100644
--- a/src/mongo/db/op_observer_impl_test.cpp
+++ b/src/mongo/db/op_observer_impl_test.cpp
@@ -282,7 +282,7 @@ TEST_F(OpObserverTest, CollModWithCollectionOptionsAndTTLInfo) {
 
     // Write to the oplog.
     {
-        AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X);
+        AutoGetCollection autoColl(opCtx.get(), nss, MODE_X);
         WriteUnitOfWork wunit(opCtx.get());
         opObserver.onCollMod(opCtx.get(), nss, uuid, collModCmd, oldCollOpts, ttlInfo);
         wunit.commit();
@@ -331,7 +331,7 @@ TEST_F(OpObserverTest, CollModWithOnlyCollectionOptions) {
 
     // Write to the oplog.
     {
-        AutoGetDb autoDb(opCtx.get(), nss.db(), MODE_X);
+        AutoGetCollection autoColl(opCtx.get(), nss, MODE_X);
         WriteUnitOfWork wunit(opCtx.get());
         opObserver.onCollMod(opCtx.get(), nss, uuid, collModCmd, oldCollOpts, boost::none);
         wunit.commit();
diff --git a/src/mongo/db/views/view_catalog.cpp b/src/mongo/db/views/view_catalog.cpp
index 9481a83c400..38031abc2c1 100644
--- a/src/mongo/db/views/view_catalog.cpp
+++ b/src/mongo/db/views/view_catalog.cpp
@@ -438,7 +438,9 @@ Status ViewCatalog::modifyView(OperationContext* opCtx,
                                const NamespaceString& viewName,
                                const NamespaceString& viewOn,
                                const BSONArray& pipeline) {
-    invariant(opCtx->lockState()->isDbLockedForMode(viewName.db(), MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(viewName, MODE_X));
+    invariant(opCtx->lockState()->isCollectionLockedForMode(
+        NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName), MODE_X));
 
     stdx::lock_guard<Latch> lk(_mutex);
 
diff --git a/src/mongo/db/views/view_catalog_test.cpp b/src/mongo/db/views/view_catalog_test.cpp
index c186573e437..ded055683e1 100644
--- a/src/mongo/db/views/view_catalog_test.cpp
+++ b/src/mongo/db/views/view_catalog_test.cpp
@@ -133,8 +133,8 @@ public:
                       const NamespaceString& viewName,
                       const NamespaceString& viewOn,
                       const BSONArray& pipeline) {
-        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_X);
-        Lock::CollectionLock collLock(operationContext(), viewName, MODE_IX);
+        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_IX);
+        Lock::CollectionLock collLock(operationContext(), viewName, MODE_X);
         Lock::CollectionLock sysCollLock(
             operationContext(),
             NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName),
@@ -592,8 +592,8 @@ TEST_F(ViewCatalogFixture, LookupRIDAfterModifyRollback) {
         wunit.commit();
     }
     {
-        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_X);
-        Lock::CollectionLock collLock(operationContext(), viewName, MODE_IX);
+        Lock::DBLock dbLock(operationContext(), viewName.db(), MODE_IX);
+        Lock::CollectionLock collLock(operationContext(), viewName, MODE_X);
         Lock::CollectionLock sysCollLock(
             operationContext(),
             NamespaceString(viewName.db(), NamespaceString::kSystemDotViewsCollectionName),