SERVER-16659 Fix CursorManager/ClientCursorPin cosmetic issues

Const-correctness, comments.

4 files changed, 57 insertions, 19 deletions

diff --git a/src/mongo/db/catalog/cursor_manager.cpp b/src/mongo/db/catalog/cursor_manager.cpp
index e097bd1aee7..ea03c3175e1 100644
--- a/src/mongo/db/catalog/cursor_manager.cpp
+++ b/src/mongo/db/catalog/cursor_manager.cpp
@@ -285,8 +285,8 @@ namespace mongo {
     }
 
     std::size_t CursorManager::timeoutCursorsGlobal(OperationContext* txn,
-        int millisSinceLastCall) {;
-    return globalCursorIdCache->timeoutCursors(txn, millisSinceLastCall);
+                                                    int millisSinceLastCall) {
+        return globalCursorIdCache->timeoutCursors(txn, millisSinceLastCall);
     }
 
     int CursorManager::eraseCursorGlobalIfAuthorized(OperationContext* txn, int n,
@@ -477,11 +477,11 @@ namespace mongo {
         cursor->unsetPinned();
     }
 
-    bool CursorManager::ownsCursorId( CursorId cursorId ) {
+    bool CursorManager::ownsCursorId( CursorId cursorId ) const {
         return _collectionCacheRuntimeId == idFromCursorId( cursorId );
     }
 
-    void CursorManager::getCursorIds( std::set<CursorId>* openCursors ) {
+    void CursorManager::getCursorIds( std::set<CursorId>* openCursors ) const {
         SimpleMutex::scoped_lock lk( _mutex );
 
         for ( CursorMap::const_iterator i = _cursors.begin(); i != _cursors.end(); ++i ) {
@@ -490,7 +490,7 @@ namespace mongo {
         }
     }
 
-    size_t CursorManager::numCursors(){
+    size_t CursorManager::numCursors() const {
         SimpleMutex::scoped_lock lk( _mutex );
         return _cursors.size();
     }
diff --git a/src/mongo/db/catalog/cursor_manager.h b/src/mongo/db/catalog/cursor_manager.h
index 5c5619c5ebb..78430d72e57 100644
--- a/src/mongo/db/catalog/cursor_manager.h
+++ b/src/mongo/db/catalog/cursor_manager.h
@@ -99,9 +99,17 @@ namespace mongo {
 
         bool eraseCursor(OperationContext* txn, CursorId id, bool checkAuth );
 
-        bool ownsCursorId( CursorId cursorId );
-        void getCursorIds( std::set<CursorId>* openCursors );
-        std::size_t numCursors();
+        /**
+         * Returns true if the space of cursor ids that cursor manager is responsible for includes
+         * the given cursor id.  Otherwise, returns false.
+         *
+         * The return value of this method does not indicate any information about whether or not a
+         * cursor actually exists with the given cursor id.  Use the find() method for that purpose.
+         */
+        bool ownsCursorId( CursorId cursorId ) const;
+
+        void getCursorIds( std::set<CursorId>* openCursors ) const;
+        std::size_t numCursors() const;
 
         /**
          * @param pin - if true, will try to pin cursor
@@ -135,7 +143,7 @@ namespace mongo {
         unsigned _collectionCacheRuntimeId;
         boost::scoped_ptr<PseudoRandom> _random;
 
-        SimpleMutex _mutex;
+        mutable SimpleMutex _mutex;
 
         typedef unordered_set<PlanExecutor*> ExecSet;
         ExecSet _nonCachedExecutors;
diff --git a/src/mongo/db/clientcursor.cpp b/src/mongo/db/clientcursor.cpp
index be61f0fa541..ffa40cab530 100644
--- a/src/mongo/db/clientcursor.cpp
+++ b/src/mongo/db/clientcursor.cpp
@@ -216,8 +216,6 @@ namespace mongo {
 
     //
     // Pin methods
-    // TODO: Simplify when we kill Cursor.  In particular, once we've pinned a CC, it won't be
-    // deleted from underneath us, so we can save the pointer and ignore the ID.
     //
 
     ClientCursorPin::ClientCursorPin( CursorManager* cursorManager, long long cursorid )
diff --git a/src/mongo/db/clientcursor.h b/src/mongo/db/clientcursor.h
index 462ae1caa4a..3f80c19d824 100644
--- a/src/mongo/db/clientcursor.h
+++ b/src/mongo/db/clientcursor.h
@@ -286,17 +286,49 @@ namespace mongo {
     };
 
     /**
-     * use this to assure we don't in the background time out cursor while it is under use.  if you
-     * are using noTimeout() already, there is no risk anyway.  Further, this mechanism guards
-     * against two getMore requests on the same cursor executing at the same time - which might be
-     * bad.  That should never happen, but if a client driver had a bug, it could (or perhaps some
-     * sort of attack situation).
-     * Must have a read lock on the collection already
-    */
-    class ClientCursorPin : boost::noncopyable {
+     * ClientCursorPin is an RAII class that manages the pinned state of a ClientCursor.
+     * ClientCursorPin objects pin the given cursor upon construction, and release the pin upon
+     * destruction.
+     *
+     * A pin extends the lifetime of a ClientCursor object until the pin's release.  Pinned
+     * ClientCursor objects cannot not be killed due to inactivity, and cannot be killed by user
+     * kill requests.  When a CursorManager is destroyed (e.g. by a collection drop), ownership of
+     * any still-pinned ClientCursor objects is transferred to their managing ClientCursorPin
+     * objects.
+     *
+     * Example usage:
+     * {
+     *     ClientCursorPin pin(cursorManager, cursorid);
+     *     ClientCursor* cursor = pin.c();
+     *     if (cursor) {
+     *         // Use cursor.
+     *     }
+     *     // Pin automatically released on block exit.
+     * }
+     *
+     * Clients that wish to access ClientCursor objects owned by collection cursor managers must
+     * hold the collection lock during pin acquisition and pin release.  This guards from a
+     * collection drop (which requires an exclusive lock on the collection) occurring concurrently
+     * with the pin request or unpin request.
+     *
+     * Clients that wish to access ClientCursor objects owned by the global cursor manager need not
+     * hold any locks; the global cursor manager can only be destroyed by a process exit.
+     */
+    class ClientCursorPin {
+        MONGO_DISALLOW_COPYING(ClientCursorPin);
     public:
+        /**
+         * Asks "cursorManager" to set a pin on the ClientCursor associated with "cursorid".  If no
+         * such cursor exists, does nothing.  If the cursor is already pinned, throws a
+         * UserException.
+         */
         ClientCursorPin( CursorManager* cursorManager, long long cursorid );
+
+        /**
+         * Calls release().
+         */
         ~ClientCursorPin();
+
         // This just releases the pin, does not delete the underlying
         // unless ownership has passed to us after kill
         void release();