diff options
Diffstat (limited to 'src/mongo/db')
-rw-r--r-- | src/mongo/db/default_baton.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/operation_context.cpp | 96 | ||||
-rw-r--r-- | src/mongo/db/operation_context.h | 19 | ||||
-rw-r--r-- | src/mongo/db/service_context.cpp | 22 |
4 files changed, 30 insertions, 114 deletions
diff --git a/src/mongo/db/default_baton.cpp b/src/mongo/db/default_baton.cpp index 8aeef1bec64..1154d377ccb 100644 --- a/src/mongo/db/default_baton.cpp +++ b/src/mongo/db/default_baton.cpp @@ -57,11 +57,8 @@ void DefaultBaton::detachImpl() noexcept { { stdx::lock_guard<stdx::mutex> lk(_mutex); - { - stdx::lock_guard<Client> lk(*_opCtx->getClient()); - invariant(_opCtx->getBaton().get() == this); - _opCtx->setBaton(nullptr); - } + invariant(_opCtx->getBaton().get() == this); + _opCtx->setBaton(nullptr); _opCtx = nullptr; _hasIngressSocket = false; diff --git a/src/mongo/db/operation_context.cpp b/src/mongo/db/operation_context.cpp index 271ea547fa2..2e9e4c0a00e 100644 --- a/src/mongo/db/operation_context.cpp +++ b/src/mongo/db/operation_context.cpp @@ -246,49 +246,26 @@ Status OperationContext::checkForInterruptNoAssert() noexcept { return Status::OK(); } -// Theory of operation for waitForConditionOrInterruptNoAssertUntil and markKilled: -// -// An operation indicates to potential killers that it is waiting on a condition variable by setting -// _waitMutex and _waitCV, while holding the lock on its parent Client. It then unlocks its Client, -// unblocking any killers, which are required to have locked the Client before calling markKilled. -// -// When _waitMutex and _waitCV are set, killers must lock _waitMutex before setting the _killCode, -// and must signal _waitCV before releasing _waitMutex. Unfortunately, they must lock _waitMutex -// without holding a lock on Client to avoid a deadlock with callers of -// waitForConditionOrInterruptNoAssertUntil(). So, in the event that _waitMutex is set, the killer -// increments _numKillers, drops the Client lock, acquires _waitMutex and then re-acquires the -// Client lock. We know that the Client, its OperationContext and _waitMutex will remain valid -// during this period because the caller of waitForConditionOrInterruptNoAssertUntil will not return -// while _numKillers > 0 and will not return until it has itself reacquired _waitMutex. Instead, -// that caller will keep waiting on _waitCV until _numKillers drops to 0. + +// waitForConditionOrInterruptNoAssertUntil returns when: // -// In essence, when _waitMutex is set, _killCode is guarded by _waitMutex and _waitCV, but when -// _waitMutex is not set, it is guarded by the Client spinlock. Changing _waitMutex is itself -// guarded by the Client spinlock and _numKillers. +// Normal condvar wait criteria: +// - cv is notified +// - deadline is passed // -// When _numKillers does drop to 0, the waiter will null out _waitMutex and _waitCV. +// OperationContext kill criteria: +// - _deadline is passed (artificial deadline or maxTimeMS) +// - markKilled is called (killOp) // -// This implementation adds a minimum of two spinlock acquire-release pairs to every condition -// variable wait. +// Baton criteria: +// - _baton is notified (someone's queuing work for the baton) +// - _baton::run returns (timeout fired / networking is ready / socket disconnected) StatusWith<stdx::cv_status> OperationContext::waitForConditionOrInterruptNoAssertUntil( stdx::condition_variable& cv, stdx::unique_lock<stdx::mutex>& m, Date_t deadline) noexcept { invariant(getClient()); - { - stdx::lock_guard<Client> clientLock(*getClient()); - invariant(!_waitMutex); - invariant(!_waitCV); - invariant(_waitThread == kNoWaiterThread); - invariant(0 == _numKillers); - - // This interrupt check must be done while holding the client lock, so as not to race with a - // concurrent caller of markKilled. - auto status = checkForInterruptNoAssert(); - if (!status.isOK()) { - return status; - } - _waitMutex = m.mutex(); - _waitCV = &cv; - _waitThread = stdx::this_thread::get_id(); + + if (auto status = checkForInterruptNoAssert(); !status.isOK()) { + return status; } // If the maxTimeNeverTimeOut failpoint is set, behave as though the operation's deadline does @@ -314,22 +291,10 @@ StatusWith<stdx::cv_status> OperationContext::waitForConditionOrInterruptNoAsser cv, m, deadline, _baton.get()); }(); - // Continue waiting on cv until no other thread is attempting to kill this one. - Waitable::wait(_baton.get(), getServiceContext()->getPreciseClockSource(), cv, m, [this] { - stdx::lock_guard<Client> clientLock(*getClient()); - if (0 == _numKillers) { - _waitMutex = nullptr; - _waitCV = nullptr; - _waitThread = kNoWaiterThread; - return true; - } - return false; - }); - - auto status = checkForInterruptNoAssert(); - if (!status.isOK()) { + if (auto status = checkForInterruptNoAssert(); !status.isOK()) { return status; } + if (opHasDeadline && waitStatus == stdx::cv_status::timeout && deadline == getDeadline()) { // It's possible that the system clock used in stdx::condition_variable::wait_until // is slightly ahead of the FastClock used in checkForInterrupt. In this case, @@ -352,33 +317,8 @@ void OperationContext::markKilled(ErrorCodes::Error killCode) { log() << "operation was interrupted because a client disconnected"; } - stdx::unique_lock<stdx::mutex> lkWaitMutex; - - // If we have a _waitMutex, it means this opCtx is currently blocked in - // waitForConditionOrInterrupt. - // - // From there, we also know which thread is actually doing that waiting (it's recorded in - // _waitThread). If that thread isn't our thread, it's necessary to do the regular numKillers - // song and dance mentioned in waitForConditionOrInterrupt. - // - // If it is our thread, we know that we're currently inside that call to - // waitForConditionOrInterrupt and are being invoked by a callback run from Baton->run. And - // that means we don't need to deal with the waitMutex and waitCV (because we're running - // callbacks, which means run is returning, which means we'll be checking _killCode in the near - // future). - if (_waitMutex && stdx::this_thread::get_id() != _waitThread) { - invariant(++_numKillers > 0); - getClient()->unlock(); - ON_BLOCK_EXIT([this] { - getClient()->lock(); - invariant(--_numKillers >= 0); - }); - lkWaitMutex = stdx::unique_lock<stdx::mutex>{*_waitMutex}; - } - _killCode.compareAndSwap(ErrorCodes::OK, killCode); - if (lkWaitMutex && _numKillers == 0) { - invariant(_waitCV); - _waitCV->notify_all(); + if (_killCode.compareAndSwap(ErrorCodes::OK, killCode) == ErrorCodes::OK) { + _baton->notify(); } } diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h index 56a363aab3e..ffa9678d7a0 100644 --- a/src/mongo/db/operation_context.h +++ b/src/mongo/db/operation_context.h @@ -446,27 +446,8 @@ private: // once from OK to some kill code. AtomicWord<ErrorCodes::Error> _killCode{ErrorCodes::OK}; - // A transport Baton associated with the operation. The presence of this object implies that a - // client thread is doing it's own async networking by blocking on it's own thread. BatonHandle _baton; - // If non-null, _waitMutex and _waitCV are the (mutex, condition variable) pair that the - // operation is currently waiting on inside a call to waitForConditionOrInterrupt...(). - // - // _waitThread is the calling thread's thread id. - // - // All access guarded by the Client's lock. - stdx::mutex* _waitMutex = nullptr; - stdx::condition_variable* _waitCV = nullptr; - stdx::thread::id _waitThread; - - // If _waitMutex and _waitCV are non-null, this is the number of threads in a call to markKilled - // actively attempting to kill the operation. If this value is non-zero, the operation is inside - // waitForConditionOrInterrupt...() and must stay there until _numKillers reaches 0. - // - // All access guarded by the Client's lock. - int _numKillers = 0; - WriteConcernOptions _writeConcern; // The timepoint at which this operation exceeds its time limit. diff --git a/src/mongo/db/service_context.cpp b/src/mongo/db/service_context.cpp index ee26756a642..ac8937dbc6c 100644 --- a/src/mongo/db/service_context.cpp +++ b/src/mongo/db/service_context.cpp @@ -248,21 +248,20 @@ ServiceContext::UniqueOperationContext ServiceContext::makeOperationContext(Clie opCtx->setRecoveryUnit(std::make_unique<RecoveryUnitNoop>(), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } - { - stdx::lock_guard<Client> lk(*client); - client->setOperationContext(opCtx.get()); - } + // The baton must be attached before attaching to a client if (_transportLayer) { _transportLayer->makeBaton(opCtx.get()); } else { makeBaton(opCtx.get()); } + { + stdx::lock_guard<Client> lk(*client); + client->setOperationContext(opCtx.get()); + } return UniqueOperationContext(opCtx.release()); }; void ServiceContext::OperationContextDeleter::operator()(OperationContext* opCtx) const { - opCtx->getBaton()->detach(); - auto client = opCtx->getClient(); if (client->session()) { _numCurrentOps.subtractAndFetch(1); @@ -272,6 +271,8 @@ void ServiceContext::OperationContextDeleter::operator()(OperationContext* opCtx stdx::lock_guard<Client> lk(*client); client->resetOperationContext(); } + opCtx->getBaton()->detach(); + onDestroy(opCtx, service->_clientObservers); delete opCtx; } @@ -413,13 +414,10 @@ void ServiceContext::ServiceContextDeleter::operator()(ServiceContext* service) } BatonHandle ServiceContext::makeBaton(OperationContext* opCtx) const { - auto baton = std::make_shared<DefaultBaton>(opCtx); + invariant(!opCtx->getBaton()); - { - stdx::lock_guard<Client> lk(*opCtx->getClient()); - invariant(!opCtx->getBaton()); - opCtx->setBaton(baton); - } + auto baton = std::make_shared<DefaultBaton>(opCtx); + opCtx->setBaton(baton); return baton; } |