SERVER-13495: fix ClientCursor::_pinValue concurrency

5 files changed, 74 insertions, 38 deletions

diff --git a/src/mongo/db/catalog/collection_cursor_cache.cpp b/src/mongo/db/catalog/collection_cursor_cache.cpp
index 06a06091091..4ff28aa3a71 100644
--- a/src/mongo/db/catalog/collection_cursor_cache.cpp
+++ b/src/mongo/db/catalog/collection_cursor_cache.cpp
@@ -192,12 +192,7 @@ namespace mongo {
             return false;
         Client::Context context( ns, db );
         Collection* collection = db->getCollection( ns );
-        ClientCursor* cursor = NULL;
-        if ( collection ) {
-            cursor = collection->cursorCache()->find( id );
-        }
-
-        if ( !cursor ) {
+        if ( !collection ) {
             if ( checkAuth )
                 audit::logKillCursorsAuthzCheck( currentClient.get(),
                                                  nss,
@@ -205,21 +200,8 @@ namespace mongo {
                                                  ErrorCodes::CursorNotFound );
             return false;
         }
-        
-        if ( checkAuth )
-            audit::logKillCursorsAuthzCheck( currentClient.get(),
-                                             nss,
-                                             id,
-                                             ErrorCodes::OK );
 
-        massert( 16089,
-                 str::stream() << "Cannot kill active cursor " << id,
-                 cursor->pinValue() < 100 );
-
-        cursor->kill();
-        collection->cursorCache()->deregisterCursor( cursor );
-        delete cursor;
-        return true;
+        return collection->cursorCache()->eraseCursor( id, checkAuth );
     }
 
     std::size_t GlobalCursorIdCache::timeoutCursors( unsigned millisSinceLastCall ) {
@@ -279,15 +261,15 @@ namespace mongo {
 
 
     CollectionCursorCache::CollectionCursorCache( const StringData& ns )
-        : _ns( ns.toString() ),
+        : _nss( ns ),
           _mutex( "CollectionCursorCache" ) {
-        _collectionCacheRuntimeId = _globalCursorIdCache.created( _ns );
+        _collectionCacheRuntimeId = _globalCursorIdCache.created( _nss.ns() );
         _random.reset( new PseudoRandom( _globalCursorIdCache.nextSeed() ) );
     }
 
     CollectionCursorCache::~CollectionCursorCache() {
         invalidateAll( true );
-        _globalCursorIdCache.destroyed( _collectionCacheRuntimeId, _ns );
+        _globalCursorIdCache.destroyed( _collectionCacheRuntimeId, _nss.ns() );
     }
 
     void CollectionCursorCache::invalidateAll( bool collectionGoingAway ) {
@@ -415,12 +397,28 @@ namespace mongo {
         _nonCachedRunners.erase( runner );
     }
 
-    ClientCursor* CollectionCursorCache::find( CursorId id ) {
+    ClientCursor* CollectionCursorCache::find( CursorId id, bool pin ) {
         SimpleMutex::scoped_lock lk( _mutex );
         CursorMap::const_iterator it = _cursors.find( id );
         if ( it == _cursors.end() )
             return NULL;
-        return it->second;
+
+        ClientCursor* cursor = it->second;
+        if ( pin ) {
+            uassert( 12051,
+                     "clientcursor already in use? driver problem?",
+                     cursor->_pinValue < 100 );
+            cursor->_pinValue += 100;
+        }
+
+        return cursor;
+    }
+
+    void CollectionCursorCache::unpin( ClientCursor* cursor ) {
+        SimpleMutex::scoped_lock lk( _mutex );
+
+        invariant( cursor->_pinValue >= 100 );
+        cursor->_pinValue -= 100;
     }
 
     void CollectionCursorCache::getCursorIds( std::set<CursorId>* openCursors ) {
@@ -460,6 +458,38 @@ namespace mongo {
         _deregisterCursor_inlock( cc );
     }
 
+    bool CollectionCursorCache::eraseCursor( CursorId id, bool checkAuth ) {
+
+        SimpleMutex::scoped_lock lk( _mutex );
+
+        CursorMap::iterator it = _cursors.find( id );
+        if ( it == _cursors.end() ) {
+            if ( checkAuth )
+                audit::logKillCursorsAuthzCheck( currentClient.get(),
+                                                 _nss,
+                                                 id,
+                                                 ErrorCodes::CursorNotFound );
+            return false;
+        }
+
+        ClientCursor* cursor = it->second;
+
+        if ( checkAuth )
+            audit::logKillCursorsAuthzCheck( currentClient.get(),
+                                             _nss,
+                                             id,
+                                             ErrorCodes::OK );
+
+        massert( 16089,
+                 str::stream() << "Cannot kill active cursor " << id,
+                 cursor->pinValue() < 100 );
+
+        cursor->kill();
+        _deregisterCursor_inlock( cursor );
+        delete cursor;
+        return true;
+    }
+
     void CollectionCursorCache::_deregisterCursor_inlock( ClientCursor* cc ) {
         invariant( cc );
         CursorId id = cc->cursorid();
diff --git a/src/mongo/db/catalog/collection_cursor_cache.h b/src/mongo/db/catalog/collection_cursor_cache.h
index 4e8254515de..284d303391f 100644
--- a/src/mongo/db/catalog/collection_cursor_cache.h
+++ b/src/mongo/db/catalog/collection_cursor_cache.h
@@ -33,6 +33,7 @@
 #include "mongo/db/clientcursor.h"
 #include "mongo/db/diskloc.h"
 #include "mongo/db/invalidation_type.h"
+#include "mongo/db/namespace_string.h"
 #include "mongo/platform/unordered_set.h"
 #include "mongo/util/concurrency/mutex.h"
 
@@ -92,10 +93,19 @@ namespace mongo {
         CursorId registerCursor( ClientCursor* cc );
         void deregisterCursor( ClientCursor* cc );
 
+        bool eraseCursor( CursorId id, bool checkAuth );
+
         void getCursorIds( std::set<CursorId>* openCursors );
         std::size_t numCursors();
 
-        ClientCursor* find( CursorId id );
+        /**
+         * @param pin - if true, will try to pin cursor
+         *                  if pinned already, will assert
+         *                  otherwise will pin
+         */
+        ClientCursor* find( CursorId id, bool pin );
+
+        void unpin( ClientCursor* cursor );
 
         // ----------------------
 
@@ -113,7 +123,7 @@ namespace mongo {
         CursorId _allocateCursorId_inlock();
         void _deregisterCursor_inlock( ClientCursor* cc );
 
-        string _ns;
+        NamespaceString _nss;
         unsigned _collectionCacheRuntimeId;
         scoped_ptr<PseudoRandom> _random;
 
diff --git a/src/mongo/db/clientcursor.cpp b/src/mongo/db/clientcursor.cpp
index f9a1c431459..26f937fdc1d 100644
--- a/src/mongo/db/clientcursor.cpp
+++ b/src/mongo/db/clientcursor.cpp
@@ -269,13 +269,7 @@ namespace mongo {
     ClientCursorPin::ClientCursorPin( const Collection* collection, long long cursorid )
         : _cursor( NULL ) {
         cursorStatsOpenPinned.increment();
-        _cursor = collection->cursorCache()->find( cursorid );
-        if ( _cursor ) {
-            uassert( 12051,
-                     "clientcursor already in use? driver problem?",
-                     _cursor->_pinValue < 100 );
-            _cursor->_pinValue += 100;
-        }
+        _cursor = collection->cursorCache()->find( cursorid, true );
     }
 
     ClientCursorPin::~ClientCursorPin() {
@@ -287,8 +281,7 @@ namespace mongo {
         if ( !_cursor )
             return;
 
-        invariant( _cursor->_pinValue >= 100 );
-        _cursor->_pinValue -= 100;
+        invariant( _cursor->pinValue() >= 100 );
 
         if ( _cursor->collection() == NULL ) {
             // the ClientCursor was killed while we had it
@@ -296,6 +289,9 @@ namespace mongo {
             delete _cursor;
             _cursor = NULL; // defensive
         }
+        else {
+            _cursor->collection()->cursorCache()->unpin( _cursor );
+        }
     }
 
     void ClientCursorPin::deleteUnderlying() {
diff --git a/src/mongo/db/clientcursor.h b/src/mongo/db/clientcursor.h
index 78075bde52b..bbd1e73af9e 100644
--- a/src/mongo/db/clientcursor.h
+++ b/src/mongo/db/clientcursor.h
@@ -149,8 +149,8 @@ namespace mongo {
 
     private:
         friend class ClientCursorMonitor;
-        friend class ClientCursorPin;
         friend class CmdCursorInfo;
+        friend class CollectionCursorCache;
 
         /**
          * Initialization common between both constructors for the ClientCursor.
diff --git a/src/mongo/dbtests/querytests.cpp b/src/mongo/dbtests/querytests.cpp
index 839fa49287d..8c53cc324b7 100644
--- a/src/mongo/dbtests/querytests.cpp
+++ b/src/mongo/dbtests/querytests.cpp
@@ -341,7 +341,7 @@ namespace QueryTests {
             {
                 Client::ReadContext ctx( ns );
                 ASSERT( 1 == ctx.ctx().db()->getCollection( ns )->cursorCache()->numCursors() );
-                ASSERT( ctx.ctx().db()->getCollection( ns )->cursorCache()->find( cursorId ) );
+                ASSERT( ctx.ctx().db()->getCollection( ns )->cursorCache()->find( cursorId, false ) );
             }
 
             // Check that the cursor can be iterated until all documents are returned.