Bug#14003080:65104: MAX_USER_CONNECTIONS WITH PROCESSLIST EMPTY

Analysis:
-------------
If server is started with limit of MAX_CONNECTIONS and 
MAX_USER_CONNECTIONS then only MAX_USER_CONNECTIONS of any particular
users can be connected to server and total MAX_CONNECTIONS of client can
be connected to server.

Server maintains a counter for total CONNECTIONS and total CONNECTIONS 
from particular user.

Here, MAX_CONNECTIONS of connections are created to server. Out of this
MAX_CONNECTIONS, connections from particular user (say USER1) are
also created. The connections from USER1 is lesser than 
MAX_USER_CONNECTIONS. After that there was one more connection request from
USER1. Since USER1 can still create connections as he havent reached
MAX_USER_CONNECTIONS, server increments counter of CONNECTIONS per user.
As server already has MAX_CONNECTIONS of connections, next check to total
CONNECTION count fails. In this case control is returned WITHOUT 
decrementing the CONNECTIONS per user. So the counter per user CONNECTIONS goes
on incrementing for each attempt until current connections are closed. 
And because of this counter per CONNECTIONS reached MAX_USER_CONNECTIONS. 
So, next connections form USER1 user always returns with MAX_USER_CONNECTION 
limit error, even when total connection to sever are less than MAX_CONNECTIONS.

Fix:
-------------
This issue is occurred because of not handling counters properly in the
server. Changed the code to handle per user connection counters properly.

sql/sql_acl.cc

@@ -7810,8 +7810,6 @@ get_cached_table_access(GRANT_INTERNAL_INFO *grant_internal_info,
 #undef HAVE_OPENSSL
 #ifdef NO_EMBEDDED_ACCESS_CHECKS
 #define initialized 0
-#define decrease_user_connections(X)        /* nothing */
-#define check_for_max_user_connections(X, Y)   0
 #endif
 #endif
 #ifndef HAVE_OPENSSL
@@ -9297,7 +9295,7 @@ acl_authenticate(THD *thd, uint connect_errors, uint com_change_user_pkt_len)
     mpvio.packets_read++;    // take COM_CHANGE_USER packet into account
 
     /* Clear variables that are allocated */
-    thd->user_connect= 0;
+    thd->set_user_connect(NULL);
 
     if (parse_com_change_user_packet(&mpvio, com_change_user_pkt_len))
     {
@@ -9460,11 +9458,11 @@ acl_authenticate(THD *thd, uint connect_errors, uint com_change_user_pkt_len)
   else
     sctx->skip_grants();
 
-  if (thd->user_connect &&
-      (thd->user_connect->user_resources.conn_per_hour ||
-       thd->user_connect->user_resources.user_conn ||
+  const USER_CONN *uc;
+  if ((uc= thd->get_user_connect()) &&
+      (uc->user_resources.conn_per_hour || uc->user_resources.user_conn ||
        global_system_variables.max_user_connections) &&
-      check_for_max_user_connections(thd, thd->user_connect))
+       check_for_max_user_connections(thd, uc))
   {
     DBUG_RETURN(1); // The error is set in check_for_max_user_connections()
   }
@@ -9486,6 +9484,7 @@ acl_authenticate(THD *thd, uint connect_errors, uint com_change_user_pkt_len)
     mysql_mutex_unlock(&LOCK_connection_count);
     if (!count_ok)
     {                                         // too many connections
+      release_user_connection(thd);
       my_error(ER_CON_COUNT_ERROR, MYF(0));
       DBUG_RETURN(1);
     }
@@ -9504,11 +9503,7 @@ acl_authenticate(THD *thd, uint connect_errors, uint com_change_user_pkt_len)
     if (mysql_change_db(thd, &mpvio.db, FALSE))
     {
       /* mysql_change_db() has pushed the error message. */
-      if (thd->user_connect)
-      {
-        decrease_user_connections(thd->user_connect);
-        thd->user_connect= 0;
-      }
+      release_user_connection(thd);
       DBUG_RETURN(1);
     }
   }

sql/sql_class.cc

@@ -852,7 +852,7 @@ THD::THD()
 #if defined(ENABLED_PROFILING)
   profiling.set_thd(this);
 #endif
-  user_connect=(USER_CONN *)0;
+  m_user_connect= NULL;
   my_hash_init(&user_vars, system_charset_info, USER_VARS_HASH_SIZE, 0, 0,
                (my_hash_get_key) get_var_key,
                (my_hash_free_key) free_user_var, 0);
@@ -5009,3 +5009,87 @@ bool Discrete_intervals_list::append(Discrete_interval *new_interval)
 }
 
 #endif /* !defined(MYSQL_CLIENT) */
+
+void THD::set_user_connect(USER_CONN *uc)
+{
+  DBUG_ENTER("THD::set_user_connect");
+
+  m_user_connect= uc;
+
+  DBUG_VOID_RETURN;
+}
+
+void THD::increment_user_connections_counter()
+{
+  DBUG_ENTER("THD::increment_user_connections_counter");
+
+  m_user_connect->connections++;
+
+  DBUG_VOID_RETURN;
+}
+
+void THD::decrement_user_connections_counter()
+{
+  DBUG_ENTER("THD::decrement_user_connections_counter");
+
+  DBUG_ASSERT(m_user_connect->connections > 0);
+  m_user_connect->connections--;
+
+  DBUG_VOID_RETURN;
+}
+
+void THD::increment_con_per_hour_counter()
+{
+  DBUG_ENTER("THD::decrement_conn_per_hour_counter");
+
+  m_user_connect->conn_per_hour++;
+
+  DBUG_VOID_RETURN;
+}
+
+void THD::increment_updates_counter()
+{
+  DBUG_ENTER("THD::increment_updates_counter");
+
+  m_user_connect->updates++;
+
+  DBUG_VOID_RETURN;
+}
+
+void THD::increment_questions_counter()
+{
+  DBUG_ENTER("THD::increment_updates_counter");
+
+  m_user_connect->questions++;
+
+  DBUG_VOID_RETURN;
+}
+
+/*
+  Reset per-hour user resource limits when it has been more than
+  an hour since they were last checked
+
+  SYNOPSIS:
+    time_out_user_resource_limits()
+
+  NOTE:
+    This assumes that the LOCK_user_conn mutex has been acquired, so it is
+    safe to test and modify members of the USER_CONN structure.
+*/
+void THD::time_out_user_resource_limits()
+{
+  mysql_mutex_assert_owner(&LOCK_user_conn);
+  ulonglong check_time= start_utime;
+  DBUG_ENTER("time_out_user_resource_limits");
+
+  /* If more than a hour since last check, reset resource checking */
+  if (check_time - m_user_connect->reset_utime >= LL(3600000000))
+  {
+    m_user_connect->questions=1;
+    m_user_connect->updates=0;
+    m_user_connect->conn_per_hour=0;
+    m_user_connect->reset_utime= check_time;
+  }
+
+  DBUG_VOID_RETURN;
+}

sql/sql_class.h

@@ -1960,7 +1960,26 @@ class THD :public Statement,
   */
   ha_rows    examined_row_count;
 
-  USER_CONN *user_connect;
+private:
+  USER_CONN *m_user_connect;
+
+public:
+  void set_user_connect(USER_CONN *uc);
+  const USER_CONN* get_user_connect()
+  { return m_user_connect; }
+
+  void increment_user_connections_counter();
+  void decrement_user_connections_counter();
+
+  void increment_con_per_hour_counter();
+
+  void increment_updates_counter();
+
+  void increment_questions_counter();
+
+  void time_out_user_resource_limits();
+
+public:
   CHARSET_INFO *db_charset;
   Warning_info *warning_info;
   Diagnostics_area *stmt_da;

sql/sql_connect.cc

@@ -105,8 +105,8 @@ int get_or_create_user_conn(THD *thd, const char *user,
       goto end;
     }
   }
-  thd->user_connect=uc;
-  uc->connections++;
+  thd->set_user_connect(uc);
+  thd->increment_user_connections_counter();
 end:
   mysql_mutex_unlock(&LOCK_user_conn);
   return return_val;
@@ -131,7 +131,7 @@ int get_or_create_user_conn(THD *thd, const char *user,
     1	error
 */
 
-int check_for_max_user_connections(THD *thd, USER_CONN *uc)
+int check_for_max_user_connections(THD *thd, const USER_CONN *uc)
 {
   int error=0;
   DBUG_ENTER("check_for_max_user_connections");
@@ -145,7 +145,7 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc)
     error=1;
     goto end;
   }
-  time_out_user_resource_limits(thd, uc);
+  thd->time_out_user_resource_limits();
   if (uc->user_resources.user_conn &&
       uc->user_resources.user_conn < uc->connections)
   {
@@ -164,18 +164,18 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc)
     error=1;
     goto end;
   }
-  uc->conn_per_hour++;
+  thd->increment_con_per_hour_counter();
 
 end:
   if (error)
   {
-    uc->connections--; // no need for decrease_user_connections() here
+    thd->decrement_user_connections_counter();
     /*
       The thread may returned back to the pool and assigned to a user
       that doesn't have a limit. Ensure the user is not using resources
       of someone else.
     */
-    thd->user_connect= NULL;
+    thd->set_user_connect(NULL);
   }
   mysql_mutex_unlock(&LOCK_user_conn);
   DBUG_RETURN(error);
@@ -214,38 +214,37 @@ void decrease_user_connections(USER_CONN *uc)
   DBUG_VOID_RETURN;
 }
 
-
 /*
-  Reset per-hour user resource limits when it has been more than
-  an hour since they were last checked
-
-  SYNOPSIS:
-    time_out_user_resource_limits()
-    thd			Thread handler
-    uc			User connection details
+   Decrements user connections count from the USER_CONN held by THD
+   And removes USER_CONN from the hash if no body else is using it.
 
-  NOTE:
-    This assumes that the LOCK_user_conn mutex has been acquired, so it is
-    safe to test and modify members of the USER_CONN structure.
-*/
-
-void time_out_user_resource_limits(THD *thd, USER_CONN *uc)
+   SYNOPSIS
+     release_user_connection()
+     THD  Thread context object.
+ */
+void release_user_connection(THD *thd)
 {
-  ulonglong check_time= thd->start_utime;
-  DBUG_ENTER("time_out_user_resource_limits");
+  const USER_CONN *uc= thd->get_user_connect();
+  DBUG_ENTER("release_user_connection");
 
-  /* If more than a hour since last check, reset resource checking */
-  if (check_time  - uc->reset_utime >= LL(3600000000))
+  if (uc)
   {
-    uc->questions=1;
-    uc->updates=0;
-    uc->conn_per_hour=0;
-    uc->reset_utime= check_time;
+    mysql_mutex_lock(&LOCK_user_conn);
+    DBUG_ASSERT(uc->connections > 0);
+    thd->decrement_user_connections_counter();
+    if (!uc->connections && !mqh_used)
+    {
+      /* Last connection for user; Delete it */
+      (void) my_hash_delete(&hash_user_connections,(uchar*) uc);
+    }
+    mysql_mutex_unlock(&LOCK_user_conn);
+    thd->set_user_connect(NULL);
   }
 
   DBUG_VOID_RETURN;
 }
 
+
 /*
   Check if maximum queries per hour limit has been reached
   returns 0 if OK.
@@ -254,29 +253,34 @@ void time_out_user_resource_limits(THD *thd, USER_CONN *uc)
 bool check_mqh(THD *thd, uint check_command)
 {
   bool error= 0;
-  USER_CONN *uc=thd->user_connect;
+  const USER_CONN *uc=thd->get_user_connect();
   DBUG_ENTER("check_mqh");
   DBUG_ASSERT(uc != 0);
 
   mysql_mutex_lock(&LOCK_user_conn);
 
-  time_out_user_resource_limits(thd, uc);
+  thd->time_out_user_resource_limits();
 
   /* Check that we have not done too many questions / hour */
-  if (uc->user_resources.questions &&
-      uc->questions++ >= uc->user_resources.questions)
+  if (uc->user_resources.questions)
+  {
+    thd->increment_questions_counter();
+    if ((uc->questions - 1) >= uc->user_resources.questions)
     {
       my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, "max_questions",
                (long) uc->user_resources.questions);
       error=1;
       goto end;
     }
+  }
   if (check_command < (uint) SQLCOM_END)
   {
     /* Check that we have not done too many updates / hour */
     if (uc->user_resources.updates &&
-        (sql_command_flags[check_command] & CF_CHANGES_DATA) &&
-	uc->updates++ >= uc->user_resources.updates)
+        (sql_command_flags[check_command] & CF_CHANGES_DATA))
+    {
+      thd->increment_updates_counter();
+      if ((uc->updates - 1) >= uc->user_resources.updates)
       {
         my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, "max_updates",
                  (long) uc->user_resources.updates);
@@ -284,10 +288,35 @@ bool check_mqh(THD *thd, uint check_command)
         goto end;
       }
     }
+  }
 end:
   mysql_mutex_unlock(&LOCK_user_conn);
   DBUG_RETURN(error);
 }
+#else
+
+int check_for_max_user_connections(THD *thd, const USER_CONN *uc)
+{
+  return 0;
+}
+
+void decrease_user_connections(USER_CONN *uc)
+{
+  return;
+}
+
+void release_user_connection(THD *thd)
+{
+  const USER_CONN *uc= thd->get_user_connect();
+  DBUG_ENTER("release_user_connection");
+
+  if (uc)
+  {
+    thd->set_user_connect(NULL);
+  }
+
+  DBUG_VOID_RETURN;
+}
 
 #endif /* NO_EMBEDDED_ACCESS_CHECKS */
 
@@ -609,16 +638,13 @@ void end_connection(THD *thd)
 {
   NET *net= &thd->net;
   plugin_thdvar_cleanup(thd);
-  if (thd->user_connect)
-  {
-    decrease_user_connections(thd->user_connect);
+
   /*
     The thread may returned back to the pool and assigned to a user
     that doesn't have a limit. Ensure the user is not using resources
     of someone else.
   */
-    thd->user_connect= NULL;
-  }
+  release_user_connection(thd);
 
   if (thd->killed || (net->error && net->vio != 0))
   {

sql/sql_connect.h

@@ -33,6 +33,7 @@ void reset_mqh(LEX_USER *lu, bool get_them);
 bool check_mqh(THD *thd, uint check_command);
 void time_out_user_resource_limits(THD *thd, USER_CONN *uc);
 void decrease_user_connections(USER_CONN *uc);
+void release_user_connection(THD *thd);
 bool thd_init_client_charset(THD *thd, uint cs_number);
 bool setup_connection_thread_globals(THD *thd);
 bool thd_prepare_connection(THD *thd);
@@ -47,6 +48,6 @@ void prepare_new_connection_state(THD* thd);
 void end_connection(THD *thd);
 int get_or_create_user_conn(THD *thd, const char *user,
                             const char *host, const USER_RESOURCES *mqh);
-int check_for_max_user_connections(THD *thd, USER_CONN *uc);
+int check_for_max_user_connections(THD *thd, const USER_CONN *uc);
 
 #endif /* SQL_CONNECT_INCLUDED */

sql/sql_parse.cc

@@ -949,7 +949,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
 
     uint save_db_length= thd->db_length;
     char *save_db= thd->db;
-    USER_CONN *save_user_connect= thd->user_connect;
+    USER_CONN *save_user_connect=
+      const_cast<USER_CONN*>(thd->get_user_connect());
     Security_context save_security_ctx= *thd->security_ctx;
     CHARSET_INFO *save_character_set_client=
       thd->variables.character_set_client;
@@ -964,7 +965,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
     {
       my_free(thd->security_ctx->user);
       *thd->security_ctx= save_security_ctx;
-      thd->user_connect= save_user_connect;
+      thd->set_user_connect(save_user_connect);
       thd->reset_db (save_db, save_db_length);
       thd->variables.character_set_client= save_character_set_client;
       thd->variables.collation_connection= save_collation_connection;
@@ -5583,7 +5584,7 @@ void mysql_parse(THD *thd, char *rawbuf, uint length,
     if (!err)
     {
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
-      if (mqh_used && thd->user_connect &&
+      if (mqh_used && thd->get_user_connect() &&
 	  check_mqh(thd, lex->sql_command))
       {
 	thd->net.error = 0;

sql/sys_vars.h

@@ -799,8 +799,9 @@ class Sys_var_max_user_conn: public Sys_var_uint
   { }
   uchar *session_value_ptr(THD *thd, LEX_STRING *base)
   {
-    if (thd->user_connect && thd->user_connect->user_resources.user_conn)
-      return (uchar*) &(thd->user_connect->user_resources.user_conn);
+    const USER_CONN *uc= thd->get_user_connect();
+    if (uc && uc->user_resources.user_conn)
+      return (uchar*) &(uc->user_resources.user_conn);
     return global_value_ptr(thd, base);
   }
 };