Merge branch 'user-caching' into maint

* user-caching:
  Always replace user pointers
  Don't populate auth users in set_chanlist, it loops through get_user_by_host()
  Document changes
  Clear out cached member pointers that are not used.
  Fill Auth user cache in set_chanlist
  Fix Auth::NewNick not swapping ht_nick cache
  Use existing block to clear user
  Stop caching handles in auth
  Must clear Auth users in clear_chanlist
  Consolidate clear_chanlist logic
  Consolidate clearing member user pointers into clear_cached_users()
  Add missing cache setting/clearing on conf.bots
  Consolidate user cache filling
  Consolidate user pointer cache clearing
  Remove pointless check
  Add some missing user pointer invalidations in clear_usrlist

doc/UPDATES.md

@@ -36,6 +36,7 @@
   * Fix bot forgetting its nick/jupenick during restart and reverting to
     botnick if restarting in the middle of a server connect or attempted
     NICK change that fails.
+  * Fix some various internal caching issues.
 
 # 1.4.6
   * Disable demo TCL support by default to prevent confusion during build.

src/auth.cc

@@ -1,4 +1,3 @@
-/* I hereby release this into the Public Domain - Bryan Drewery */
 /*
  * auth.c -- handles:
  *   auth system functions
@@ -39,39 +38,31 @@
 
 #include "stat.h"
 
-bd::HashTable<bd::String, Auth*> Auth::ht_handle(10);
 bd::HashTable<bd::String, Auth*> Auth::ht_host(10);
+bd::HashTable<bd::String, Auth*> Auth::ht_nick(10);
 
 Auth::Auth(const char *_nick, const char *_host, struct userrec *u)
 {
   Status(AUTHING);
   strlcpy(nick, _nick, NICKLEN);
   strlcpy(host, _host, UHOSTLEN);
-  if (u) {
-    user = u;
-    strlcpy(handle, u->handle, sizeof(handle));
-  } else {
-    user = NULL;
-    handle[0] = '*';
-    handle[1] = 0;
-  }
-
+  user = u;
 
   ht_host[host] = this;
-  if (user)
-    ht_handle[handle] = this;
+  ht_nick[_nick] = this;
 
-  sdprintf(STR("New auth created! (%s!%s) [%s]"), nick, host, handle);
+  sdprintf(STR("New auth created! (%s!%s) [%s]"), nick, host,
+      u ? u->handle : "*");
   authtime = atime = now;
   idx = -1;
 }
 
 Auth::~Auth()
 {
-  sdprintf(STR("Removing auth: (%s!%s) [%s]"), nick, host, handle);
-  if (user)
-    ht_handle.remove(handle);
+  sdprintf(STR("Removing auth: (%s!%s) [%s]"), nick, host,
+      user ? user->handle : "*");
   ht_host.remove(host);
+  ht_nick.remove(nick);
 }
 
 void Auth::MakeHash()
@@ -88,7 +79,11 @@ void Auth::Done()
 }
 
 void Auth::NewNick(const char *newnick) {
+  if (ht_nick.contains(nick)) {
+    Auth::ht_nick.remove(nick);
+  }
   strlcpy(nick, newnick, NICKLEN);
+  ht_nick[newnick] = this;
 }
 
 Auth *Auth::Find(const char *_host)
@@ -96,17 +91,8 @@ Auth *Auth::Find(const char *_host)
 
   if (ht_host.contains(_host)) {
     Auth *auth = ht_host[_host];
-    sdprintf(STR("Found auth: (%s!%s) [%s]"), auth->nick, auth->host, auth->handle);
-    return auth;
-  }
-  return NULL;
-}
-
-Auth *Auth::Find(const char *handle, bool _hand)
-{
-  if (ht_handle.contains(handle)) {
-    Auth *auth = ht_handle[handle];
-    sdprintf(STR("Found auth (by handle): %s (%s!%s)"), handle, auth->nick, auth->host);
+    sdprintf(STR("Found auth: (%s!%s) [%s]"), auth->nick, auth->host,
+        auth->user ? auth->user->handle : "*");
     return auth;
   }
   return NULL;
@@ -115,27 +101,44 @@ Auth *Auth::Find(const char *handle, bool _hand)
 static void auth_clear_users_block(const bd::String key, Auth* auth, void *param)
 {
   if (auth->user) {
-    sdprintf(STR("Clearing USER for auth: (%s!%s) [%s]"), auth->nick, auth->host, auth->handle);
+    sdprintf(STR("Clearing USER for auth: (%s!%s) [%s]"), auth->nick,
+        auth->host, auth->user ? auth->user->handle : "*");
     auth->user = NULL;
   }
 }
 
-void Auth::NullUsers()
+void Auth::NullUsers(const char *nick)
 {
-  ht_host.each(auth_clear_users_block);
+  if (nick == NULL) {
+    ht_host.each(auth_clear_users_block);
+  } else {
+    if (ht_nick.contains(nick)) {
+      Auth *auth = ht_nick[nick];
+      auth_clear_users_block(nick, auth, NULL);
+    }
+  }
 }
 
 static void auth_fill_users_block(const bd::String key, Auth* auth, void* param)
 {
-  if (strcmp(auth->handle, "*")) {
-    sdprintf(STR("Filling USER for auth: (%s!%s) [%s]"), auth->nick, auth->host, auth->handle);
-    auth->user = get_user_by_handle(userlist, auth->handle);
-  }
+  char from[NICKLEN + UHOSTLEN];
+
+  sdprintf(STR("Filling USER for auth: (%s!%s) [%s]"), auth->nick, auth->host,
+      auth->user ? auth->user->handle : "*");
+  simple_snprintf(from, sizeof(from), "%s!%s", auth->nick, auth->host);
+  auth->user = get_user_by_host(from);
 }
 
-void Auth::FillUsers()
+void Auth::FillUsers(const char *nick)
 {
-  ht_host.each(auth_fill_users_block);
+  if (nick == NULL) {
+    ht_host.each(auth_fill_users_block);
+  } else {
+    if (ht_nick.contains(nick)) {
+      Auth *auth = ht_nick[nick];
+      auth_fill_users_block(nick, auth, NULL);
+    }
+  }
 }
 
 
@@ -143,7 +146,7 @@ static void auth_expire_block(const bd::String key, Auth* auth, void* param)
 {
   if (auth->Authed() && ((now - auth->atime) >= (60 * 60))) {
     Auth::ht_host.remove(key);
-    Auth::ht_handle.remove(key);
+    Auth::ht_nick.remove(auth->nick);
     delete auth;
   }
 }
@@ -158,7 +161,8 @@ void Auth::ExpireAuths()
 
 static void auth_delete_all_block(const bd::String, Auth* auth, void* param)
 {
-  putlog(LOG_DEBUG, "*", STR("Removing (%s!%s) [%s], from auth list."), auth->nick, auth->host, auth->handle);
+  putlog(LOG_DEBUG, "*", STR("Removing (%s!%s) [%s], from auth list."),
+      auth->nick, auth->host, auth->user ? auth->user->handle : "*");
   delete auth;
 }
 
@@ -168,7 +172,7 @@ void Auth::DeleteAll()
     putlog(LOG_DEBUG, "*", STR("Removing auth entries."));
     ht_host.each(auth_delete_all_block);
     ht_host.clear();
-    ht_handle.clear();
+    ht_nick.clear();
   }
 }
 
@@ -185,7 +189,7 @@ sdprintf(STR("GETIDX: auth: %s, idx: %d"), nick, idx);
       idx = -1;
     else if (!dcc[idx].irc || dcc[idx].simul == -1)
       idx = -1;
-    else if (strcmp(dcc[idx].nick, handle))
+    else if (user && strcmp(dcc[idx].nick, user->handle))
       idx = -1;
     else {
       sdprintf(STR("FIRST FOUND: %d"), idx);
@@ -199,7 +203,8 @@ sdprintf(STR("GETIDX: auth: %s, idx: %d"), nick, idx);
 
   for (i = 0; i < dcc_total; i++) {
     if (dcc[i].type && dcc[i].irc &&
-       (((chname && chname[0]) && !strcmp(dcc[i].simulbot, chname) && !strcmp(dcc[i].nick, handle)) ||
+       (((chname && chname[0]) && !strcmp(dcc[i].simulbot, chname) &&
+         user && !strcmp(dcc[i].nick, user->handle)) ||
        (!(chname && chname[0]) && !strcmp(dcc[i].simulbot, nick)))) {
       putlog(LOG_DEBUG, "*", STR("Simul found old idx for %s/%s: (%s!%s)"), nick, chname, nick, host);
       dcc[i].simultime = now;
@@ -214,6 +219,8 @@ sdprintf(STR("GETIDX: auth: %s, idx: %d"), nick, idx);
   idx = new_dcc(&DCC_CHAT, sizeof(struct chat_info));
 
   if (idx != -1) {
+    char from[NICKLEN + UHOSTLEN];
+
     dcc[idx].sock = -1;
     dcc[idx].timeval = now;
     dcc[idx].irc = 1;
@@ -224,10 +231,11 @@ sdprintf(STR("GETIDX: auth: %s, idx: %d"), nick, idx);
     strlcpy(dcc[idx].simulbot, nick, sizeof(dcc[idx].simulbot));
     strlcpy(dcc[idx].u.chat->con_chan, chname ? chname : "*", sizeof(dcc[idx].u.chat->con_chan));
     dcc[idx].u.chat->strip_flags = STRIP_ALL;
-    strlcpy(dcc[idx].nick, handle, sizeof(dcc[idx].nick));
+    strlcpy(dcc[idx].nick, user ? user->handle : "*", sizeof(dcc[idx].nick));
     strlcpy(dcc[idx].host, host, sizeof(dcc[idx].host));
     dcc[idx].addr = 0L;
-    dcc[idx].user = user ? user : get_user_by_handle(userlist, handle);
+    simple_snprintf(from, sizeof(from), "%s!%s", nick, host);
+    dcc[idx].user = user ? user : get_user_by_host(from);
     return 1;
   }
 
@@ -240,7 +248,8 @@ static void auth_tell_block(const bd::String key, Auth* auth, void* param)
   int idx = (int) lparam;
 
   dprintf(idx, "(%s!%s) [%s] authtime: %li, atime: %li, Status: %d\n", auth->nick, 
-        auth->host, auth->handle, (long)auth->authtime, (long)auth->atime, auth->Status());
+        auth->host, auth->user ? auth->user->handle : "*",
+        (long)auth->authtime, (long)auth->atime, auth->Status());
 }
 
 void Auth::TellAuthed(int idx)

src/auth.h

@@ -25,9 +25,8 @@ class Auth {
   void NewNick(const char *nick);
 
   static Auth *Find(const char * host);
-  static Auth *Find(const char * handle, bool _hand);
-  static void NullUsers();
-  static void FillUsers();
+  static void NullUsers(const char *nick = NULL);
+  static void FillUsers(const char *nick = NULL);
   static void ExpireAuths();
   static void InitTimer();
   static void DeleteAll();
@@ -41,10 +40,9 @@ class Auth {
   char rand[51];
   char nick[NICKLEN];
   char host[UHOSTLEN];
-  char handle[HANDLEN + 1];
 
-  static bd::HashTable<bd::String, Auth*> ht_handle;
   static bd::HashTable<bd::String, Auth*> ht_host;
+  static bd::HashTable<bd::String, Auth*> ht_nick;
 
   private:
   int status;

src/chanprog.cc

@@ -166,24 +166,6 @@ struct userrec *check_chanlist_hand(const char *hand)
   return NULL;
 }
 
-/* Clear the user pointers in the chanlists.
- *
- * Necessary when a hostmask is added/removed, a user is added or a new
- * userfile is loaded.
- */
-void clear_chanlist(void)
-{
-  memberlist		*m = NULL;
-  struct chanset_t	*chan = NULL;
-
-  for (chan = chanset; chan; chan = chan->next)
-    for (m = chan->channel.member; m && m->nick[0]; m = m->next) {
-      m->user = NULL;
-      m->tried_getuser = 0;
-    }
-
-}
-
 /* Clear the user pointer of a specific nick in the chanlists.
  *
  * Necessary when a hostmask is added/removed, a nick changes, etc.
@@ -194,13 +176,19 @@ void clear_chanlist_member(const char *nick)
   memberlist		*m = NULL;
   struct chanset_t	*chan = NULL;
 
-  for (chan = chanset; chan; chan = chan->next)
-    for (m = chan->channel.member; m && m->nick[0]; m = m->next)
-      if (!rfc_casecmp(m->nick, nick)) {
+  for (chan = chanset; chan; chan = chan->next) {
+    for (m = chan->channel.member; m && m->nick[0]; m = m->next) {
+      if (nick == NULL || !rfc_casecmp(m->nick, nick)) {
 	m->user = NULL;
         m->tried_getuser = 0;
-	break;
+        if (nick != NULL) {
+          break;
+        }
       }
+    }
+  }
+
+  Auth::NullUsers(nick);
 }
 
 /* If this user@host is in a channel, set it (it was null)
@@ -668,15 +656,13 @@ void reload()
   else if (!conf.bot->hub)
     add_localhub();
 
+  cache_users();
+
   /* Make sure no removed users/bots are still connected. */
   check_stale_dcc_users();
 
-  for (tand_t* bot = tandbot; bot; bot = bot->next)
-    bot->u = get_user_by_handle(userlist, bot->bot);
-
   /* I don't think these will ever be called anyway. */
   if (!conf.bot->hub) {
-    Auth::FillUsers();
     check_hostmask();
   }
 

src/chanprog.h

@@ -21,8 +21,8 @@ void check_timers();
 void check_utimers();
 void rmspace(char *s);
 void set_chanlist(const char *host, struct userrec *rec);
-void clear_chanlist(void);
 void clear_chanlist_member(const char *nick);
+#define clear_chanlist() clear_chanlist_member(NULL)
 bool bot_shouldjoin(struct userrec* , struct flag_record *, const struct chanset_t *, bool = 0);
 bool shouldjoin(const struct chanset_t *);
 char *samechans(const char *, const char *);

src/mod/irc.mod/irc.cc

@@ -1168,6 +1168,9 @@ killmember(struct chanset_t *chan, char *nick, bool cacheMember)
 
   if (cacheMember) {
     x->last = now;
+    x->user = NULL;
+    x->next = NULL;
+    x->tried_getuser = 0;
     // Don't delete here, will delete when it expires from the cache.
     (*chan->channel.cached_members)[x->userhost] = x;
   } else {

src/mod/irc.mod/msgcmds.cc

@@ -294,9 +294,11 @@ static int msg_invite(char *nick, char *host, struct userrec *u, char *par)
 static void logc(const char *cmd, Auth *a, char *chname, char *par)
 {
   if (chname && chname[0])
-    putlog(LOG_CMDS, "*", "(%s!%s) !%s! %s %c%s %s", a->nick, a->host, a->handle, chname, auth_prefix[0], cmd, par ? par : "");
+    putlog(LOG_CMDS, "*", "(%s!%s) !%s! %s %c%s %s", a->nick, a->host,
+        a->user ? a->user->handle : "*", chname, auth_prefix[0], cmd, par ? par : "");
   else
-    putlog(LOG_CMDS, "*", "(%s!%s) !%s! %c%s %s", a->nick, a->host, a->handle, auth_prefix[0], cmd, par ? par : "");
+    putlog(LOG_CMDS, "*", "(%s!%s) !%s! %c%s %s", a->nick, a->host,
+        a->user ? a->user->handle : "*", auth_prefix[0], cmd, par ? par : "");
 }
 #define LOGC(cmd) logc(cmd, a, chname, par)
   
@@ -338,7 +340,7 @@ static int msg_authstart(char *nick, char *host, struct userrec *u, char *par)
 static void
 AuthFinish(Auth *auth)
 {
-  putlog(LOG_CMDS, "*", STR("(%s!%s) !%s! +AUTH"), auth->nick, auth->host, auth->handle);
+  putlog(LOG_CMDS, "*", STR("(%s!%s) !%s! +AUTH"), auth->nick, auth->host, auth->user ? auth->user->handle : "*");
   auth->Done();
   bd::String msg;
   msg = bd::String::printf(STR("You are now authorized for cmds, see %chelp"), auth_prefix[0]);

src/mod/share.mod/share.cc

@@ -1343,7 +1343,6 @@ finish_share(int idx)
 static void share_read_stream(int idx, bd::Stream& stream) {
   struct userrec *u = NULL, *ou = NULL;
   struct chanset_t *chan = NULL;
-  int i;
 
   /*
    * This is where we remove all global and channel bans/exempts/invites and
@@ -1374,23 +1373,7 @@ static void share_read_stream(int idx, bd::Stream& stream) {
   //userlist = (struct userrec *) -1;       /* Do this to prevent .user messups     */
   userlist = NULL;
 
-  /* Bot user pointers are updated to point to the new list, all others
-   * are set to NULL. If our userfile will be overriden, just set _all_
-   * to NULL directly.
-   */
-  for (i = 0; i < dcc_total; i++)
-    if (dcc[i].type)
-      dcc[i].user = NULL;
-
-  for (tand_t* bot = tandbot; bot; bot = bot->next)
-    bot->u = NULL;
-
-  if (!conf.bot->hub) {
-    Auth::NullUsers();
-  }
-
-  if (conf.bot->u)
-    conf.bot->u = NULL;
+  clear_cached_users();
 
   struct cmd_pass *old_cmdpass = cmdpass;
   cmdpass = NULL;
@@ -1407,22 +1390,11 @@ static void share_read_stream(int idx, bd::Stream& stream) {
 
     Context;
     clear_userlist(u);          /* Clear new, obsolete, user list.      */
-    clear_chanlist();           /* Remove all user references from the
-                                 * channel lists.                       */
-    for (i = 0; i < dcc_total; i++)
-      if (dcc[i].type)
-        dcc[i].user = get_user_by_handle(ou, dcc[i].nick);
-
-    for (tand_t* bot = tandbot; bot; bot = bot->next)
-      bot->u = get_user_by_handle(ou, bot->bot);
-
-    conf.bot->u = get_user_by_handle(ou, conf.bot->nick);
 
     userlist = ou;              /* Revert to old user list.             */
     lastuser = NULL;            /* Reset last accessed user ptr.        */
 
-    Auth::FillUsers();
-
+    cache_users();
     cmdpass = old_cmdpass;
 
     checkchans(2);              /* un-flag the channels, we are keeping them.. */
@@ -1443,8 +1415,7 @@ static void share_read_stream(int idx, bd::Stream& stream) {
   /* SUCCESS! */
 
   loading = 0;
-  clear_chanlist();             /* Remove all user references from the
-                                 * channel lists.                       */
+
   userlist = u;                 /* Set new user list.                   */
   lastuser = NULL;              /* Reset last accessed user ptr.        */
   putlog(LOG_BOTS, "*", "%s.", "Userlist transfer complete; switched over");
@@ -1462,18 +1433,11 @@ static void share_read_stream(int idx, bd::Stream& stream) {
   if (conf.bot->localhub)
     add_child_bots();
 
+  cache_users();
+
   /* Make sure no removed users/bots are still connected. */
   check_stale_dcc_users();
 
-  /* Refill tand list with cached user entries */
-  for (tand_t* bot = tandbot; bot; bot = bot->next)
-    bot->u = get_user_by_handle(userlist, bot->bot);
-
-  if (!conf.bot->hub) {  
-    /* copy over any auth users */
-    Auth::FillUsers();
-  }
-
   write_userfile(-1);
 
   cmdpass_free(old_cmdpass);

src/userrec.cc

@@ -172,6 +172,53 @@ void clear_masks(maskrec *m)
 
 static void freeuser(struct userrec *);
 
+void clear_cached_users()
+{
+  for (int i = 0; i < dcc_total; i++) {
+    if (dcc[i].type) {
+      dcc[i].user = NULL;
+    }
+  }
+
+  conf.bot->u = NULL;
+
+  for (conf_bot *bot = conf.bots; bot; bot = bot->next) {
+    bot->u = NULL;
+  }
+
+  for (tand_t* bot = tandbot; bot; bot = bot->next) {
+    bot->u = NULL;
+  }
+
+  if (!conf.bot->hub) {
+    clear_chanlist();           /* Remove all user references from the
+                                 * channel lists.                       */
+  }
+}
+
+void cache_users()
+{
+  for (int i = 0; i < dcc_total; i++) {
+    if (dcc[i].type) {
+      dcc[i].user = get_user_by_handle(userlist, dcc[i].nick);
+    }
+  }
+
+  conf.bot->u = get_user_by_handle(userlist, conf.bot->nick);
+
+  for (conf_bot *bot = conf.bots; bot; bot = bot->next) {
+    bot->u = get_user_by_handle(userlist, bot->nick);
+  }
+
+  for (tand_t* bot = tandbot; bot; bot = bot->next) {
+    bot->u = get_user_by_handle(userlist, bot->bot);
+  }
+
+  if (!conf.bot->hub) {
+    Auth::FillUsers();
+  }
+}
+
 void clear_userlist(struct userrec *bu)
 {
   struct userrec *v = NULL;
@@ -183,13 +230,7 @@ void clear_userlist(struct userrec *bu)
   if (userlist == bu) {
     struct chanset_t *cst = NULL;
 
-    for (int i = 0; i < dcc_total; i++)
-      if (dcc[i].type)
-        dcc[i].user = NULL;
-
-    conf.bot->u = NULL;
-
-    clear_chanlist();
+    clear_cached_users();
     lastuser = NULL;
 
     while (global_ign)

src/userrec.h

@@ -8,6 +8,8 @@ namespace bd {
 struct userrec *adduser(struct userrec *, const char *, char *, char *, flag_t, int);
 void addhost_by_handle(char *, char *);
 void clear_masks(struct maskrec *);
+void clear_cached_users();
+void cache_users();
 void clear_userlist(struct userrec *);
 int u_pass_match(struct userrec *, const char *);
 int delhost_by_handle(char *, char *);