SERVER-51518 Adjust lock invariant in PrimaryOnlyService::lookupInst().

The RstlKillOpThread will interrupt OperationContexts for which the
global lock was taken in mode IX, S, or X. Callers holding the global
lock in one of these modes are also safe.

2 files changed, 71 insertions, 1 deletions

diff --git a/src/mongo/db/repl/primary_only_service.cpp b/src/mongo/db/repl/primary_only_service.cpp
index f2e472caf75..d6daba6eee1 100644
--- a/src/mongo/db/repl/primary_only_service.cpp
+++ b/src/mongo/db/repl/primary_only_service.cpp
@@ -512,7 +512,8 @@ boost::optional<std::shared_ptr<PrimaryOnlyService::Instance>> PrimaryOnlyServic
     OperationContext* opCtx, const InstanceID& id) {
     // If this operation is holding any database locks, then it must have opted into getting
     // interrupted at stepdown to prevent deadlocks.
-    invariant(!opCtx->lockState()->isLocked() || opCtx->shouldAlwaysInterruptAtStepDownOrUp());
+    invariant(!opCtx->lockState()->isLocked() || opCtx->shouldAlwaysInterruptAtStepDownOrUp() ||
+              opCtx->lockState()->wasGlobalLockTakenInModeConflictingWithWrites());
 
     stdx::unique_lock lk(_mutex);
     opCtx->waitForConditionOrInterrupt(
diff --git a/src/mongo/db/repl/primary_only_service_test.cpp b/src/mongo/db/repl/primary_only_service_test.cpp
index 780ceb29e55..29d051085e8 100644
--- a/src/mongo/db/repl/primary_only_service_test.cpp
+++ b/src/mongo/db/repl/primary_only_service_test.cpp
@@ -439,6 +439,75 @@ TEST_F(PrimaryOnlyServiceTest, LookupInstanceInterruptible) {
                        ErrorCodes::Interrupted);
 }
 
+TEST_F(PrimaryOnlyServiceTest, LookupInstanceHoldingISLock) {
+    // Make sure the instance doesn't complete before we try to look it up.
+    TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn);
+
+    auto opCtx = makeOperationContext();
+    auto instance =
+        TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0));
+    ASSERT(instance.get());
+    ASSERT_EQ(0, instance->getID());
+
+    {
+        Lock::GlobalLock lk(opCtx.get(), MODE_IS);
+
+        // The RstlKillOpThread would only interrupt a read operation if the OperationContext opted
+        // into always being interrupted.
+        opCtx->setAlwaysInterruptAtStepDownOrUp();
+        ASSERT_FALSE(opCtx->lockState()->wasGlobalLockTakenInModeConflictingWithWrites());
+
+        auto instance2 =
+            TestService::Instance::lookup(opCtx.get(), _service, BSON("_id" << 0)).get();
+        ASSERT_EQ(instance.get(), instance2.get());
+    }
+
+    TestServiceHangDuringInitialization.setMode(FailPoint::off);
+    instance->getCompletionFuture().get();
+}
+
+TEST_F(PrimaryOnlyServiceTest, LookupInstanceHoldingIXLock) {
+    // Make sure the instance doesn't complete before we try to look it up.
+    TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn);
+
+    auto opCtx = makeOperationContext();
+    auto instance =
+        TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0));
+    ASSERT(instance.get());
+    ASSERT_EQ(0, instance->getID());
+
+    {
+        Lock::GlobalLock lk(opCtx.get(), MODE_IX);
+        ASSERT_FALSE(opCtx->shouldAlwaysInterruptAtStepDownOrUp());
+        auto instance2 =
+            TestService::Instance::lookup(opCtx.get(), _service, BSON("_id" << 0)).get();
+        ASSERT_EQ(instance.get(), instance2.get());
+    }
+
+    TestServiceHangDuringInitialization.setMode(FailPoint::off);
+    instance->getCompletionFuture().get();
+}
+
+DEATH_TEST_F(PrimaryOnlyServiceTest,
+             LookupInstanceHoldingISLockWithoutAlwaysBeingInterruptible,
+             "invariant") {
+    // Make sure the instance doesn't complete before we try to look it up.
+    TestServiceHangDuringInitialization.setMode(FailPoint::alwaysOn);
+
+    auto opCtx = makeOperationContext();
+    auto instance =
+        TestService::Instance::getOrCreate(opCtx.get(), _service, BSON("_id" << 0 << "state" << 0));
+    ASSERT(instance.get());
+    ASSERT_EQ(0, instance->getID());
+
+    {
+        Lock::GlobalLock lk(opCtx.get(), MODE_IS);
+        ASSERT_FALSE(opCtx->shouldAlwaysInterruptAtStepDownOrUp());
+        ASSERT_FALSE(opCtx->lockState()->wasGlobalLockTakenInModeConflictingWithWrites());
+        TestService::Instance::lookup(opCtx.get(), _service, BSON("_id" << 0));
+    }
+}
+
 TEST_F(PrimaryOnlyServiceTest, GetOrCreateInstanceInterruptible) {
     stepDown();
     // Make sure the service stays in state kRebuilding so that getOrCreate() has to try to wait on