SERVER-39106 GlobalLock acquisition should throw if its ticket acquisition times out due to max lock timeout

3 files changed, 46 insertions, 12 deletions

diff --git a/src/mongo/db/concurrency/d_concurrency_test.cpp b/src/mongo/db/concurrency/d_concurrency_test.cpp
index d98151fd2a5..ebe6619558b 100644
--- a/src/mongo/db/concurrency/d_concurrency_test.cpp
+++ b/src/mongo/db/concurrency/d_concurrency_test.cpp
@@ -1584,6 +1584,31 @@ TEST_F(DConcurrencyTestFixture, TicketReacquireCanBeInterrupted) {
     ASSERT_THROWS_CODE(result.get(), AssertionException, ErrorCodes::Interrupted);
 }
 
+TEST_F(DConcurrencyTestFixture,
+       GlobalLockTimeoutDueToTicketOutageShouldThrowIfMaxLockTimeoutIsEffective) {
+    auto clients = makeKClientsWithLockers(1);
+    auto opCtx = clients[0].second.get();
+
+    UseGlobalThrottling throttle(opCtx, 0);
+
+    boost::optional<Lock::GlobalLock> globalLock;
+    opCtx->lockState()->setMaxLockTimeout(Milliseconds(100));
+    ASSERT_THROWS_CODE(
+        globalLock.emplace(opCtx, MODE_IX), AssertionException, ErrorCodes::LockTimeout);
+}
+
+TEST_F(DConcurrencyTestFixture,
+       GlobalLockTimeoutDueToTicketOutageShouldFailSilentlyIfDeadlineIsEffective) {
+    auto clients = makeKClientsWithLockers(1);
+    auto opCtx = clients[0].second.get();
+
+    UseGlobalThrottling throttle(opCtx, 0);
+
+    Lock::GlobalLock globalLock(
+        opCtx, MODE_IX, Date_t::now() + Milliseconds(100), Lock::InterruptBehavior::kThrow);
+    ASSERT(!globalLock.isLocked());
+}
+
 TEST_F(DConcurrencyTestFixture, GlobalLockInInterruptedContextThrowsEvenWhenUncontested) {
     auto clients = makeKClientsWithLockers(1);
     auto opCtx = clients[0].second.get();
diff --git a/src/mongo/db/concurrency/lock_state.cpp b/src/mongo/db/concurrency/lock_state.cpp
index 11a4028e7fb..27ec015445e 100644
--- a/src/mongo/db/concurrency/lock_state.cpp
+++ b/src/mongo/db/concurrency/lock_state.cpp
@@ -313,15 +313,14 @@ void LockerImpl::reacquireTicket(OperationContext* opCtx) {
     if (clientState != kInactive)
         return;
 
-    auto acquireTicketResult = _acquireTicket(opCtx, _modeForTicket, Date_t::max());
+    auto deadline = _maxLockTimeout ? Date_t::now() + *_maxLockTimeout : Date_t::max();
+    auto acquireTicketResult = _acquireTicket(opCtx, _modeForTicket, deadline);
     uassert(ErrorCodes::LockTimeout,
             str::stream() << "Unable to acquire ticket with mode '" << _modeForTicket
                           << "' within a max lock request timeout of '"
-                          << _maxLockTimeout.get()
+                          << *_maxLockTimeout
                           << "' milliseconds.",
-            acquireTicketResult == LOCK_OK || !_maxLockTimeout);
-    // If no deadline is specified we should always get a ticket.
-    invariant(acquireTicketResult == LOCK_OK);
+            acquireTicketResult == LOCK_OK || _uninterruptibleLocksRequested);
 }
 
 LockResult LockerImpl::_acquireTicket(OperationContext* opCtx, LockMode mode, Date_t deadline) {
@@ -330,10 +329,6 @@ LockResult LockerImpl::_acquireTicket(OperationContext* opCtx, LockMode mode, Da
     if (holder) {
         _clientState.store(reader ? kQueuedReader : kQueuedWriter);
 
-        if (_maxLockTimeout && !_uninterruptibleLocksRequested) {
-            deadline = std::min(deadline, Date_t::now() + _maxLockTimeout.get());
-        }
-
         // If the ticket wait is interrupted, restore the state of the client.
         auto restoreStateOnErrorGuard = makeGuard([&] { _clientState.store(kInactive); });
 
@@ -352,7 +347,19 @@ LockResult LockerImpl::_acquireTicket(OperationContext* opCtx, LockMode mode, Da
 LockResult LockerImpl::_lockGlobalBegin(OperationContext* opCtx, LockMode mode, Date_t deadline) {
     dassert(isLocked() == (_modeForTicket != MODE_NONE));
     if (_modeForTicket == MODE_NONE) {
-        auto acquireTicketResult = _acquireTicket(opCtx, mode, deadline);
+        auto lockTimeoutDate =
+            _maxLockTimeout ? Date_t::now() + _maxLockTimeout.get() : Date_t::max();
+        auto useLockTimeout = lockTimeoutDate < deadline;
+        auto acquireTicketResult =
+            _acquireTicket(opCtx, mode, useLockTimeout ? lockTimeoutDate : deadline);
+        if (useLockTimeout) {
+            uassert(ErrorCodes::LockTimeout,
+                    str::stream() << "Unable to acquire ticket with mode '" << _modeForTicket
+                                  << "' within a max lock request timeout of '"
+                                  << *_maxLockTimeout
+                                  << "' milliseconds.",
+                    acquireTicketResult == LOCK_OK || _uninterruptibleLocksRequested);
+        }
         if (acquireTicketResult != LOCK_OK) {
             return acquireTicketResult;
         }
diff --git a/src/mongo/db/concurrency/lock_state.h b/src/mongo/db/concurrency/lock_state.h
index 38303aed76a..20364646ee5 100644
--- a/src/mongo/db/concurrency/lock_state.h
+++ b/src/mongo/db/concurrency/lock_state.h
@@ -292,8 +292,10 @@ private:
     void _releaseTicket();
 
     /**
-     * Acquires a ticket for the Locker under 'mode'. Returns LOCK_TIMEOUT if it cannot acquire a
-     * ticket within 'deadline'.
+     * Acquires a ticket for the Locker under 'mode'.
+     * Returns LOCK_OK      if a ticket is successfully acquired.
+     *         LOCK_TIMEOUT if it cannot acquire a ticket within 'deadline'.
+     * It may throw an exception when it is interrupted.
      */
     LockResult _acquireTicket(OperationContext* opCtx, LockMode mode, Date_t deadline);