Interim: remove proxy_auth_username_cache.

Attempt fixing cache cleanup and strange iterator behaviors.
May crash on shutdown if there are entries in cache.

diff --git a/src/auth/Gadgets.cc b/src/auth/Gadgets.cc
index 4ab485a990ec681798ef499f6f86d8e6c00ecac0..3eddb8f28589bbd4b59df9e7867d97395f76c1a3 100644 (file)
--- a/src/auth/Gadgets.cc
+++ b/src/auth/Gadgets.cc
@@ -69,10 +69,6 @@ authenticateRegisterWithCacheManager(Auth::ConfigVector * config)
 void
 authenticateInit(Auth::ConfigVector * config)
 {
-    /* Do this first to clear memory and remove dead state on a reconfigure */
-    if (proxy_auth_username_cache)
-        Auth::User::CachedACLsReset();
-
     /* If we do not have any auth config state to create stop now. */
     if (!config)
         return;
@@ -84,9 +80,6 @@ authenticateInit(Auth::ConfigVector * config)
             schemeCfg->init(schemeCfg);
     }
 
-    if (!proxy_auth_username_cache)
-        Auth::User::cacheInit();
-
     authenticateRegisterWithCacheManager(config);
 }
 
@@ -101,16 +94,9 @@ authenticateRotate(void)
 void
 authenticateReset(void)
 {
-    debugs(29, 2, HERE << "Reset authentication State.");
-
-    /* free all username cache entries */
-    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;
-    }
+    debugs(29, 2, "Reset authentication State.");
+
+    // username cache is cleared via Runner registry
 
     /* schedule shutdown of the helpers */
     authenticateRotate();
@@ -119,20 +105,6 @@ authenticateReset(void)
     Auth::TheConfig.clear();
 }
 
-AuthUserHashPointer::AuthUserHashPointer(Auth::User::Pointer anAuth_user):
-    auth_user(anAuth_user)
-{
-    key = (void *)anAuth_user->userKey();
-    next = NULL;
-    hash_join(proxy_auth_username_cache, (hash_link *) this);
-}
-
-Auth::User::Pointer
-AuthUserHashPointer::user() const
-{
-    return auth_user;
-}
-
 std::vector<Auth::User::Pointer>
 authenticateCachedUsersList()
 {

diff --git a/src/auth/Gadgets.h b/src/auth/Gadgets.h
index 4174f9a0836ead3e4fc3adee2e06280329bce411..3a312522b6e36eefae97a35edce172b2aa573f19 100644 (file)
--- a/src/auth/Gadgets.h
+++ b/src/auth/Gadgets.h
@@ -15,32 +15,6 @@
 #include "auth/User.h"
 #include "hash.h"
 
-/**
- \ingroup AuthAPI
- *
- * 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
-{
-    MEMPROXY_CLASS(AuthUserHashPointer);
-
-public:
-    AuthUserHashPointer(Auth::User::Pointer);
-    ~AuthUserHashPointer() { auth_user = NULL; };
-
-    Auth::User::Pointer user() const;
-
-private:
-    Auth::User::Pointer auth_user;
-};
-
 namespace Auth
 {
 class Scheme;

diff --git a/src/auth/User.cc b/src/auth/User.cc
index ad7e801676cae167b451e8fbb27b4e2af0302c76..305fbbab69fe2b9122a31e2e6af5bdccfc60e05e 100644 (file)
--- a/src/auth/User.cc
+++ b/src/auth/User.cc
@@ -22,8 +22,6 @@
 #include "SquidTime.h"
 #include "Store.h"
 
-time_t Auth::User::last_discard = 0;
-
 Auth::User::User(Auth::Config *aConfig, const char *aRequestRealm) :
     auth_type(Auth::AUTH_UNKNOWN),
     config(aConfig),
@@ -139,84 +137,6 @@ Auth::User::~User()
     auth_type = Auth::AUTH_UNKNOWN;
 }
 
-void
-Auth::User::cacheInit(void)
-{
-    if (!proxy_auth_username_cache) {
-        /* First time around, 7921 should be big enough */
-        proxy_auth_username_cache = hash_create((HASHCMP *) strcmp, 7921, hash_string);
-        assert(proxy_auth_username_cache);
-        eventAdd("User Cache Maintenance", cacheCleanup, NULL, ::Config.authenticateGCInterval, 1);
-        last_discard = squid_curtime;
-    }
-}
-
-void
-Auth::User::CachedACLsReset()
-{
-    /*
-     * This must complete all at once, because we are ensuring correctness.
-     */
-    AuthUserHashPointer *usernamehash;
-    Auth::User::Pointer auth_user;
-    debugs(29, 3, HERE << "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();
-        /* free cached acl results */
-        aclCacheMatchFlush(&auth_user->proxy_match_cache);
-    }
-
-    //new-style moved to UserNameCache::syncConfig()
-
-    debugs(29, 3, HERE << "Finished.");
-}
-
-void
-Auth::User::cacheCleanup(void *)
-{
-    /*
-     * We walk the hash by username as that is the unique key we use.
-     * For big hashs we could consider stepping through the cache, 100/200
-     * entries at a time. Lets see how it flys first.
-     */
-    AuthUserHashPointer *usernamehash;
-    Auth::User::Pointer auth_user;
-    char const *username = NULL;
-    debugs(29, 3, HERE << "Cleaning the user cache now");
-    debugs(29, 3, HERE << "Current time: " << current_time.tv_sec);
-    hash_first(proxy_auth_username_cache);
-
-    while ((usernamehash = ((AuthUserHashPointer *) hash_next(proxy_auth_username_cache)))) {
-        auth_user = usernamehash->user();
-        username = auth_user->username();
-
-        /* if we need to have indedendent expiry clauses, insert a module call
-         * here */
-        debugs(29, 4, HERE << "Cache entry:\n\tType: " <<
-               auth_user->auth_type << "\n\tUsername: " << username <<
-               "\n\texpires: " <<
-               (long int) (auth_user->expiretime + ::Config.authenticateTTL) <<
-               "\n\treferences: " << auth_user->LockCount());
-
-        if (auth_user->expiretime + ::Config.authenticateTTL <= current_time.tv_sec) {
-            debugs(29, 5, HERE << "Removing user " << username << " from cache due to timeout.");
-
-            /* Old credentials are always removed. Existing users must hold their own
-             * Auth::User::Pointer to the credentials. Cache exists only for finding
-             * and re-using current valid credentials.
-             */
-            hash_remove_link(proxy_auth_username_cache, usernamehash);
-            delete usernamehash;
-        }
-    }
-
-    debugs(29, 3, HERE << "Finished cleaning the user cache.");
-    eventAdd("User Cache Maintenance", cacheCleanup, NULL, ::Config.authenticateGCInterval, 1);
-    last_discard = squid_curtime;
-}
-
 void
 Auth::User::clearIp()
 {

diff --git a/src/auth/User.h b/src/auth/User.h
index e849b20db7c7b1f649b7c9e2a46a5f1875b34c61..923a5c9208459af8f69415fdd4e7761b2b847825 100644 (file)
--- a/src/auth/User.h
+++ b/src/auth/User.h
@@ -20,7 +20,6 @@
 #include "Notes.h"
 #include "SBuf.h"
 
-class AuthUserHashPointer;
 class StoreEntry;
 
 namespace Auth
@@ -58,8 +57,6 @@ public:
     NotePairs notes;
 
 public:
-    static void cacheInit();
-    static void CachedACLsReset();
     static SBuf BuildUserKey(const char *username, const char *realm);
 
     void absorb(Auth::User::Pointer from);
@@ -107,12 +104,6 @@ protected:
     User(Auth::Config *, const char *requestRealm);
 
 private:
-    /**
-     * Garbage Collection for the username cache.
-     */
-    static void cacheCleanup(void *unused);
-    static time_t last_discard; /// Time of last username cache garbage collection.
-
     /**
      * DPW 2007-05-08
      * The username_ memory will be allocated via

diff --git a/src/auth/UserNameCache.cc b/src/auth/UserNameCache.cc
index efac3690f4422661837cff1c0800d07a1212ec3e..5aa0677eb8dd0e2c5aae9953a9a09e7e607838d3 100644 (file)
--- a/src/auth/UserNameCache.cc
+++ b/src/auth/UserNameCache.cc
@@ -52,13 +52,23 @@ UserNameCache::Cleanup(void *data)
     UserNameCache *self = static_cast<UserNameCache *>(data);
     // cache entries with expiretime <= expirationTime are to be evicted
     const time_t expirationTime =  current_time.tv_sec - ::Config.authenticateTTL;
+
+    //XXX for some reason if evicting directly iterators are invalidated
+    // trying to defer deletion by using a queue
+    std::list<StoreType::iterator> toDelete;
     const auto end = self->store_.end();
     for (auto i = self->store_.begin(); i != end; ++i) {
+        debugs(29, 2, "considering " << i->first << "(expires in " <<
+                        (expirationTime - i->second->expiretime) << " sec)");
         if (i->second->expiretime <= expirationTime) {
-            debugs(29, 2, "evicting " << i->first);
-            self->store_.erase(i);
+            debugs(29, 2, "queueing for eviction " << i->first);
+            toDelete.push_back(i);
         }
     }
+    for (auto i : toDelete) {
+        debugs(29, 2, "evicting " << i->first);
+        self->store_.erase(i); //less efficient but safer than iter
+    }
     eventAdd(self->cacheCleanupEventName.c_str(), &UserNameCache::Cleanup,
                     self, ::Config.authenticateGCInterval, 1);
 }

diff --git a/src/auth/basic/User.cc b/src/auth/basic/User.cc
index f6b8d669e6c6436d145ff67de82f0647265a9df7..8c31744c6b8fb03306b20101d414d824979960d2 100644 (file)
--- a/src/auth/basic/User.cc
+++ b/src/auth/basic/User.cc
@@ -9,7 +9,6 @@
 #include "squid.h"
 #include "auth/basic/Config.h"
 #include "auth/basic/User.h"
-#include "auth/Gadgets.h" // for AuthUserHashPointer
 #include "auth/UserNameCache.h"
 #include "Debug.h"
 #include "SquidConfig.h"
@@ -92,7 +91,5 @@ Auth::Basic::User::Cache()
 void
 Auth::Basic::User::addToNameCache()
 {
-    /* AuthUserHashPointer will self-register with the username cache */
-    new AuthUserHashPointer(this); //legacy
     Cache()->insert(this);
 }

diff --git a/src/auth/digest/User.cc b/src/auth/digest/User.cc
index 5dd558fc6e66fe661b304677527481ccdb7c639e..21d833b49cc7883f4d24b412f1054d4858a2223d 100644 (file)
--- a/src/auth/digest/User.cc
+++ b/src/auth/digest/User.cc
@@ -9,7 +9,6 @@
 #include "squid.h"
 #include "auth/digest/Config.h"
 #include "auth/digest/User.h"
-#include "auth/Gadgets.h" // for AuthUserHashPointer
 #include "auth/UserNameCache.h"
 #include "Debug.h"
 #include "dlink.h"
@@ -84,7 +83,5 @@ Auth::Digest::User::Cache()
 void
 Auth::Digest::User::addToNameCache()
 {
-    /* AuthUserHashPointer will self-register with the username cache */
-    new AuthUserHashPointer(this); //legacy
     Cache()->insert(this);
 }

diff --git a/src/auth/negotiate/User.cc b/src/auth/negotiate/User.cc
index 47bdfb6889fb2b8dd0b3b65361b16b1319952d31..2c9af467f7f9ab793a15b1fcb7459b313ffbfe45 100644 (file)
--- a/src/auth/negotiate/User.cc
+++ b/src/auth/negotiate/User.cc
@@ -8,7 +8,6 @@
 
 #include "squid.h"
 #include "auth/Config.h"
-#include "auth/Gadgets.h" // for AuthUserHashPointer
 #include "auth/negotiate/User.h"
 #include "auth/UserNameCache.h"
 #include "Debug.h"
@@ -39,7 +38,5 @@ Auth::Negotiate::User::Cache()
 void
 Auth::Negotiate::User::addToNameCache()
 {
-    /* AuthUserHashPointer will self-register with the username cache */
-    new AuthUserHashPointer(this); //legacy
     Cache()->insert(this);
 }

diff --git a/src/auth/ntlm/User.cc b/src/auth/ntlm/User.cc
index 57b23e4d44cc0ab67b1bef161afa85afc17eb37a..6e218b3c207eccaf69bab7fe3d232c6a5eef9c48 100644 (file)
--- a/src/auth/ntlm/User.cc
+++ b/src/auth/ntlm/User.cc
@@ -8,7 +8,6 @@
 
 #include "squid.h"
 #include "auth/Config.h"
-#include "auth/Gadgets.h" // for AuthUserHashPointer
 #include "auth/ntlm/User.h"
 #include "auth/UserNameCache.h"
 #include "Debug.h"
@@ -39,7 +38,5 @@ Auth::Ntlm::User::Cache()
 void
 Auth::Ntlm::User::addToNameCache()
 {
-    /* AuthUserHashPointer will self-register with the username cache */
-    new AuthUserHashPointer(this); //legacy
     Cache()->insert(this);
 }

diff --git a/src/globals.h b/src/globals.h
index 2f8c7d8af44ad34321b9b7a24d2f6b299e103613..b317871acd8c977b91b9fcd2166dc0c29fef9cc7 100644 (file)
--- a/src/globals.h
+++ b/src/globals.h
@@ -88,7 +88,6 @@ extern int store_swap_low;  /* 0 */
 extern int store_swap_high; /* 0 */
 extern size_t store_pages_max;  /* 0 */
 extern int64_t store_maxobjsize;    /* 0 */
-extern hash_table *proxy_auth_username_cache;   /* NULL */
 extern int incoming_sockets_accepted;
 #if _SQUID_WINDOWS_
 extern unsigned int WIN32_Socks_initialized;    /* 0 */

diff --git a/src/tests/stub_libauth.cc b/src/tests/stub_libauth.cc
index 00e99240273d01e054cbef0bbefcbb0933036321..10e22d57e626988657ae38176597009ba95fd544 100644 (file)
--- a/src/tests/stub_libauth.cc
+++ b/src/tests/stub_libauth.cc
@@ -25,9 +25,6 @@ void authenticateInit(Auth::ConfigVector *) STUB
 void authenticateRotate(void) STUB
 void authenticateReset(void) STUB
 
-AuthUserHashPointer::AuthUserHashPointer(Auth::User::Pointer anAuth_user) STUB
-Auth::User::Pointer AuthUserHashPointer::user() const STUB_RETVAL(NULL)
-
 #include "auth/Scheme.h"
 #include <vector>
 std::vector<Auth::Scheme::Pointer> *Auth::Scheme::_Schemes = NULL;