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

As a side-effect, clean up operation killing in ServiceContextMongoD.

1 files changed, 34 insertions, 25 deletions

diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp
index eaef2f6f639..eb00449c625 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() {
-        boost::lock_guard<boost::mutex> clientLock(_mutex);
+        stdx::lock_guard<stdx::mutex> clientLock(_mutex);
         _globalKill = true;
-        for (size_t i = 0; i < _killOpListeners.size(); i++) {
+        for (const auto listener : _killOpListeners) {
             try {
-                _killOpListeners[i]->interruptAll();
+                listener->interruptAll();
             }
             catch (...) {
                 std::terminate();
@@ -223,30 +223,38 @@ namespace mongo {
 
     bool ServiceContextMongoD::_killOperationsAssociatedWithClientAndOpId_inlock(
             Client* client, unsigned int opId) {
-        for( CurOp *k = CurOp::getFromClient(client); k; k = k->parent() ) {
+        OperationContext* opCtx = client->getOperationContext();
+        if (!opCtx) {
+            return false;
+        }
+        for( CurOp *k = CurOp::get(opCtx); k; k = k->parent() ) {
             if ( k->opNum() != opId )
                 continue;
 
-            k->kill();
-            for( CurOp *l = CurOp::getFromClient(client); l; l = l->parent() ) {
-                l->kill();
-            }
-
-            for (size_t i = 0; i < _killOpListeners.size(); i++) {
-                try {
-                    _killOpListeners[i]->interrupt(opId);
-                }
-                catch (...) {
-                    std::terminate();
-                }
-            }
+            _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());
+            }
+            catch (...) {
+                std::terminate();
+            }
+        }
+    }
+
     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;
@@ -263,17 +271,18 @@ namespace mongo {
                 continue;
             }
 
-            if (CurOp::getFromClient(client)->opNum() == txn->getOpID()) {
-                // Don't kill ourself.
+            stdx::lock_guard<Client> lk(*client);
+            OperationContext* toKill = client->getOperationContext();
+            if (!toKill) {
                 continue;
             }
 
-            bool found = _killOperationsAssociatedWithClientAndOpId_inlock(
-                    client, CurOp::getFromClient(client)->opNum());
-            if (!found) {
-                warning() << "Attempted to kill operation " << CurOp::getFromClient(client)->opNum()
-                          << " but the opId changed";
+            if (toKill->getOpID() == txn->getOpID()) {
+                // Don't kill ourself.
+                continue;
             }
+
+            _killOperation_inlock(toKill);
         }
     }
 
@@ -282,7 +291,7 @@ namespace mongo {
     }
 
     void ServiceContextMongoD::registerKillOpListener(KillOpListenerInterface* listener) {
-        boost::lock_guard<boost::mutex> clientLock(_mutex);
+        stdx::lock_guard<stdx::mutex> clientLock(_mutex);
         _killOpListeners.push_back(listener);
     }