SERVER-13961 Remove some usages of Lock::isLockedXXX and dbtemprelease

These are the most obvious usages, which do not require any refactoring.

14 files changed, 84 insertions, 74 deletions

diff --git a/src/mongo/db/catalog/database_holder.cpp b/src/mongo/db/catalog/database_holder.cpp
index 1978c395600..67d9282a894 100644
--- a/src/mongo/db/catalog/database_holder.cpp
+++ b/src/mongo/db/catalog/database_holder.cpp
@@ -38,14 +38,23 @@
 #include "mongo/db/d_concurrency.h"
 #include "mongo/db/operation_context_impl.h"
 #include "mongo/db/storage/mmap_v1/dur.h"
+#include "mongo/util/file_allocator.h"
+
 
 namespace mongo {
 
-    Database* DatabaseHolder::getOrCreate(OperationContext* txn, const string& ns, const string& path, bool& justCreated) {
-        string dbname = _todb( ns );
+    Database* DatabaseHolder::getOrCreate(
+                OperationContext* txn, const string& ns, const string& path, bool& justCreated) {
+
+        const string dbname = _todb( ns );
+        invariant(txn->lockState()->isAtLeastReadLocked(dbname));
+
+        if (txn->lockState()->hasAnyWriteLock() && FileAllocator::get()->hasFailed()) {
+            uassert(17507, "Can't take a write lock while out of disk space", false);
+        }
+
         {
             SimpleMutex::scoped_lock lk(_m);
-            Lock::assertAtLeastReadLocked(ns);
             DBs& m = _paths[path];
             {
                 DBs::iterator i = m.find(dbname);
diff --git a/src/mongo/db/client.cpp b/src/mongo/db/client.cpp
index 46f2c2c10da..b2adc2dfce1 100644
--- a/src/mongo/db/client.cpp
+++ b/src/mongo/db/client.cpp
@@ -269,25 +269,18 @@ namespace mongo {
         _db(db)
     {
         verify(_db);
-        checkNotStale();
+        if (_doVersion) checkNotStale();
         _client->_context = this;
         _client->_curOp->enter( this );
     }
        
     void Client::Context::_finishInit() {
-        dassert( Lock::isLocked() );
-        int writeLocked = Lock::somethingWriteLocked();
-        if ( writeLocked && FileAllocator::get()->hasFailed() ) {
-            uassert(14031, "Can't take a write lock while out of disk space", false);
-        }
-        
         OperationContextImpl txn; // TODO get rid of this once reads require transactions
         _db = dbHolder().getOrCreate(&txn, _ns, _path, _justCreated);
-        verify(_db);
+        invariant(_db);
+
         if( _doVersion ) checkNotStale();
-        massert(16107,
-                str::stream() << "Don't have a lock on: " << _ns,
-                txn.lockState()->isAtLeastReadLocked(_ns));
+
         _client->_context = this;
         _client->_curOp->enter( this );
     }
diff --git a/src/mongo/db/commands/copydb.cpp b/src/mongo/db/commands/copydb.cpp
index 21dd7804d22..ceeef5df29a 100644
--- a/src/mongo/db/commands/copydb.cpp
+++ b/src/mongo/db/commands/copydb.cpp
@@ -153,11 +153,6 @@ namespace mongo {
                 return false;
             }
 
-            // SERVER-4328 todo lock just the two db's not everything for the fromself case
-            scoped_ptr<Lock::ScopedLock> lk( fromSelf ?
-                                             static_cast<Lock::ScopedLock*>(new Lock::GlobalWrite(txn->lockState())) :
-                                             static_cast<Lock::ScopedLock*>(new Lock::DBWrite(txn->lockState(), todb)));
-
             Cloner cloner;
             string username = cmdObj.getStringField( "username" );
             string nonce = cmdObj.getStringField( "nonce" );
@@ -166,7 +161,6 @@ namespace mongo {
                 uassert( 13008, "must call copydbgetnonce first", authConn_.get() );
                 BSONObj ret;
                 {
-                    dbtemprelease t(txn->lockState());
                     if ( !authConn_->runCommand( cloneOptions.fromDB,
                                                  BSON( "authenticate" << 1 << "user" << username
                                                        << "nonce" << nonce << "key" << key ), ret ) ) {
@@ -183,12 +177,20 @@ namespace mongo {
                 if (!cs.isValid()) {
                     return false;
                 }
+
                 DBClientBase* conn = cs.connect(errmsg);
                 if (!conn) {
                     return false;
                 }
                 cloner.setConnection(conn);
             }
+
+
+            // SERVER-4328 todo lock just the two db's not everything for the fromself case
+            scoped_ptr<Lock::ScopedLock> lk( fromSelf ?
+                                             static_cast<Lock::ScopedLock*>(new Lock::GlobalWrite(txn->lockState())) :
+                                             static_cast<Lock::ScopedLock*>(new Lock::DBWrite(txn->lockState(), todb)));
+
             Client::Context ctx(todb);
             return cloner.go(txn, ctx, fromhost, cloneOptions, NULL, errmsg );
         }
diff --git a/src/mongo/db/commands/fsync.cpp b/src/mongo/db/commands/fsync.cpp
index 8849c821bd3..4c79e7029b9 100644
--- a/src/mongo/db/commands/fsync.cpp
+++ b/src/mongo/db/commands/fsync.cpp
@@ -95,7 +95,7 @@ namespace mongo {
         }
         virtual bool run(OperationContext* txn, const string& dbname, BSONObj& cmdObj, int, string& errmsg, BSONObjBuilder& result, bool fromRepl) {
 
-            if (Lock::isLocked()) {
+            if (txn->lockState()->isLocked()) {
                 errmsg = "fsync: Cannot execute fsync command from contexts that hold a data lock";
                 return false;
             }
@@ -200,7 +200,6 @@ namespace mongo {
     
     // @return true if unlocked
     bool _unlockFsync() {
-        verify(!Lock::isLocked());
         SimpleMutex::scoped_lock lk( fsyncCmd.m );
         if( !fsyncCmd.locked ) { 
             return false;
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 55b840bb9f3..6212156693e 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -209,7 +209,7 @@ namespace mongo {
     }
 
     bool _unlockFsync();
-    void unlockFsync(const char *ns, Message& m, DbResponse &dbresponse) {
+    static void unlockFsync(const char *ns, Message& m, DbResponse &dbresponse) {
         BSONObj obj;
 
         const bool isAuthorized = cc().getAuthorizationSession()->isAuthorizedForActionsOnResource(
@@ -342,9 +342,13 @@ namespace mongo {
         const char *ns = m.singleData()->_data + 4;
 
         Client& c = cc();
-        if (!c.isGod())
+        if (!c.isGod()) {
             c.getAuthorizationSession()->startRequest(txn);
 
+            // We should not be holding any locks at this point
+            invariant(!txn->lockState()->isLocked());
+        }
+
         if ( op == dbQuery ) {
             if( strstr(ns, ".$cmd") ) {
                 isCommand = true;
diff --git a/src/mongo/db/lockstate.cpp b/src/mongo/db/lockstate.cpp
index 852e4b89a83..512f7e468c7 100644
--- a/src/mongo/db/lockstate.cpp
+++ b/src/mongo/db/lockstate.cpp
@@ -84,11 +84,19 @@ namespace mongo {
         return false;
     }
 
-    bool LockState::isWriteLocked(const StringData& ns) {
-        if (threadState() == 'W')
+    bool LockState::isLocked() const {
+        return threadState() != 0;
+    }
+
+    bool LockState::isWriteLocked() const {
+        return (threadState() == 'W' || threadState() == 'w');
+    }
+
+    bool LockState::isWriteLocked(const StringData& ns) const {
+        if (isWriteLocked()) {
             return true;
-        if (threadState() != 'w')
-            return false;
+        }
+
         return isLocked(ns);
     }
 
diff --git a/src/mongo/db/lockstate.h b/src/mongo/db/lockstate.h
index 693ae520a18..03e9cfb56f1 100644
--- a/src/mongo/db/lockstate.h
+++ b/src/mongo/db/lockstate.h
@@ -60,7 +60,9 @@ namespace mongo {
         bool hasAnyWriteLock() const; // wW
         
         bool isLocked(const StringData& ns) const; // rwRW
-        bool isWriteLocked(const StringData& ns);
+        bool isLocked() const;
+        bool isWriteLocked() const;
+        bool isWriteLocked(const StringData& ns) const;
         bool isAtLeastReadLocked(const StringData& ns) const;
         bool isNested() const;
 
diff --git a/src/mongo/db/repl/master_slave.cpp b/src/mongo/db/repl/master_slave.cpp
index 2494177a02c..69a154d30fb 100644
--- a/src/mongo/db/repl/master_slave.cpp
+++ b/src/mongo/db/repl/master_slave.cpp
@@ -331,6 +331,7 @@ namespace repl {
     void ReplSource::forceResync( OperationContext* txn, const char *requester ) {
         BSONObj info;
         {
+            // This is always a GlobalWrite lock (so no ns/db used from the context)
             dbtemprelease t(txn->lockState());
             if (!oplogReader.connect(hostName, _me)) {
                 msgassertedNoTrace( 14051 , "unable to connect to resync");
@@ -370,7 +371,7 @@ namespace repl {
      *                      Currently this will only be set if there is an error in the initial
      *                      system.namespaces query.
      */
-    bool cloneFrom(OperationContext* txn,
+    static bool cloneFrom(OperationContext* txn,
                            Client::Context& context,
                            const string& masterHost,
                            const CloneOptions& options,
@@ -469,6 +470,7 @@ namespace repl {
         OpTime lastTime;
         bool dbOk = false;
         {
+            // This is always a GlobalWrite lock (so no ns/db used from the context)
             dbtemprelease release(txn->lockState());
 
             // We always log an operation after executing it (never before), so
@@ -898,23 +900,17 @@ namespace repl {
             int n = 0;
             time_t saveLast = time(0);
             while ( 1 ) {
-
-                bool moreInitialSyncsPending = !addDbNextPass.empty() && n; // we need "&& n" to assure we actually process at least one op to get a sync point recorded in the first place.
+                // we need "&& n" to assure we actually process at least one op to get a sync
+                // point recorded in the first place.
+                const bool moreInitialSyncsPending = !addDbNextPass.empty() && n;
 
                 if ( moreInitialSyncsPending || !oplogReader.more() ) {
                     Lock::GlobalWrite lk(txn->lockState());
-
-                    // NOTE aaron 2011-03-29 This block may be unnecessary, but I'm leaving it in place to avoid changing timing behavior.
-                    {
-                        dbtemprelease t(txn->lockState());
-                        if ( !moreInitialSyncsPending && oplogReader.more() ) {
-                            continue;
-                        }
-                        // otherwise, break out of loop so we can set to completed or clone more dbs
-                    }
                     
-                    if( oplogReader.awaitCapable() && tailing )
+                    if (oplogReader.awaitCapable() && tailing) {
                         okResultCode = 0; // don't sleep
+                    }
+
                     syncedTo = nextOpTime;
                     save(); // note how far we are synced up to now
                     log() << "repl:   applied " << n << " operations" << endl;
@@ -922,8 +918,6 @@ namespace repl {
                     log() << "repl:  end sync_pullOpLog syncedTo: " << syncedTo.toStringLong() << endl;
                     break;
                 }
-                else {
-                }
 
                 OCCASIONALLY if( n > 0 && ( n > 100000 || time(0) - saveLast > 60 ) ) {
                     // periodically note our progress, in case we are doing a lot of work and crash
@@ -1314,8 +1308,9 @@ namespace repl {
 
     void pretouchOperation(OperationContext* txn, const BSONObj& op) {
 
-        if( Lock::somethingWriteLocked() )
+        if (txn->lockState()->isWriteLocked()) {
             return; // no point pretouching if write locked. not sure if this will ever fire, but just in case.
+        }
 
         const char *which = "o";
         const char *opType = op.getStringField("op");
diff --git a/src/mongo/db/repl/repl_set_impl.h b/src/mongo/db/repl/repl_set_impl.h
index da6d2efdf3d..5a33df4b01f 100644
--- a/src/mongo/db/repl/repl_set_impl.h
+++ b/src/mongo/db/repl/repl_set_impl.h
@@ -291,7 +291,7 @@ namespace repl {
         void _syncThread();
         void syncTail();
         unsigned _syncRollback(OperationContext* txn, OplogReader& r);
-        void syncFixUp(FixUpInfo& h, OplogReader& r);
+        void syncFixUp(OperationContext* txn, FixUpInfo& h, OplogReader& r);
 
         // keep a list of hosts that we've tried recently that didn't work
         map<string,time_t> _veto;
diff --git a/src/mongo/db/repl/repl_settings.cpp b/src/mongo/db/repl/repl_settings.cpp
index 65db46755ab..8c893959b9c 100644
--- a/src/mongo/db/repl/repl_settings.cpp
+++ b/src/mongo/db/repl/repl_settings.cpp
@@ -117,7 +117,7 @@ namespace repl {
                 }
                 
                 if ( level > 1 ) {
-                    wassert( !Lock::isLocked() );
+                    wassert(txn->lockState()->threadState() == 0);
                     // note: there is no so-style timeout on this connection; perhaps we should have one.
                     ScopedDbConnection conn(s["host"].valuestr());
                     
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index d31830aebe3..5d8bc99cca5 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -230,14 +230,12 @@ namespace repl {
 
     int getRBID(DBClientConnection*);
 
-    static void syncRollbackFindCommonPoint(DBClientConnection* them, FixUpInfo& fixUpInfo) {
-        OperationContextImpl txn; // XXX
-        verify(Lock::isLocked());
+    static void syncRollbackFindCommonPoint(OperationContext* txn, DBClientConnection* them, FixUpInfo& fixUpInfo) {
         Client::Context ctx(rsoplog);
 
         boost::scoped_ptr<Runner> runner(
                 InternalPlanner::collectionScan(rsoplog,
-                                                ctx.db()->getCollection(&txn, rsoplog),
+                                                ctx.db()->getCollection(txn, rsoplog),
                                                 InternalPlanner::BACKWARD));
 
         BSONObj ourObj;
@@ -360,9 +358,9 @@ namespace repl {
         return cloner.copyCollection(txn, ns, BSONObj(), errmsg, true, false, true, false);
     }
 
-    void ReplSetImpl::syncFixUp(FixUpInfo& fixUpInfo, OplogReader& oplogreader) {
+    void ReplSetImpl::syncFixUp(
+                    OperationContext* txn, FixUpInfo& fixUpInfo, OplogReader& oplogreader) {
         DBClientConnection* them = oplogreader.conn();
-        OperationContextImpl txn;
 
         // fetch all first so we needn't handle interruption in a fancy way
 
@@ -440,11 +438,11 @@ namespace repl {
                 sethbmsg(str::stream() << "rollback 4.1 coll resync " << ns);
 
                 Client::Context ctx(ns);
-                ctx.db()->dropCollection(&txn, ns);
+                ctx.db()->dropCollection(txn, ns);
                 {
                     string errmsg;
-                    dbtemprelease release(txn.lockState());
-                    bool ok = copyCollectionFromRemote(&txn, them->getServerAddress(), ns, errmsg);
+                    dbtemprelease release(txn->lockState());
+                    bool ok = copyCollectionFromRemote(txn, them->getServerAddress(), ns, errmsg);
                     uassert(15909, str::stream() << "replSet rollback error resyncing collection "
                                                  << ns << ' ' << errmsg, ok);
                 }
@@ -494,12 +492,12 @@ namespace repl {
                 it++) {
             Client::Context ctx(*it);
             log() << "replSet rollback drop: " << *it << rsLog;
-            ctx.db()->dropCollection(&txn, *it);
+            ctx.db()->dropCollection(txn, *it);
         }
 
         sethbmsg("rollback 4.7");
         Client::Context ctx(rsoplog);
-        Collection* oplogCollection = ctx.db()->getCollection(&txn, rsoplog);
+        Collection* oplogCollection = ctx.db()->getCollection(txn, rsoplog);
         uassert(13423,
                 str::stream() << "replSet error in rollback can't find " << rsoplog,
                 oplogCollection);
@@ -528,7 +526,7 @@ namespace repl {
                     continue;
                 }
 
-                txn.recoveryUnit()->commitIfNeeded();
+                txn->recoveryUnit()->commitIfNeeded();
 
                 // keep an archive of items rolled back
                 shared_ptr<Helpers::RemoveSaver>& removeSaver = removeSavers[doc.ns];
@@ -540,7 +538,7 @@ namespace repl {
 
                 // Add the doc to our rollback file
                 BSONObj obj;
-                bool found = Helpers::findOne(&txn, ctx.db()->getCollection(&txn, doc.ns), pattern, obj, false);
+                bool found = Helpers::findOne(txn, ctx.db()->getCollection(txn, doc.ns), pattern, obj, false);
                 if (found) {
                     removeSaver->goingToDelete(obj);
                 }
@@ -553,7 +551,7 @@ namespace repl {
                     // TODO 1.6 : can't delete from a capped collection.  need to handle that here.
                     deletes++;
 
-                    Collection* collection = ctx.db()->getCollection(&txn, doc.ns);
+                    Collection* collection = ctx.db()->getCollection(txn, doc.ns);
                     if (collection) {
                         if (collection->isCapped()) {
                             // can't delete from a capped collection - so we truncate instead. if
@@ -562,7 +560,7 @@ namespace repl {
                                 // TODO: IIRC cappedTruncateAfter does not handle completely empty.
                                 // this will crazy slow if no _id index.
                                 long long start = Listener::getElapsedTimeMillis();
-                                DiskLoc loc = Helpers::findOne(&txn, collection, pattern, false);
+                                DiskLoc loc = Helpers::findOne(txn, collection, pattern, false);
                                 if (Listener::getElapsedTimeMillis() - start > 200)
                                     log() << "replSet warning roll back slow no _id index for "
                                           << doc.ns << " perhaps?" << rsLog;
@@ -570,12 +568,12 @@ namespace repl {
                                 // DiskLoc loc = Helpers::findById(nsd, pattern);
                                 if (!loc.isNull()) {
                                     try {
-                                        collection->temp_cappedTruncateAfter(&txn, loc, true);
+                                        collection->temp_cappedTruncateAfter(txn, loc, true);
                                     }
                                     catch (DBException& e) {
                                         if (e.getCode() == 13415) {
                                             // hack: need to just make cappedTruncate do this...
-                                            uassertStatusOK(collection->truncate(&txn));
+                                            uassertStatusOK(collection->truncate(txn));
                                         }
                                         else {
                                             throw e;
@@ -589,7 +587,7 @@ namespace repl {
                             }
                         }
                         else {
-                            deleteObjects(&txn, 
+                            deleteObjects(txn, 
                                           ctx.db(),
                                           doc.ns,
                                           pattern,
@@ -605,7 +603,7 @@ namespace repl {
                                 BSONObj nsResult = them->findOne(sys, QUERY("name" << doc.ns));
                                 if (nsResult.isEmpty()) {
                                     // we should drop
-                                    ctx.db()->dropCollection(&txn, doc.ns);
+                                    ctx.db()->dropCollection(txn, doc.ns);
                                 }
                             }
                             catch (DBException&) {
@@ -631,7 +629,7 @@ namespace repl {
                     UpdateLifecycleImpl updateLifecycle(true, requestNs);
                     request.setLifecycle(&updateLifecycle);
 
-                    update(&txn, ctx.db(), request, &debug);
+                    update(txn, ctx.db(), request, &debug);
 
                 }
             }
@@ -652,7 +650,7 @@ namespace repl {
         LOG(2) << "replSet rollback truncate oplog after " << fixUpInfo.commonPoint.toStringPretty()
                << rsLog;
         // TODO: fatal error if this throws?
-        oplogCollection->temp_cappedTruncateAfter(&txn, fixUpInfo.commonPointOurDiskloc, false);
+        oplogCollection->temp_cappedTruncateAfter(txn, fixUpInfo.commonPointOurDiskloc, false);
 
         Status status = getGlobalAuthorizationManager()->initialize();
         if (!status.isOK()) {
@@ -700,7 +698,7 @@ namespace repl {
 
     unsigned ReplSetImpl::_syncRollback(OperationContext* txn, OplogReader& oplogreader) {
         verify(!lockedByMe());
-        verify(!Lock::isLocked());
+        verify(txn->lockState()->threadState() == 0);
 
         sethbmsg("rollback 0");
 
@@ -727,7 +725,7 @@ namespace repl {
 
             sethbmsg("rollback 2 FindCommonPoint");
             try {
-                syncRollbackFindCommonPoint(oplogreader.conn(), how);
+                syncRollbackFindCommonPoint(txn, oplogreader.conn(), how);
             }
             catch (RSFatalException& e) {
                 sethbmsg(string(e.what()));
@@ -746,7 +744,7 @@ namespace repl {
 
         incRBID();
         try {
-            syncFixUp(how, oplogreader);
+            syncFixUp(txn, how, oplogreader);
         }
         catch (RSFatalException& e) {
             sethbmsg("rollback fixup error");
diff --git a/src/mongo/db/storage/data_file.cpp b/src/mongo/db/storage/data_file.cpp
index 915484aa0e3..2a21cc7e87c 100644
--- a/src/mongo/db/storage/data_file.cpp
+++ b/src/mongo/db/storage/data_file.cpp
@@ -186,7 +186,7 @@ namespace mongo {
 
             {
                 // "something" is too vague, but we checked for the right db to be locked higher up the call stack
-                if( !Lock::somethingWriteLocked() ) {
+                if (!txn->lockState()->isWriteLocked()) {
                     txn->lockState()->dump();
                     log() << "*** TEMP NOT INITIALIZING FILE " << filename << ", not in a write lock." << endl;
                     log() << "temp bypass until more elaborate change - case that is manifesting is benign anyway" << endl;
diff --git a/src/mongo/db/storage/mmap_v1/dur_commitjob.h b/src/mongo/db/storage/mmap_v1/dur_commitjob.h
index 48ac023e68b..cdbf1e8ae27 100644
--- a/src/mongo/db/storage/mmap_v1/dur_commitjob.h
+++ b/src/mongo/db/storage/mmap_v1/dur_commitjob.h
@@ -136,7 +136,6 @@ namespace mongo {
             void note(void* p, int len);
 
             std::vector< shared_ptr<DurOp> >& ops() {
-                dassert( Lock::isLocked() );          // a rather weak check, we require more than that
                 groupCommitMutex.dassertLocked(); // this is what really makes the below safe
                 return _intentsAndDurOps._durOps;                
             }
diff --git a/src/mongo/s/d_migrate.cpp b/src/mongo/s/d_migrate.cpp
index c44d207dd9c..0381eccd9af 100644
--- a/src/mongo/s/d_migrate.cpp
+++ b/src/mongo/s/d_migrate.cpp
@@ -1092,7 +1092,8 @@ namespace mongo {
             // Track last result from TO shard for sanity check
             BSONObj res;
             for ( int i=0; i<86400; i++ ) { // don't want a single chunk move to take more than a day
-                verify(!txn->lockState()->threadState());
+                invariant(!txn->lockState()->threadState());
+
                 // Exponential sleep backoff, up to 1024ms. Don't sleep much on the first few
                 // iterations, since we want empty chunk migrations to be fast.
                 sleepmillis( 1 << std::min( i , 10 ) );