SERVER-14062 StorageEngine::cleanShutdown should not accept OpCtx

There should not be active OperationContext when cleanShutdown is called,
because it should be run under a global lock and should not be acquiring
locks or performing recoverable operations.

This change is necessary for the removal of the OperationContext registry
and ensuring there is one OperationContext per client.

17 files changed, 47 insertions, 38 deletions

diff --git a/src/mongo/db/commands/fsync.cpp b/src/mongo/db/commands/fsync.cpp
index fb4e41bb4b7..3be2914801a 100644
--- a/src/mongo/db/commands/fsync.cpp
+++ b/src/mongo/db/commands/fsync.cpp
@@ -152,7 +152,7 @@ namespace mongo {
     void FSyncLockThread::doRealWork() {
         SimpleMutex::scoped_lock lkf(filesLockedFsync);
 
-        OperationContextImpl txn;   // XXX?
+        OperationContextImpl txn;
         ScopedTransaction transaction(&txn, MODE_X);
         Lock::GlobalWrite global(txn.lockState()); // No WriteUnitOfWork needed
 
diff --git a/src/mongo/db/concurrency/lock_state.h b/src/mongo/db/concurrency/lock_state.h
index 9aa540dee6e..8ec2b0d837e 100644
--- a/src/mongo/db/concurrency/lock_state.h
+++ b/src/mongo/db/concurrency/lock_state.h
@@ -255,6 +255,7 @@ namespace mongo {
         Lock::ScopedLock* _scopedLk;
     };
 
+    typedef LockerImpl<false> DefaultLockerImpl;
     typedef LockerImpl<true> MMAPV1LockerImpl;
 
 
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 5d0199aef9a..a787f347d00 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -45,6 +45,7 @@
 #include "mongo/db/commands.h"
 #include "mongo/db/commands/fsync.h"
 #include "mongo/db/concurrency/d_concurrency.h"
+#include "mongo/db/concurrency/lock_state.h"
 #include "mongo/db/concurrency/write_conflict_exception.h"
 #include "mongo/db/currentop_command.h"
 #include "mongo/db/db.h"
@@ -1024,10 +1025,7 @@ namespace {
         return shutdownInProgress.loadRelaxed() != 0;
     }
 
-    static void shutdownServer(OperationContext* txn) {
-        // Must hold global lock to get to here
-        invariant(txn->lockState()->isW());
-
+    static void shutdownServer() {
         log(LogComponent::kNetwork) << "shutdown: going to close listening sockets..." << endl;
         ListeningSockets::get()->closeAll();
 
@@ -1039,7 +1037,7 @@ namespace {
         boost::thread close_socket_thread( stdx::bind(MessagingPort::closeAllSockets, 0) );
 
         StorageEngine* storageEngine = getGlobalEnvironment()->getGlobalStorageEngine();
-        storageEngine->cleanShutdown(txn);
+        storageEngine->cleanShutdown();
     }
 
     void exitCleanly(ExitCode code) {
@@ -1055,20 +1053,30 @@ namespace {
             invariant(false);
         }
 
-        OperationContextImpl txn;
-
         getGlobalEnvironment()->setKillAllOperations();
 
         repl::getGlobalReplicationCoordinator()->shutdown();
 
-        ScopedTransaction transaction(&txn, MODE_X);
-        Lock::GlobalWrite lk(txn.lockState());
+        // We should always be able to acquire the global lock at shutdown.
+        //
+        // TODO: This call chain uses the locker directly, because we do not want to start an
+        // operation context, which also instantiates a recovery unit. Also, using the
+        // lockGlobalBegin/lockGlobalComplete sequence, we avoid taking the flush lock. This will
+        // all go away if we start acquiring the global/flush lock as part of ScopedTransaction.
+        DefaultLockerImpl globalLocker(0);
+        LockResult result = globalLocker.lockGlobalBegin(MODE_X);
+        if (result == LOCK_WAITING) {
+            result = globalLocker.lockGlobalComplete(UINT_MAX);
+        }
+
+        invariant(LOCK_OK == result);
+
         log() << "now exiting" << endl;
 
         // Execute the graceful shutdown tasks, such as flushing the outstanding journal 
         // and data files, close sockets, etc.
         try {
-            shutdownServer(&txn);
+            shutdownServer();
         }
         catch (const DBException& ex) {
             severe() << "shutdown failed with DBException " << ex;
@@ -1082,6 +1090,7 @@ namespace {
             severe() << "shutdown failed with exception";
             std::terminate();
         }
+
         dbexit( code );
     }
 
diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h
index fcdd3724c62..70dba10d2fa 100644
--- a/src/mongo/db/operation_context.h
+++ b/src/mongo/db/operation_context.h
@@ -176,7 +176,6 @@ namespace mongo {
         bool _ended;
     };
 
-    class Database;
 
     /**
      * RAII-style class to mark the scope of a transaction. ScopedTransactions may be nested.
diff --git a/src/mongo/db/storage/devnull/devnull_kv_engine.h b/src/mongo/db/storage/devnull/devnull_kv_engine.h
index 6539fb9014a..473dcebfbab 100644
--- a/src/mongo/db/storage/devnull/devnull_kv_engine.h
+++ b/src/mongo/db/storage/devnull/devnull_kv_engine.h
@@ -92,7 +92,7 @@ namespace mongo {
             return std::vector<std::string>();
         }
 
-        virtual void cleanShutdown(OperationContext* txn) {};
+        virtual void cleanShutdown() {};
 
     private:
         boost::shared_ptr<void> _catalogInfo;
diff --git a/src/mongo/db/storage/in_memory/in_memory_engine.h b/src/mongo/db/storage/in_memory/in_memory_engine.h
index c4f821dbb9d..0c913cf8296 100644
--- a/src/mongo/db/storage/in_memory/in_memory_engine.h
+++ b/src/mongo/db/storage/in_memory/in_memory_engine.h
@@ -78,7 +78,7 @@ namespace mongo {
             return Status::OK();
         }
 
-        virtual void cleanShutdown(OperationContext* txn) {};
+        virtual void cleanShutdown() {};
 
         std::vector<std::string> getAllIdents( OperationContext* opCtx ) const;
     private:
diff --git a/src/mongo/db/storage/kv/kv_engine.h b/src/mongo/db/storage/kv/kv_engine.h
index 1ff79bca388..9f343e1238c 100644
--- a/src/mongo/db/storage/kv/kv_engine.h
+++ b/src/mongo/db/storage/kv/kv_engine.h
@@ -119,7 +119,7 @@ namespace mongo {
          *
          * There is intentionally no uncleanShutdown().
          */
-        virtual void cleanShutdown(OperationContext* txn) = 0;
+        virtual void cleanShutdown() = 0;
 
         /**
          * The destructor will never be called from mongod, but may be called from tests.
diff --git a/src/mongo/db/storage/kv/kv_storage_engine.cpp b/src/mongo/db/storage/kv/kv_storage_engine.cpp
index 1928cc6008e..a1ec8b1e53c 100644
--- a/src/mongo/db/storage/kv/kv_storage_engine.cpp
+++ b/src/mongo/db/storage/kv/kv_storage_engine.cpp
@@ -151,7 +151,7 @@ namespace mongo {
 
     }
 
-    void KVStorageEngine::cleanShutdown(OperationContext* txn) {
+    void KVStorageEngine::cleanShutdown() {
 
         for ( DBMap::const_iterator it = _dbs.begin(); it != _dbs.end(); ++it ) {
             delete it->second;
@@ -161,7 +161,7 @@ namespace mongo {
         _catalog.reset( NULL );
         _catalogRecordStore.reset( NULL );
 
-        _engine->cleanShutdown(txn);
+        _engine->cleanShutdown();
         // intentionally not deleting _engine
     }
 
diff --git a/src/mongo/db/storage/kv/kv_storage_engine.h b/src/mongo/db/storage/kv/kv_storage_engine.h
index 669d163520b..5e52ca1c3a0 100644
--- a/src/mongo/db/storage/kv/kv_storage_engine.h
+++ b/src/mongo/db/storage/kv/kv_storage_engine.h
@@ -88,7 +88,7 @@ namespace mongo {
                                        bool preserveClonedFilesOnFailure = false,
                                        bool backupOriginalFiles = false );
 
-        virtual void cleanShutdown(OperationContext* txn);
+        virtual void cleanShutdown();
 
         // ------ kv ------
 
diff --git a/src/mongo/db/storage/mmap_v1/dur.cpp b/src/mongo/db/storage/mmap_v1/dur.cpp
index 782b0c7b328..5b5a26292d8 100644
--- a/src/mongo/db/storage/mmap_v1/dur.cpp
+++ b/src/mongo/db/storage/mmap_v1/dur.cpp
@@ -288,12 +288,13 @@ namespace mongo {
             return true;
         }
 
-        void DurableImpl::commitAndStopDurThread(OperationContext* txn) {
-            // This is only called by clean shutdown and the global lock must be held to ensure
-            // there will not be any more writes.
-            invariant(txn->lockState()->isW());
+        void DurableImpl::commitAndStopDurThread() {
+            NotifyAll::When when = commitJob._notify.now();
+
+            // There is always just one waiting anyways
+            flushRequested.notify_one();
+            commitJob._notify.waitFor(when);
 
-            commitNow(txn);
             shutdownRequested.store(1);
         }
 
diff --git a/src/mongo/db/storage/mmap_v1/dur.h b/src/mongo/db/storage/mmap_v1/dur.h
index 0eb7dd61dfc..404ae5d923a 100644
--- a/src/mongo/db/storage/mmap_v1/dur.h
+++ b/src/mongo/db/storage/mmap_v1/dur.h
@@ -142,7 +142,7 @@ namespace mongo {
              *
              * Must be called under the global X lock.
              */
-            virtual void commitAndStopDurThread(OperationContext* txn) = 0;
+            virtual void commitAndStopDurThread() = 0;
 
             /** Declare write intent for an int */
             inline int& writingInt(int& d) { return *static_cast<int*>(writingPtr( &d, sizeof(d))); }
@@ -204,7 +204,7 @@ namespace mongo {
             bool commitIfNeeded(OperationContext* txn);
             void syncDataAndTruncateJournal(OperationContext* txn) {}
             bool isDurable() const { return false; }
-            void commitAndStopDurThread(OperationContext* txn) { }
+            void commitAndStopDurThread() { }
         };
 
         class DurableImpl : public DurableInterface {
@@ -219,7 +219,7 @@ namespace mongo {
             bool commitIfNeeded(OperationContext* txn);
             void syncDataAndTruncateJournal(OperationContext* txn);
             bool isDurable() const { return true; }
-            void commitAndStopDurThread(OperationContext* txn);
+            void commitAndStopDurThread();
         };
 
     } // namespace dur
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
index 08355b74fe6..2bd02577c3b 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.cpp
@@ -403,7 +403,7 @@ namespace {
         return getDur().isDurable();
     }
 
-    void MMAPV1Engine::cleanShutdown(OperationContext* txn) {
+    void MMAPV1Engine::cleanShutdown() {
         // wait until file preallocation finishes
         // we would only hang here if the file_allocator code generates a
         // synchronous signal, which we don't expect
@@ -413,8 +413,7 @@ namespace {
         if (storageGlobalParams.dur) {
             log() << "shutdown: final commit..." << endl;
 
-            getDur().commitAndStopDurThread(txn);
-
+            getDur().commitAndStopDurThread();
             flushAllFiles(true);
         }
 
diff --git a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
index df5ab7930ef..b7fab00d07b 100644
--- a/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
+++ b/src/mongo/db/storage/mmap_v1/mmap_v1_engine.h
@@ -64,11 +64,11 @@ namespace mongo {
 
         virtual bool isDurable() const;
 
-        Status closeDatabase(OperationContext* txn, const StringData& db );
+        virtual Status closeDatabase(OperationContext* txn, const StringData& db);
 
-        Status dropDatabase(OperationContext* txn, const StringData& db );
+        virtual Status dropDatabase(OperationContext* txn, const StringData& db);
 
-        void cleanShutdown(OperationContext* txn);
+        virtual void cleanShutdown();
 
     private:
         static void _listDatabases( const std::string& directory,
diff --git a/src/mongo/db/storage/rocks/rocks_engine.h b/src/mongo/db/storage/rocks/rocks_engine.h
index a31a9aedf2d..ca8ef727325 100644
--- a/src/mongo/db/storage/rocks/rocks_engine.h
+++ b/src/mongo/db/storage/rocks/rocks_engine.h
@@ -109,7 +109,7 @@ namespace mongo {
             return Status::OK();
         }
 
-        virtual void cleanShutdown(OperationContext* txn) {}
+        virtual void cleanShutdown() {}
 
         // rocks specific api
 
diff --git a/src/mongo/db/storage/storage_engine.h b/src/mongo/db/storage/storage_engine.h
index 5e8f623c188..7de2681019c 100644
--- a/src/mongo/db/storage/storage_engine.h
+++ b/src/mongo/db/storage/storage_engine.h
@@ -166,7 +166,7 @@ namespace mongo {
          *
          * There is intentionally no uncleanShutdown().
          */
-        virtual void cleanShutdown(OperationContext* txn) {}
+        virtual void cleanShutdown() = 0;
 
     protected:
         /**
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
index ee4a61d3e12..2b429a5250f 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.cpp
@@ -166,7 +166,7 @@ namespace mongo {
 
     WiredTigerKVEngine::~WiredTigerKVEngine() {
         if (_conn) {
-            cleanShutdown(NULL); // our impl doesn't use the OperationContext
+            cleanShutdown();
         }
 
         _sizeStorer.reset( NULL );
@@ -175,7 +175,7 @@ namespace mongo {
 
     }
 
-    void WiredTigerKVEngine::cleanShutdown(OperationContext* txn) {
+    void WiredTigerKVEngine::cleanShutdown() {
         log() << "WiredTigerKVEngine shutting down";
         syncSizeInfo(true);
         if (_conn) {
@@ -236,7 +236,7 @@ namespace mongo {
             _sizeStorer->storeInto( &session, _sizeStorerUri );
             invariantWTOK( s->commit_transaction( s, NULL ) );
         }
-        catch ( const WriteConflictException& de ) {
+        catch (const WriteConflictException&) {
             // ignore, it means someone else is doing it
         }
     }
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
index 2d69f6877f9..e8f6b9e148e 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_kv_engine.h
@@ -101,7 +101,7 @@ namespace mongo {
 
         std::vector<std::string> getAllIdents( OperationContext* opCtx ) const;
 
-        virtual void cleanShutdown(OperationContext* txn);
+        virtual void cleanShutdown();
 
         // wiredtiger specific
         // Calls WT_CONNECTION::reconfigure on the underlying WT_CONNECTION