From: Amos Jeffries Date: Sun, 25 Apr 2010 06:06:49 +0000 (+1200) Subject: Straighten out AuthUserHashPointer code X-Git-Tag: SQUID_3_2_0_1~167^2~25 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=427cb33ad412e7b32219592b2139b512a19a22e0;p=thirdparty%2Fsquid.git 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 424b2b3e63..7948bac290 100644 --- 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(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 bad1fe726d..78016189f8 100644 --- 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 a6c9e248d9..c388dc1b2b 100644 --- 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 67a777095e..2e0c5d43b5 100644 --- 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 45a990e2fc..1896a08e11 100644 --- 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 */