Revert "SERVER-18277 Clarify locking of Client when accessing its stored OperationContext."

This reverts commit 5c2d133871b2ad2adf6c617364d036ca25261f2d.

9 files changed, 43 insertions, 78 deletions

diff --git a/src/mongo/db/client.cpp b/src/mongo/db/client.cpp
index a7d3de69b16..4b220a308dd 100644
--- a/src/mongo/db/client.cpp
+++ b/src/mongo/db/client.cpp
@@ -117,11 +117,14 @@ namespace mongo {
     void Client::setOperationContext(OperationContext* txn) {
         // We can only set the OperationContext once before resetting it.
         invariant(txn != NULL && _txn == NULL);
+
+        boost::unique_lock<SpinLock> uniqueLock(_lock);
         _txn = txn;
     }
 
     void Client::resetOperationContext() {
         invariant(_txn != NULL);
+        boost::unique_lock<SpinLock> uniqueLock(_lock);
         _txn = NULL;
     }
 
diff --git a/src/mongo/db/client.h b/src/mongo/db/client.h
index 597ed07dd70..63750b69523 100644
--- a/src/mongo/db/client.h
+++ b/src/mongo/db/client.h
@@ -86,29 +86,11 @@ namespace mongo {
         void lock() { _lock.lock(); }
         void unlock() { _lock.unlock(); }
 
-        /**
-         * Sets the active operation context on this client to "txn", which must be non-NULL.
-         *
-         * It is an error to call this method if there is already an operation context on Client.
-         * It is an error to call this on an unlocked client.
-         */
+        // Changes the currently active operation context on this client. There can only be one
+        // active OperationContext at a time.
         void setOperationContext(OperationContext* txn);
-
-        /**
-         * Clears the active operation context on this client.
-         *
-         * There must already be such a context set on this client.
-         * It is an error to call this on an unlocked client.
-         */
         void resetOperationContext();
-
-        /**
-         * Gets the operation context active on this client, or nullptr if there is no such context.
-         *
-         * It is an error to call this method on an unlocked client, or to use the value returned
-         * by this method while the client is not locked.
-         */
-        OperationContext* getOperationContext() { return _txn; }
+        const OperationContext* getOperationContext() const { return _txn; }
 
         // TODO(spencer): SERVER-10228 SERVER-14779 Remove this/move it fully into OperationContext.
         bool isInDirectClient() const { return _inDirectClient; }
diff --git a/src/mongo/db/clientlistplugin.cpp b/src/mongo/db/clientlistplugin.cpp
index 89f0f5788f1..9bd2ca06f3f 100644
--- a/src/mongo/db/clientlistplugin.cpp
+++ b/src/mongo/db/clientlistplugin.cpp
@@ -92,7 +92,7 @@ namespace {
                 invariant(client);
 
                 // Make the client stable
-                stdx::lock_guard<Client> lk(*client);
+                boost::unique_lock<Client> clientLock(*client);
                 const OperationContext* txn = client->getOperationContext();
                 if (!txn) continue;
 
@@ -205,7 +205,7 @@ namespace {
                 BSONObjBuilder b;
 
                 // Make the client stable
-                stdx::lock_guard<Client> lk(*client);
+                boost::unique_lock<Client> clientLock(*client);
 
                 client->reportState(b);
 
diff --git a/src/mongo/db/commands/current_op.cpp b/src/mongo/db/commands/current_op.cpp
index 402558814df..7d3957ea42f 100644
--- a/src/mongo/db/commands/current_op.cpp
+++ b/src/mongo/db/commands/current_op.cpp
@@ -106,7 +106,7 @@ namespace mongo {
 
                 invariant(client);
 
-                stdx::lock_guard<Client> lk(*client);
+                boost::unique_lock<Client> uniqueLock(*client);
                 const OperationContext* opCtx = client->getOperationContext();
 
                 if (!includeAll) {
@@ -132,7 +132,7 @@ namespace mongo {
 
                     // LockState
                     Locker::LockerInfo lockerInfo;
-                    opCtx->lockState()->getLockerInfo(&lockerInfo);
+                    client->getOperationContext()->lockState()->getLockerInfo(&lockerInfo);
                     fillLockerInfo(lockerInfo, infoBuilder);
                 }
                 else {
diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index 640c82063e0..57b3d780d11 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -147,7 +147,11 @@ namespace mongo {
     CurOp* CurOp::get(const OperationContext* opCtx) { return get(*opCtx); }
 
     CurOp* CurOp::get(const OperationContext& opCtx) {
-        return _curopStack(opCtx.getClient()).top();
+        return getFromClient(opCtx.getClient());
+    }
+
+    CurOp* CurOp::getFromClient(const Client* client) {
+        return _curopStack(client).top();
     }
 
     CurOp::CurOp(Client* client) : CurOp(client, &_curopStack(client)) {}
diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h
index 66201f53fdd..b3f96d66d3a 100644
--- a/src/mongo/db/curop.h
+++ b/src/mongo/db/curop.h
@@ -38,10 +38,12 @@
 #include "mongo/db/server_options.h"
 #include "mongo/platform/atomic_word.h"
 #include "mongo/util/concurrency/spin_lock.h"
+#include "mongo/util/net/hostandport.h"
 #include "mongo/util/progress_meter.h"
 #include "mongo/util/thread_safe_string.h"
 #include "mongo/util/time_support.h"
 
+
 namespace mongo {
 
     class Client;
@@ -195,6 +197,7 @@ namespace mongo {
     class CurOp {
         MONGO_DISALLOW_COPYING(CurOp);
     public:
+        static CurOp* getFromClient(const Client* client);
         static CurOp* get(const OperationContext* opCtx);
         static CurOp* get(const OperationContext& opCtx);
 
diff --git a/src/mongo/db/operation_context_impl.cpp b/src/mongo/db/operation_context_impl.cpp
index b57090e04b9..07fd8b2f6ad 100644
--- a/src/mongo/db/operation_context_impl.cpp
+++ b/src/mongo/db/operation_context_impl.cpp
@@ -83,13 +83,11 @@ namespace {
         StorageEngine* storageEngine = getGlobalServiceContext()->getGlobalStorageEngine();
         _recovery.reset(storageEngine->newRecoveryUnit());
 
-        stdx::lock_guard<Client> lk(*_client);
         _client->setOperationContext(this);
     }
 
     OperationContextImpl::~OperationContextImpl() {
         _locker->assertEmptyAndReset();
-        stdx::lock_guard<Client> lk(*_client);
         _client->resetOperationContext();
     }
 
diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp
index eb00449c625..eaef2f6f639 100644
--- a/src/mongo/db/service_context_d.cpp
+++ b/src/mongo/db/service_context_d.cpp
@@ -205,11 +205,11 @@ namespace mongo {
     }
 
     void ServiceContextMongoD::setKillAllOperations() {
-        stdx::lock_guard<stdx::mutex> clientLock(_mutex);
+        boost::lock_guard<boost::mutex> clientLock(_mutex);
         _globalKill = true;
-        for (const auto listener : _killOpListeners) {
+        for (size_t i = 0; i < _killOpListeners.size(); i++) {
             try {
-                listener->interruptAll();
+                _killOpListeners[i]->interruptAll();
             }
             catch (...) {
                 std::terminate();
@@ -223,38 +223,30 @@ namespace mongo {
 
     bool ServiceContextMongoD::_killOperationsAssociatedWithClientAndOpId_inlock(
             Client* client, unsigned int opId) {
-        OperationContext* opCtx = client->getOperationContext();
-        if (!opCtx) {
-            return false;
-        }
-        for( CurOp *k = CurOp::get(opCtx); k; k = k->parent() ) {
+        for( CurOp *k = CurOp::getFromClient(client); k; k = k->parent() ) {
             if ( k->opNum() != opId )
                 continue;
 
-            _killOperation_inlock(opCtx);
-            return true;
-        }
-        return false;
-    }
-
-    void ServiceContextMongoD::_killOperation_inlock(OperationContext* opCtx) {
-        for(CurOp *l = CurOp::get(opCtx); l; l = l->parent()) {
-            l->kill();
-        }
-
-        for (const auto listener : _killOpListeners) {
-            try {
-                listener->interrupt(opCtx->getOpID());
+            k->kill();
+            for( CurOp *l = CurOp::getFromClient(client); l; l = l->parent() ) {
+                l->kill();
             }
-            catch (...) {
-                std::terminate();
+
+            for (size_t i = 0; i < _killOpListeners.size(); i++) {
+                try {
+                    _killOpListeners[i]->interrupt(opId);
+                }
+                catch (...) {
+                    std::terminate();
+                }
             }
+            return true;
         }
+        return false;
     }
 
     bool ServiceContextMongoD::killOperation(unsigned int opId) {
         for (LockedClientsCursor cursor(this); Client* client = cursor.next();) {
-            stdx::lock_guard<Client> lk(*client);
             bool found = _killOperationsAssociatedWithClientAndOpId_inlock(client, opId);
             if (found) {
                 return true;
@@ -271,18 +263,17 @@ namespace mongo {
                 continue;
             }
 
-            stdx::lock_guard<Client> lk(*client);
-            OperationContext* toKill = client->getOperationContext();
-            if (!toKill) {
-                continue;
-            }
-
-            if (toKill->getOpID() == txn->getOpID()) {
+            if (CurOp::getFromClient(client)->opNum() == txn->getOpID()) {
                 // Don't kill ourself.
                 continue;
             }
 
-            _killOperation_inlock(toKill);
+            bool found = _killOperationsAssociatedWithClientAndOpId_inlock(
+                    client, CurOp::getFromClient(client)->opNum());
+            if (!found) {
+                warning() << "Attempted to kill operation " << CurOp::getFromClient(client)->opNum()
+                          << " but the opId changed";
+            }
         }
     }
 
@@ -291,7 +282,7 @@ namespace mongo {
     }
 
     void ServiceContextMongoD::registerKillOpListener(KillOpListenerInterface* listener) {
-        stdx::lock_guard<stdx::mutex> clientLock(_mutex);
+        boost::lock_guard<boost::mutex> clientLock(_mutex);
         _killOpListeners.push_back(listener);
     }
 
diff --git a/src/mongo/db/service_context_d.h b/src/mongo/db/service_context_d.h
index 1d707f33d63..9437d37aa07 100644
--- a/src/mongo/db/service_context_d.h
+++ b/src/mongo/db/service_context_d.h
@@ -80,24 +80,8 @@ namespace mongo {
 
     private:
 
-        /**
-         * Kills the active operation on "client" if that operation is associated with operation id
-         * "opId".
-         *
-         * Returns true if an operation was killed.
-         *
-         * Must only be called by a thread owning both this service context's mutex and the
-         * client's.
-         */
         bool _killOperationsAssociatedWithClientAndOpId_inlock(Client* client, unsigned int opId);
 
-        /**
-         * Kills the given operation.
-         *
-         * Caller must own the service context's _mutex.
-         */
-        void _killOperation_inlock(OperationContext* opCtx);
-
         bool _globalKill;
 
         // protected by parent class's _mutex