SERVER-14995 Move _killPending from CurOp to OperationContext.

Also, limit the lifetime of OperationContext in MongoD so that it goes out of
scope before sending a reply to the client.  This is necessary so that
operations do not appear in the currentOp command result after the server
sends a response to the client.

19 files changed, 84 insertions, 65 deletions

diff --git a/src/mongo/db/clientlistplugin.cpp b/src/mongo/db/clientlistplugin.cpp
index 6eb854c739d..42499f5c116 100644
--- a/src/mongo/db/clientlistplugin.cpp
+++ b/src/mongo/db/clientlistplugin.cpp
@@ -208,12 +208,12 @@ namespace {
                 b.appendBool("active", static_cast<bool>(txn));
                 if (txn) {
                     b.append("opid", txn->getOpID());
-
-                    // CurOp
-                    if (CurOp::get(txn)) {
-                        CurOp::get(txn)->reportState(&b);
+                    if (txn->isKillPending()) {
+                        b.append("killPending", true);
                     }
 
+                    CurOp::get(txn)->reportState(&b);
+
                     // LockState
                     if (txn->lockState()) {
                         StringBuilder ss;
diff --git a/src/mongo/db/commands/current_op.cpp b/src/mongo/db/commands/current_op.cpp
index b26c811d2d8..d28cb91874e 100644
--- a/src/mongo/db/commands/current_op.cpp
+++ b/src/mongo/db/commands/current_op.cpp
@@ -124,11 +124,12 @@ namespace mongo {
                 infoBuilder.appendBool("active", static_cast<bool>(opCtx));
                 if (opCtx) {
                     infoBuilder.append("opid", opCtx->getOpID());
-                    // CurOp
-                    if (CurOp::get(opCtx)) {
-                        CurOp::get(opCtx)->reportState(&infoBuilder);
+                    if (opCtx->isKillPending()) {
+                        infoBuilder.append("killPending", true);
                     }
 
+                    CurOp::get(opCtx)->reportState(&infoBuilder);
+
                     // LockState
                     Locker::LockerInfo lockerInfo;
                     opCtx->lockState()->getLockerInfo(&lockerInfo);
diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index 4c41b5bd365..f896b1524b3 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -165,7 +165,6 @@ namespace mongo {
         _maxTimeTracker.reset();
         _message = "";
         _progressMeter.finished();
-        _killPending.store(0);
         _numYields = 0;
         _expectedLatencyMs = 0;
         _op = 0;
@@ -268,16 +267,9 @@ namespace mongo {
             }
         }
 
-        if( killPending() )
-            builder->append("killPending", true);
-
         builder->append( "numYields" , _numYields );
     }
 
-    void CurOp::kill() {
-        _killPending.store(1);
-    }
-
     void CurOp::setMaxTimeMicros(uint64_t maxTimeMicros) {
         _maxTimeMicros = maxTimeMicros;
 
diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h
index f58ee576370..03467cd0cb9 100644
--- a/src/mongo/db/curop.h
+++ b/src/mongo/db/curop.h
@@ -242,9 +242,6 @@ namespace mongo {
         /**
          * Checks whether this operation has been running longer than its time limit.  Returns
          * false if not, or if the operation has no time limit.
-         *
-         * Note that KillCurrentOp objects are responsible for interrupting CurOp objects that
-         * have exceeded their allotted time; CurOp objects do not interrupt themselves.
          */
         bool maxTimeHasExpired();
 
@@ -298,12 +295,9 @@ namespace mongo {
         std::string getMessage() const { return _message.toString(); }
         ProgressMeter& getProgressMeter() { return _progressMeter; }
         CurOp *parent() const { return _parent; }
-        void kill(); 
-        bool killPendingStrict() const { return _killPending.load(); }
-        bool killPending() const { return _killPending.loadRelaxed(); }
         void yielded() { _numYields++; }
         int numYields() const { return _numYields; }
-        
+
         long long getExpectedLatencyMs() const { return _expectedLatencyMs; }
         void setExpectedLatencyMs( long long latency ) { _expectedLatencyMs = latency; }
 
@@ -336,7 +330,6 @@ namespace mongo {
         OpDebug _debug;
         ThreadSafeString _message;
         ProgressMeter _progressMeter;
-        AtomicInt32 _killPending;
         int _numYields;
         
         // this is how much "extra" time a query might take
diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 61678070dcf..f430cceb675 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -163,9 +163,13 @@ namespace mongo {
                     break;
                 }
 
-                OperationContextImpl txn;
                 DbResponse dbresponse;
-                assembleResponse(&txn, m, dbresponse, port->remote());
+                {
+                    OperationContextImpl txn;
+                    assembleResponse(&txn, m, dbresponse, port->remote());
+                    // txn must go out of scope here so that the operation cannot show up in
+                    // currentOp results after the response reaches the client.
+                }
 
                 if ( dbresponse.response ) {
                     port->reply(m, *dbresponse.response, dbresponse.responseTo);
diff --git a/src/mongo/db/operation_context.cpp b/src/mongo/db/operation_context.cpp
index 5c87bee7220..703eca38030 100644
--- a/src/mongo/db/operation_context.cpp
+++ b/src/mongo/db/operation_context.cpp
@@ -43,4 +43,12 @@ namespace mongo {
         return _client;
     }
 
+    void OperationContext::markKilled() {
+        _killPending.store(1);
+    }
+
+    bool OperationContext::isKillPending() const {
+        return _killPending.loadRelaxed();
+    }
+
 }  // namespace mongo
diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h
index e3c28ef2c05..291fd3b6923 100644
--- a/src/mongo/db/operation_context.h
+++ b/src/mongo/db/operation_context.h
@@ -33,6 +33,7 @@
 #include "mongo/db/storage/recovery_unit.h"
 #include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/write_concern_options.h"
+#include "mongo/platform/atomic_word.h"
 #include "mongo/util/decorable.h"
 
 namespace mongo {
@@ -106,12 +107,14 @@ namespace mongo {
         // --- operation level info? ---
 
         /**
-         * If the thread is not interrupted, returns Status::OK(), otherwise returns the cause
-         * for the interruption. The throw variant returns a user assertion corresponding to the
-         * interruption status.
+         * Raises a UserAssertion if this operation is in a killed state.
          */
-        virtual void checkForInterrupt() const = 0;
-        virtual Status checkForInterruptNoAssert() const = 0;
+        virtual void checkForInterrupt() = 0;
+
+        /**
+         * Returns Status::OK() unless this operation is in a killed state.
+         */
+        virtual Status checkForInterruptNoAssert() = 0;
 
         /**
          * Delegates to CurOp, but is included here to break dependencies.
@@ -167,6 +170,25 @@ namespace mongo {
          */
         virtual bool writesAreReplicated() const = 0;
 
+        /**
+         * Marks this operation as killed.
+         *
+         * Subsequent calls to checkForInterrupt and checkForInterruptNoAssert by the thread
+         * executing the operation will indicate that the operation has been killed.
+         *
+         * May be called by any thread that has locked the Client owning this operation context,
+         * or by the thread executing on behalf of this operation context.
+         */
+        void markKilled();
+
+        /**
+         * Returns true if markKilled has been called on this operation context.
+         *
+         * May be called by any thread that has locked the Client owning this operation context,
+         * or by the thread executing on behalf of this operation context.
+         */
+        bool isKillPending() const;
+
     protected:
         OperationContext(Client* client,
                          unsigned int opId,
@@ -183,6 +205,7 @@ namespace mongo {
         // safe to access _locker in the destructor of OperationContext.
         Locker* const _locker;
 
+        AtomicInt32 _killPending{0};
         WriteConcernOptions _writeConcern;
     };
 
diff --git a/src/mongo/db/operation_context_impl.cpp b/src/mongo/db/operation_context_impl.cpp
index 6eccefce706..3f048b598d2 100644
--- a/src/mongo/db/operation_context_impl.cpp
+++ b/src/mongo/db/operation_context_impl.cpp
@@ -175,7 +175,7 @@ namespace {
 
     } // namespace
 
-    void OperationContextImpl::checkForInterrupt() const {
+    void OperationContextImpl::checkForInterrupt() {
         // We cannot interrupt operation, while it's inside of a write unit of work, because logOp
         // cannot handle being iterrupted.
         if (lockState()->inAWriteUnitOfWork()) return;
@@ -183,14 +183,14 @@ namespace {
         uassertStatusOK(checkForInterruptNoAssert());
     }
 
-    Status OperationContextImpl::checkForInterruptNoAssert() const {
+    Status OperationContextImpl::checkForInterruptNoAssert() {
         if (getGlobalServiceContext()->getKillAllOperations()) {
             return Status(ErrorCodes::InterruptedAtShutdown, "interrupted at shutdown");
         }
 
         CurOp* curOp = CurOp::get(this);
         if (curOp->maxTimeHasExpired()) {
-            curOp->kill();
+            markKilled();
             return Status(ErrorCodes::ExceededTimeLimit, "operation exceeded time limit");
         }
 
@@ -199,11 +199,11 @@ namespace {
                 log() << "set pending kill on "
                       << (curOp->parent() ? "nested" : "top-level")
                       << " op " << getOpID() << ", for checkForInterruptFail";
-                curOp->kill();
+                markKilled();
             }
         }
 
-        if (curOp->killPending()) {
+        if (isKillPending()) {
             return Status(ErrorCodes::Interrupted, "operation was interrupted");
         }
 
diff --git a/src/mongo/db/operation_context_impl.h b/src/mongo/db/operation_context_impl.h
index 362455581fd..e2065a1ba84 100644
--- a/src/mongo/db/operation_context_impl.h
+++ b/src/mongo/db/operation_context_impl.h
@@ -56,8 +56,8 @@ namespace mongo {
 
         virtual uint64_t getRemainingMaxTimeMicros() const override;
 
-        virtual void checkForInterrupt() const override;
-        virtual Status checkForInterruptNoAssert() const override;
+        virtual void checkForInterrupt() override;
+        virtual Status checkForInterruptNoAssert() override;
 
         virtual bool isPrimaryFor( StringData ns ) override;
 
diff --git a/src/mongo/db/operation_context_noop.h b/src/mongo/db/operation_context_noop.h
index 254f553445b..cd73c42dee0 100644
--- a/src/mongo/db/operation_context_noop.h
+++ b/src/mongo/db/operation_context_noop.h
@@ -88,8 +88,8 @@ namespace mongo {
             return &_pm;
         }
 
-        virtual void checkForInterrupt() const override { }
-        virtual Status checkForInterruptNoAssert() const override {
+        virtual void checkForInterrupt() override { }
+        virtual Status checkForInterruptNoAssert() override {
             return Status::OK();
         }
 
diff --git a/src/mongo/db/repl/operation_context_repl_mock.cpp b/src/mongo/db/repl/operation_context_repl_mock.cpp
index 10d2e69beca..78587bda2f3 100644
--- a/src/mongo/db/repl/operation_context_repl_mock.cpp
+++ b/src/mongo/db/repl/operation_context_repl_mock.cpp
@@ -51,11 +51,11 @@ namespace repl {
 
     OperationContextReplMock::~OperationContextReplMock() = default;
 
-    void OperationContextReplMock::checkForInterrupt() const {
+    void OperationContextReplMock::checkForInterrupt() {
         uassertStatusOK(checkForInterruptNoAssert());
     }
 
-    Status OperationContextReplMock::checkForInterruptNoAssert() const {
+    Status OperationContextReplMock::checkForInterruptNoAssert() {
         if (!_checkForInterruptStatus.isOK()) {
             return _checkForInterruptStatus;
         }
diff --git a/src/mongo/db/repl/operation_context_repl_mock.h b/src/mongo/db/repl/operation_context_repl_mock.h
index 6c5e76b046f..c79580f1482 100644
--- a/src/mongo/db/repl/operation_context_repl_mock.h
+++ b/src/mongo/db/repl/operation_context_repl_mock.h
@@ -50,9 +50,9 @@ namespace repl {
         OperationContextReplMock(Client* client, unsigned int opNum);
         virtual ~OperationContextReplMock();
 
-        virtual void checkForInterrupt() const override;
+        virtual void checkForInterrupt() override;
 
-        virtual Status checkForInterruptNoAssert() const override;
+        virtual Status checkForInterruptNoAssert() override;
 
         void setCheckForInterruptStatus(Status status);
 
diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h
index 9ce8a4e3a1f..d76d0fa3299 100644
--- a/src/mongo/db/repl/replication_coordinator.h
+++ b/src/mongo/db/repl/replication_coordinator.h
@@ -181,7 +181,7 @@ namespace repl {
          * ErrorCodes::ShutdownInProgress if we are mid-shutdown
          * ErrorCodes::Interrupted if the operation was killed with killop()
          */
-        virtual StatusAndDuration awaitReplication(const OperationContext* txn,
+        virtual StatusAndDuration awaitReplication(OperationContext* txn,
                                                    const OpTime& opTime,
                                                    const WriteConcernOptions& writeConcern) = 0;
 
@@ -190,7 +190,7 @@ namespace repl {
          * performed on the client associated with "txn".
          */
         virtual StatusAndDuration awaitReplicationOfLastOpForClient(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const WriteConcernOptions& writeConcern) = 0;
 
         /**
@@ -298,7 +298,7 @@ namespace repl {
          * Note: getDuration() on the returned ReadAfterOpTimeResponse will only be valid if
          * its didWait() method returns true.
          */
-        virtual ReadAfterOpTimeResponse waitUntilOpTime(const OperationContext* txn,
+        virtual ReadAfterOpTimeResponse waitUntilOpTime(OperationContext* txn,
                                                         const ReadAfterOpTimeArgs& settings) = 0;
 
         /**
diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp
index 79a47f52a7f..2cd4e8c2f1f 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.cpp
+++ b/src/mongo/db/repl/replication_coordinator_impl.cpp
@@ -767,7 +767,7 @@ namespace {
     }
 
     ReadAfterOpTimeResponse ReplicationCoordinatorImpl::waitUntilOpTime(
-            const OperationContext* txn,
+            OperationContext* txn,
             const ReadAfterOpTimeArgs& settings) {
         const auto& ts = settings.getOpTime();
         const auto& timeout = settings.getTimeout();
@@ -1035,7 +1035,7 @@ namespace {
     }
 
     ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorImpl::awaitReplication(
-            const OperationContext* txn,
+            OperationContext* txn,
             const OpTime& opTime,
             const WriteConcernOptions& writeConcern) {
         Timer timer;
@@ -1045,7 +1045,7 @@ namespace {
 
     ReplicationCoordinator::StatusAndDuration
             ReplicationCoordinatorImpl::awaitReplicationOfLastOpForClient(
-                    const OperationContext* txn,
+                    OperationContext* txn,
                     const WriteConcernOptions& writeConcern) {
         Timer timer;
         boost::unique_lock<boost::mutex> lock(_mutex);
@@ -1060,7 +1060,7 @@ namespace {
     ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorImpl::_awaitReplication_inlock(
             const Timer* timer,
             boost::unique_lock<boost::mutex>* lock,
-            const OperationContext* txn,
+            OperationContext* txn,
             const OpTime& opTime,
             const WriteConcernOptions& writeConcern) {
 
diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h
index c015fdbfb97..d8dabc082eb 100644
--- a/src/mongo/db/repl/replication_coordinator_impl.h
+++ b/src/mongo/db/repl/replication_coordinator_impl.h
@@ -123,12 +123,12 @@ namespace repl {
         virtual void interruptAll();
 
         virtual ReplicationCoordinator::StatusAndDuration awaitReplication(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const OpTime& opTime,
                 const WriteConcernOptions& writeConcern);
 
         virtual ReplicationCoordinator::StatusAndDuration awaitReplicationOfLastOpForClient(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const WriteConcernOptions& writeConcern);
 
         virtual Status stepDown(OperationContext* txn,
@@ -160,7 +160,7 @@ namespace repl {
         virtual OpTime getMyLastOptime() const override;
 
         virtual ReadAfterOpTimeResponse waitUntilOpTime(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const ReadAfterOpTimeArgs& settings) override;
 
         virtual OID getElectionId() override;
@@ -496,7 +496,7 @@ namespace repl {
         ReplicationCoordinator::StatusAndDuration _awaitReplication_inlock(
                 const Timer* timer,
                 boost::unique_lock<boost::mutex>* lock,
-                const OperationContext* txn,
+                OperationContext* txn,
                 const OpTime& opTime,
                 const WriteConcernOptions& writeConcern);
 
diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp
index 9a4e069a747..c5c76b8b7d3 100644
--- a/src/mongo/db/repl/replication_coordinator_mock.cpp
+++ b/src/mongo/db/repl/replication_coordinator_mock.cpp
@@ -89,7 +89,7 @@ namespace repl {
     void ReplicationCoordinatorMock::clearSyncSourceBlacklist() {}
 
     ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorMock::awaitReplication(
-            const OperationContext* txn,
+            OperationContext* txn,
             const OpTime& opTime,
             const WriteConcernOptions& writeConcern) {
         // TODO
@@ -98,7 +98,7 @@ namespace repl {
 
     ReplicationCoordinator::StatusAndDuration
             ReplicationCoordinatorMock::awaitReplicationOfLastOpForClient(
-                    const OperationContext* txn,
+                    OperationContext* txn,
                     const WriteConcernOptions& writeConcern) {
         return StatusAndDuration(Status::OK(), Milliseconds(0));
     }
@@ -150,7 +150,7 @@ namespace repl {
     }
 
     ReadAfterOpTimeResponse ReplicationCoordinatorMock::waitUntilOpTime(
-            const OperationContext* txn,
+            OperationContext* txn,
             const ReadAfterOpTimeArgs& settings) {
         return ReadAfterOpTimeResponse();
     }
diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h
index 216a60fcafe..d1a73ea8110 100644
--- a/src/mongo/db/repl/replication_coordinator_mock.h
+++ b/src/mongo/db/repl/replication_coordinator_mock.h
@@ -65,12 +65,12 @@ namespace repl {
         virtual void clearSyncSourceBlacklist();
 
         virtual ReplicationCoordinator::StatusAndDuration awaitReplication(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const OpTime& opTime,
                 const WriteConcernOptions& writeConcern);
 
         virtual ReplicationCoordinator::StatusAndDuration awaitReplicationOfLastOpForClient(
-                const OperationContext* txn,
+                OperationContext* txn,
                 const WriteConcernOptions& writeConcern);
 
         virtual Status stepDown(OperationContext* txn, 
@@ -102,7 +102,7 @@ namespace repl {
         virtual OpTime getMyLastOptime() const;
 
         virtual ReadAfterOpTimeResponse waitUntilOpTime(
-                        const OperationContext* txn,
+                        OperationContext* txn,
                         const ReadAfterOpTimeArgs& settings) override;
 
         virtual OID getElectionId();
diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp
index a9b22a986fb..de3b418e694 100644
--- a/src/mongo/db/service_context_d.cpp
+++ b/src/mongo/db/service_context_d.cpp
@@ -37,7 +37,6 @@
 #include "mongo/base/init.h"
 #include "mongo/base/initializer.h"
 #include "mongo/db/client.h"
-#include "mongo/db/curop.h"
 #include "mongo/db/op_observer.h"
 #include "mongo/db/operation_context_impl.h"
 #include "mongo/db/storage/storage_engine.h"
@@ -46,6 +45,7 @@
 #include "mongo/db/storage_options.h"
 #include "mongo/scripting/engine.h"
 #include "mongo/stdx/memory.h"
+#include "mongo/stdx/mutex.h"
 #include "mongo/util/log.h"
 #include "mongo/util/map_util.h"
 #include "mongo/util/mongoutils/str.h"
@@ -235,9 +235,7 @@ namespace mongo {
     }
 
     void ServiceContextMongoD::_killOperation_inlock(OperationContext* opCtx) {
-        for(CurOp *l = CurOp::get(opCtx); l; l = l->parent()) {
-            l->kill();
-        }
+        opCtx->markKilled();
 
         for (const auto listener : _killOpListeners) {
             try {
diff --git a/src/mongo/s/d_migrate.cpp b/src/mongo/s/d_migrate.cpp
index b3aa9312038..90e9815364e 100644
--- a/src/mongo/s/d_migrate.cpp
+++ b/src/mongo/s/d_migrate.cpp
@@ -2492,7 +2492,7 @@ namespace {
          * Returns true if the majority of the nodes and the nodes corresponding to the given
          * writeConcern (if not empty) have applied till the specified lastOp.
          */
-        bool opReplicatedEnough(const OperationContext* txn,
+        bool opReplicatedEnough(OperationContext* txn,
                                 const repl::OpTime& lastOpApplied,
                                 const WriteConcernOptions& writeConcern) {
             WriteConcernOptions majorityWriteConcern;