SERVER-9518 Rename AuthzUpgradeLockGuard and make it a generic guard …

…for AuthorizationManager.

src/mongo/db/auth/authorization_manager.cpp

@@ -203,6 +203,11 @@ namespace mongo {
 
     Status AuthorizationManager::acquireUser(const UserName& userName, User** acquiredUser) {
         boost::lock_guard<boost::mutex> lk(_lock);
+        return _acquireUser_inlock(userName, acquiredUser);
+    }
+
+    Status AuthorizationManager::_acquireUser_inlock(const UserName& userName,
+                                                     User** acquiredUser) {
         unordered_map<UserName, User*>::iterator it = _userCache.find(userName);
         if (it != _userCache.end()) {
             fassert(16914, it->second);
@@ -410,35 +415,51 @@ namespace mongo {
         return Status::OK();
     }
 
-    namespace {
-        class AuthzUpgradeLockGuard {
-            MONGO_DISALLOW_COPYING(AuthzUpgradeLockGuard);
-        public:
-            explicit AuthzUpgradeLockGuard(AuthzManagerExternalState* externalState)
-                : _externalState(externalState), _locked(false) {
-            }
 
-            ~AuthzUpgradeLockGuard() {
-                if (_locked)
-                    unlock();
-            }
+    AuthorizationManager::Guard::Guard(AuthorizationManager* authzManager)
+        : _authzManager(authzManager),
+          _lockedForUpdate(false),
+          _authzManagerLock(authzManager->_lock) {}
 
-            bool tryLock() {
-                fassert(17111, !_locked);
-                _locked = _externalState->tryLockUpgradeProcess();
-                return _locked;
+    AuthorizationManager::Guard::~Guard() {
+        if (_lockedForUpdate) {
+            if (!_authzManagerLock.owns_lock()) {
+                _authzManagerLock.lock();
             }
+            releaseAuthzUpdateLock();
+        }
+    }
 
-            void unlock() {
-                fassert(17112, _locked);
-                _externalState->unlockUpgradeProcess();
-                _locked = false;
-            }
-        private:
-            AuthzManagerExternalState* _externalState;
-            bool _locked;
-        };
+    bool AuthorizationManager::Guard::tryAcquireAuthzUpdateLock() {
+        fassert(17111, !_lockedForUpdate);
+        fassert(17126, _authzManagerLock.owns_lock());
+        _lockedForUpdate = _authzManager->_externalState->tryAcquireAuthzUpdateLock();
+        return _lockedForUpdate;
+    }
 
+    void AuthorizationManager::Guard::releaseAuthzUpdateLock() {
+        fassert(17112, _lockedForUpdate);
+        fassert(17127, _authzManagerLock.owns_lock());
+        _authzManager->_externalState->releaseAuthzUpdateLock();
+        _lockedForUpdate = false;
+    }
+
+    void AuthorizationManager::Guard::acquireAuthorizationManagerLock() {
+        fassert(17129, !_authzManagerLock.owns_lock());
+        _authzManagerLock.lock();
+    }
+
+    void AuthorizationManager::Guard::releaseAuthorizationManagerLock() {
+        fassert(17128, _authzManagerLock.owns_lock());
+        _authzManagerLock.unlock();
+    }
+
+    Status AuthorizationManager::Guard::acquireUser(const UserName& userName, User** acquiredUser) {
+        fassert(17130, _authzManagerLock.owns_lock());
+        return _authzManager->_acquireUser_inlock(userName, acquiredUser);
+    }
+
+    namespace {
         BSONObj userAsV2PrivilegeDocument(const User& user) {
             BSONObjBuilder builder;
 
@@ -495,9 +516,8 @@ namespace mongo {
     }  // namespace
 
     Status AuthorizationManager::upgradeAuthCollections() {
-        boost::lock_guard<boost::mutex> lkLocal(_lock);
-        AuthzUpgradeLockGuard lkUpgrade(_externalState.get());
-        if (!lkUpgrade.tryLock()) {
+        AuthorizationManager::Guard lkUpgrade(this);
+        if (!lkUpgrade.tryAcquireAuthzUpdateLock()) {
             return Status(ErrorCodes::LockBusy, "Could not lock auth data upgrade process lock.");
         }
         int durableVersion = 0;

src/mongo/db/auth/authorization_manager.h

@@ -63,6 +63,9 @@ namespace mongo {
         MONGO_DISALLOW_COPYING(AuthorizationManager);
     public:
 
+        // Locks the AuthorizationManager and guards access to its members
+        class Guard;
+
         // The newly constructed AuthorizationManager takes ownership of "externalState"
         explicit AuthorizationManager(AuthzManagerExternalState* externalState);
 
@@ -223,6 +226,8 @@ namespace mongo {
 
     private:
 
+        Status _acquireUser_inlock(const UserName& userName, User** acquiredUser);
+
         /**
          * Returns the current version number of the authorization system.  Should only be called
          * when holding _lock.
@@ -292,6 +297,81 @@ namespace mongo {
          * Protects _userCache, _roleGraph, _version, and _parser.
          */
         boost::mutex _lock;
+
+        friend class AuthorizationManager::Guard;
+    };
+
+
+    /*
+     * Guard object for locking an AuthorizationManager.
+     * There are two different locks that this object interacts with: the AuthorizationManager's
+     * _lock (henceforth called the AM::_lock), and the authzUpdateLock.  The AM::_lock protects
+     * reading and writing the in-memory data structures managed by the AuthorizationManager.  The
+     * authzUpdateLock is the lock that serializes all writes to the persistent authorization
+     * documents (admin.system.users, admin.system.roles, admin.system.version).
+     * When the guard is constructed it initially locks the AM::_lock.  The guard's destructor will
+     * always release the AM::_lock if it is held.
+     * The guard then provides some public methods that allow interaction with the
+     * AuthorizationManager while inside the AM::_lock.
+     * If modifications to the authorization documents in persistent storage is required, then
+     * consumers of the guard can call tryAcquireAuthzUpdateLock() to lock the authzUpdateLock.
+     * If that is successful, consumers can then call releaseAuthorizationManagerLock to unlock the
+     * AM::_lock, while keeping the authzUpdateLock locked.  This allows
+     * modifications to the authorization documents to occur without the AM::_lock needing to be
+     * held the whole time (which prevents authentication and access control
+     * checks).
+     * Since changing the state of the authzUpdateLock requires the AM::_lock to
+     * be held, if the guard's destructor is called when the authzUpdateLock is held but the
+     * AM::_lock is not, it will first acquire the AM::_lock, then release the authzUpdateLock,
+     * then finally release the AM::_lock.
+     *
+     * Note: This locking semantics only works because we never block to acquire the
+     * authzUpdateLock.  If a blocking acquireAuthzUpdateLock were introduced, it could introduce
+     * deadlocks.
+     */
+    class AuthorizationManager::Guard {
+        MONGO_DISALLOW_COPYING(Guard);
+    public:
+        explicit Guard(AuthorizationManager* authzManager);
+        ~Guard();
+
+        /**
+         * Tries to acquire the global lock guarding modifications to all persistent data related
+         * to authorization, namely the admin.system.users, admin.system.roles, and
+         * admin.system.version collections.  This serializes all writers to the authorization
+         * documents, but does not impact readers.
+         * The AuthorizationManager's _lock must be held before this is called.
+         */
+        bool tryAcquireAuthzUpdateLock();
+
+        /**
+         * Releases the lock guarding modifications to persistent authorization data, which must
+         * already be held.
+         * The AuthorizationManager's _lock must be held before this is called.
+         */
+        void releaseAuthzUpdateLock();
+
+        /**
+         * Releases the AuthorizationManager's _lock.
+         */
+        void releaseAuthorizationManagerLock();
+
+        /**
+         * Acquires the AuthorizationManager's _lock.
+         */
+        void acquireAuthorizationManagerLock();
+
+        /**
+         * Delegates to AuthorizationManager's _acquireUser_inlock method.
+         */
+        Status acquireUser(const UserName& userName, User** acquiredUser);
+
+    private:
+        AuthorizationManager* _authzManager;
+        // True if the Guard has locked the lock that guards modifications to authz documents.
+        bool _lockedForUpdate;
+        // For locking the AuthorizationManager's _lock
+        boost::unique_lock<boost::mutex> _authzManagerLock;
     };
 
 } // namespace mongo

src/mongo/db/auth/authz_manager_external_state.h

@@ -150,14 +150,18 @@ namespace mongo {
                                       const NamespaceString& toName) = 0;
 
         /**
-         * Tries to acquire the global lock guarding the upgrade process.
+         * Tries to acquire the global lock guarding modifications to all persistent data related
+         * to authorization, namely the admin.system.users, admin.system.roles, and
+         * admin.system.version collections.  This serializes all writers to the authorization
+         * documents, but does not impact readers.
          */
-        virtual bool tryLockUpgradeProcess() = 0;
+        virtual bool tryAcquireAuthzUpdateLock() = 0;
 
         /**
-         * Releases the lock guarding the upgrade process, which must already be held.
+         * Releases the lock guarding modifications to persistent authorization data, which must
+         * already be held.
          */
-        virtual void unlockUpgradeProcess() = 0;
+        virtual void releaseAuthzUpdateLock() = 0;
 
     protected:
         AuthzManagerExternalState(); // This class should never be instantiated directly.

src/mongo/db/auth/authz_manager_external_state_d.cpp

@@ -231,11 +231,11 @@ namespace {
         fassertFailed(17098);
     }
 
-    bool AuthzManagerExternalStateMongod::tryLockUpgradeProcess() {
+    bool AuthzManagerExternalStateMongod::tryAcquireAuthzUpdateLock() {
         fassertFailed(17099);
     }
 
-    void AuthzManagerExternalStateMongod::unlockUpgradeProcess() {
+    void AuthzManagerExternalStateMongod::releaseAuthzUpdateLock() {
         fassertFailed(17100);
     }
 

src/mongo/db/auth/authz_manager_external_state_d.h

@@ -79,8 +79,8 @@ namespace mongo {
                                         const NamespaceString& newName);
         virtual Status copyCollection(const NamespaceString& fromName,
                                       const NamespaceString& toName);
-        virtual bool tryLockUpgradeProcess();
-        virtual void unlockUpgradeProcess();
+        virtual bool tryAcquireAuthzUpdateLock();
+        virtual void releaseAuthzUpdateLock();
 
     protected:
         virtual Status _findUser(const string& usersNamespace,

src/mongo/db/auth/authz_manager_external_state_mock.cpp

@@ -208,11 +208,11 @@ namespace mongo {
         return Status::OK();
     }
 
-    bool AuthzManagerExternalStateMock::tryLockUpgradeProcess() {
+    bool AuthzManagerExternalStateMock::tryAcquireAuthzUpdateLock() {
         return true;
     }
 
-    void AuthzManagerExternalStateMock::unlockUpgradeProcess() {}
+    void AuthzManagerExternalStateMock::releaseAuthzUpdateLock() {}
 
     std::vector<BSONObj> AuthzManagerExternalStateMock::getCollectionContents(
             const NamespaceString& collectionName) {

src/mongo/db/auth/authz_manager_external_state_mock.h

@@ -94,8 +94,8 @@ namespace mongo {
                                         const NamespaceString& newName);
         virtual Status copyCollection(const NamespaceString& fromName,
                                       const NamespaceString& toName);
-        virtual bool tryLockUpgradeProcess();
-        virtual void unlockUpgradeProcess();
+        virtual bool tryAcquireAuthzUpdateLock();
+        virtual void releaseAuthzUpdateLock();
 
         std::vector<BSONObj> getCollectionContents(const NamespaceString& collectionName);
 

src/mongo/db/auth/authz_manager_external_state_s.cpp

@@ -258,11 +258,11 @@ namespace {
         fassertFailed(17108);
     }
 
-    bool AuthzManagerExternalStateMongos::tryLockUpgradeProcess() {
+    bool AuthzManagerExternalStateMongos::tryAcquireAuthzUpdateLock() {
         fassertFailed(17109);
     }
 
-    void AuthzManagerExternalStateMongos::unlockUpgradeProcess() {
+    void AuthzManagerExternalStateMongos::releaseAuthzUpdateLock() {
         fassertFailed(17110);
     }
 

src/mongo/db/auth/authz_manager_external_state_s.h

@@ -79,8 +79,8 @@ namespace mongo {
                                         const NamespaceString& newName);
         virtual Status copyCollection(const NamespaceString& fromName,
                                       const NamespaceString& toName);
-        virtual bool tryLockUpgradeProcess();
-        virtual void unlockUpgradeProcess();
+        virtual bool tryAcquireAuthzUpdateLock();
+        virtual void releaseAuthzUpdateLock();
 
     protected:
         virtual Status _findUser(const string& usersNamespace,