Straighten out AuthUserHashPointer code

* AuthserHashPointer is a 'temporary' type which may only be stored by the
  username hash table. Remove a unused pointer to it in AuthUser.

* Removes a duplicate walk of the username hash table on reconfigure.

* Removes several useless functions.

diff --git a/src/auth/Gadgets.cc b/src/auth/Gadgets.cc
index 424b2b3e633c0a15e1c9744c7989d7c50db971ed..7948bac290e489aeff60e5275678353fb4d911fb 100644 (file)
--- a/src/auth/Gadgets.cc
+++ b/src/auth/Gadgets.cc
@@ -60,7 +60,7 @@ authenticateActiveSchemeCount(void)
         if ((*i)->configured())
             ++rv;
 
-    debugs(29, 9, "authenticateActiveSchemeCount: " << rv << " active.");
+    debugs(29, 9, HERE << rv << " active.");
 
     return rv;
 }
@@ -70,7 +70,7 @@ authenticateSchemeCount(void)
 {
     int rv = AuthScheme::GetSchemes().size();
 
-    debugs(29, 9, "authenticateSchemeCount: " << rv << " active.");
+    debugs(29, 9, HERE << rv << " active.");
 
     return rv;
 }
@@ -87,6 +87,11 @@ authenticateRegisterWithCacheManager(Auth::authConfig * config)
 void
 authenticateInit(Auth::authConfig * config)
 {
+    /* Do this first to clear memory and remove dead state on a reconfigure */
+    if (proxy_auth_username_cache)
+        AuthUser::CachedACLsReset();
+
+    /* If we do not have any auth config state to create stop now. */
     if (!config)
         return;
 
@@ -99,8 +104,6 @@ authenticateInit(Auth::authConfig * config)
 
     if (!proxy_auth_username_cache)
         AuthUser::cacheInit();
-    else
-        AuthUser::CachedACLsReset();
 
     authenticateRegisterWithCacheManager(config);
 }
@@ -108,11 +111,16 @@ authenticateInit(Auth::authConfig * config)
 void
 authenticateShutdown(void)
 {
-    debugs(29, 2, "authenticateShutdown: shutting down auth schemes");
+    debugs(29, 2, HERE << "Shutting down auth schemes");
     /* free the cache if we are shutting down */
-
     if (shutting_down) {
-        hashFreeItems(proxy_auth_username_cache, AuthUserHashPointer::removeFromCache);
+        hash_first(proxy_auth_username_cache);
+        AuthUserHashPointer *usernamehash;
+        while ((usernamehash = ((AuthUserHashPointer *) hash_next(proxy_auth_username_cache)))) {
+            debugs(29, 5, HERE << "Clearing entry for user: " << usernamehash->user()->username());
+            hash_remove_link(proxy_auth_username_cache, (hash_link *)usernamehash);
+            delete usernamehash;
+        }
         AuthScheme::FreeAll();
     } else {
         for (AuthScheme::iterator i = (AuthScheme::GetSchemes()).begin(); i != (AuthScheme::GetSchemes()).end(); ++i)
@@ -120,33 +128,6 @@ authenticateShutdown(void)
     }
 }
 
-/**
- * Cleans all config-dependent data from the auth_user cache.
- \note It DOES NOT Flush the user cache.
- */
-void
-authenticateUserCacheRestart(void)
-{
-    AuthUserHashPointer *usernamehash;
-    AuthUser::Pointer auth_user;
-    debugs(29, 3, HERE << "Clearing config dependent cache data.");
-    hash_first(proxy_auth_username_cache);
-
-    while ((usernamehash = ((AuthUserHashPointer *) hash_next(proxy_auth_username_cache)))) {
-        auth_user = usernamehash->user();
-        debugs(29, 5, "authenticateUserCacheRestat: Clearing cache ACL results for user: " << auth_user->username());
-    }
-}
-
-// TODO: remove this wrapper. inline the actions.
-void
-AuthUserHashPointer::removeFromCache(void *usernamehash_p)
-{
-    AuthUserHashPointer *usernamehash = static_cast<AuthUserHashPointer *>(usernamehash_p);
-    hash_remove_link(proxy_auth_username_cache, (hash_link *)usernamehash);
-    delete usernamehash;
-}
-
 AuthUserHashPointer::AuthUserHashPointer(AuthUser::Pointer anAuth_user):
         auth_user(anAuth_user)
 {

diff --git a/src/auth/Gadgets.h b/src/auth/Gadgets.h
index bad1fe726d33437eed990411810a077a3109fd47..78016189f883adab7121d3c2a12c92e389fd5585 100644 (file)
--- a/src/auth/Gadgets.h
+++ b/src/auth/Gadgets.h
@@ -43,9 +43,14 @@ class AuthUser;
 /**
  \ingroup AuthAPI
  *
- * This is used to link auth_users into the username cache.
+ * This is used to link AuthUsers objects into the username cache.
  * Because some schemes may link in aliases to a user,
  * the link is not part of the AuthUser structure itself.
+ *
+ * Code must not hold onto copies of these objects.
+ * They may exist only so long as the AuthUser being referenced
+ * is recorded in the cache. Any caller using hash_remove_link
+ * must then delete the AuthUserHashPointer.
  */
 class AuthUserHashPointer : public hash_link {
     /* first two items must be same as hash_link */
@@ -57,7 +62,6 @@ public:
     ~AuthUserHashPointer() { auth_user = NULL; };
 
     AuthUser::Pointer user() const;
-    static void removeFromCache(void *anAuthUserHashPointer);
 
 private:
     AuthUser::Pointer auth_user;
@@ -87,8 +91,6 @@ extern int authenticateActiveSchemeCount(void);
 /// \ingroup AuthAPI
 extern int authenticateSchemeCount(void);
 
-/// \ingroup AuthAPI
-extern void authenticateUserCacheRestart(void);
 /// \ingroup AuthAPI
 extern void authenticateOnCloseConnection(ConnStateData * conn);
 

diff --git a/src/auth/User.cc b/src/auth/User.cc
index a6c9e248d9d2a6f114647553d3e38b730df2aba0..c388dc1b2b740f21602733772722fb5d6658047e 100644 (file)
--- a/src/auth/User.cc
+++ b/src/auth/User.cc
@@ -54,7 +54,6 @@ CBDATA_TYPE(AuthUserIP);
 AuthUser::AuthUser(AuthConfig *aConfig) :
         auth_type(AUTH_UNKNOWN),
         config(aConfig),
-        usernamehash(NULL),
         ipcount(0),
         expiretime(0),
         username_(NULL)
@@ -146,15 +145,6 @@ AuthUser::~AuthUser()
     debugs(29, 5, "AuthUser::~AuthUser: Freeing auth_user '" << this << "'.");
     assert(RefCountCount() == 0);
 
-    /* were they linked in by username ? */
-    if (usernamehash) {
-        debugs(29, 5, "AuthUser::~AuthUser: removing usernamehash entry '" << usernamehash << "'");
-        hash_remove_link(proxy_auth_username_cache, (hash_link *) usernamehash);
-        /* don't free the key as we use the same user string as the auth_user
-         * structure */
-        delete usernamehash;
-    }
-
     /* free cached acl results */
     aclCacheMatchFlush(&proxy_match_cache);
 
@@ -183,18 +173,15 @@ void
 AuthUser::CachedACLsReset()
 {
     /*
-     * We walk the hash by username as that is the unique key we use.
      * This must complete all at once, because we are ensuring correctness.
      */
     AuthUserHashPointer *usernamehash;
     AuthUser::Pointer auth_user;
-    char const *username = NULL;
     debugs(29, 3, "AuthUser::CachedACLsReset: Flushing the ACL caches for all users.");
     hash_first(proxy_auth_username_cache);
 
     while ((usernamehash = ((AuthUserHashPointer *) hash_next(proxy_auth_username_cache)))) {
         auth_user = usernamehash->user();
-        username = auth_user->username();
         /* free cached acl results */
         aclCacheMatchFlush(&auth_user->proxy_match_cache);
     }
@@ -221,7 +208,7 @@ AuthUser::cacheCleanup(void *datanotused)
         auth_user = usernamehash->user();
         username = auth_user->username();
 
-        /* if we need to have inpedendent expiry clauses, insert a module call
+        /* if we need to have indedendent expiry clauses, insert a module call
          * here */
         debugs(29, 4, "AuthUser::cacheCleanup: Cache entry:\n\tType: " <<
                auth_user->auth_type << "\n\tUsername: " << username <<
@@ -237,9 +224,6 @@ AuthUser::cacheCleanup(void *datanotused)
              * and re-using current valid credentials.
              */
             hash_remove_link(proxy_auth_username_cache, usernamehash);
-            /* resolve the circular references of AuthUserHashPointer<->AuthUser by cutting before deleting. */
-            if(auth_user->usernamehash == usernamehash)
-                auth_user->usernamehash = NULL;
             delete usernamehash;
         }
     }
@@ -344,11 +328,14 @@ AuthUser::addIp(Ip::Address ipaddr)
     debugs(29, 2, "authenticateAuthUserAddIp: user '" << username() << "' has been seen at a new IP address (" << ipaddr << ")");
 }
 
-/* addToNameCache: add a auth_user structure to the username cache */
+/**
+ * Add the AuthUser structure to the username cache.
+ */
 void
 AuthUser::addToNameCache()
 {
-    usernamehash = new AuthUserHashPointer(this);
+    /* AuthUserHashPointer will self-register with the username cache */
+    AuthUserHashPointer *notused = new AuthUserHashPointer(this);
 }
 
 /**

diff --git a/src/auth/User.h b/src/auth/User.h
index 67a777095e24f98b5d2c94eb602329e18f7e2d8e..2e0c5d43b5eb3120e459fb24a6362c5e81a343ee 100644 (file)
--- a/src/auth/User.h
+++ b/src/auth/User.h
@@ -64,8 +64,6 @@ public:
     AuthType auth_type;
     /** the config for this user */
     AuthConfig *config;
-    /** we only have one username associated with a given auth_user struct */
-    AuthUserHashPointer *usernamehash;
     /** we may have many proxy-authenticate strings that decode to the same user */
     dlink_list proxy_auth_list;
     dlink_list proxy_match_cache;

diff --git a/src/main.cc b/src/main.cc
index 45a990e2fc6a56a586bbdcf5a118be2659c39842..1896a08e115b7b546b3d45d5f35a655d4ba47498 100644 (file)
--- a/src/main.cc
+++ b/src/main.cc
@@ -733,7 +733,6 @@ mainReconfigureFinish(void *)
     setEffectiveUser();
     _db_init(Debug::cache_log, Debug::debugOptions);
     ipcache_restart();         /* clear stuck entries */
-    authenticateUserCacheRestart();    /* clear stuck ACL entries */
     fqdncache_restart();       /* sigh, fqdncache too */
     parseEtcHosts();
     errorInitialize();         /* reload error pages */