Fix for Bug#33507: Event scheduler creates more threads

than max_connections -- which results in user lockout.

The problem was that the variable thread_count that contains
the number of active threads was interpreted as a number of
active connections.

The fix is to introduce a new counter for active connections.

5 files changed, 231 insertions, 14 deletions

diff --git a/mysql-test/r/connect.result b/mysql-test/r/connect.result
index 25cf4f90e6d..de4eb28c500 100644
--- a/mysql-test/r/connect.result
+++ b/mysql-test/r/connect.result
@@ -115,3 +115,61 @@ create temporary table t2(id integer not null auto_increment primary key);
 set @id := 1;
 delete from t1 where id like @id;
 drop table t1;
+# ------------------------------------------------------------------
+# -- End of 4.1 tests
+# ------------------------------------------------------------------
+
+# -- Bug#33507: Event scheduler creates more threads than max_connections
+# -- which results in user lockout.
+
+GRANT USAGE ON *.* TO mysqltest_u1@localhost;
+
+SET GLOBAL max_connections = 3;
+SET GLOBAL event_scheduler = ON;
+
+# -- Waiting for old connections to close...
+
+
+# -- Disconnecting default connection...
+
+# -- Check that we allow exactly three user connections, no matter how
+# -- many threads are running.
+
+# -- Connecting (1)...
+
+# -- Waiting for root connection to close...
+
+# -- Connecting (2)...
+# -- Connecting (3)...
+# -- Connecting (4)...
+ERROR 08004: Too many connections
+
+# -- Waiting for the last connection to close...
+
+# -- Check that we allow one extra SUPER-user connection.
+
+# -- Connecting super (1)...
+# -- Connecting super (2)...
+ERROR 00000: Too many connections
+
+SELECT user FROM information_schema.processlist ORDER BY id;
+user
+event_scheduler
+mysqltest_u1
+mysqltest_u1
+mysqltest_u1
+root
+
+# -- Resetting variables...
+SET GLOBAL max_connections = 151;
+SET GLOBAL event_scheduler = OFF;
+
+# -- That's it. Closing connections...
+
+# -- Restoring default connection...
+
+# -- End of Bug#33507.
+
+# ------------------------------------------------------------------
+# -- End of 5.1 tests
+# ------------------------------------------------------------------
diff --git a/mysql-test/t/connect.test b/mysql-test/t/connect.test
index 2e66c24d877..9e8f0d9b115 100644
--- a/mysql-test/t/connect.test
+++ b/mysql-test/t/connect.test
@@ -102,4 +102,129 @@ disconnect con7;
 connection default;
 drop table t1;
 
-# End of 4.1 tests
+--disconnect con1
+--disconnect con2
+--disconnect con3
+--disconnect con4
+--disconnect con5
+--disconnect con6
+--disconnect con10
+
+--echo # ------------------------------------------------------------------
+--echo # -- End of 4.1 tests
+--echo # ------------------------------------------------------------------
+
+--echo
+--echo # -- Bug#33507: Event scheduler creates more threads than max_connections
+--echo # -- which results in user lockout.
+--echo
+
+GRANT USAGE ON *.* TO mysqltest_u1@localhost;
+
+# NOTE: if the test case fails sporadically due to spurious connections,
+# consider disabling all users.
+
+--echo
+
+let $saved_max_connections = `SELECT @@global.max_connections`;
+
+SET GLOBAL max_connections = 3;
+SET GLOBAL event_scheduler = ON;
+
+--echo
+--echo # -- Waiting for old connections to close...
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+
+--echo
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE user = 'event_scheduler';
+--source include/wait_condition.inc
+--echo
+
+--echo # -- Disconnecting default connection...
+--disconnect default
+
+--echo
+--echo # -- Check that we allow exactly three user connections, no matter how
+--echo # -- many threads are running.
+--echo 
+
+--echo # -- Connecting (1)...
+--connect (con_1,localhost,mysqltest_u1)
+
+--echo
+--echo # -- Waiting for root connection to close...
+let $wait_condition =
+  SELECT COUNT(*) = 1
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+--echo
+
+--echo # -- Connecting (2)...
+--connect (con_2,localhost,mysqltest_u1)
+
+--echo # -- Connecting (3)...
+--connect (con_3,localhost,mysqltest_u1)
+
+--echo # -- Connecting (4)...
+--disable_query_log
+--error ER_CON_COUNT_ERROR
+--connect (con_4,localhost,mysqltest_u1)
+--enable_query_log
+
+--echo
+--echo # -- Waiting for the last connection to close...
+let $wait_condition =
+  SELECT COUNT(*) = 3
+  FROM information_schema.processlist
+  WHERE db = 'test';
+--source include/wait_condition.inc
+
+--echo
+--echo # -- Check that we allow one extra SUPER-user connection.
+--echo
+
+--echo # -- Connecting super (1)...
+--connect (con_super_1,localhost,root)
+
+--echo # -- Connecting super (2)...
+--disable_query_log
+--error ER_CON_COUNT_ERROR
+--connect (con_super_2,localhost,root)
+--enable_query_log
+
+--echo
+# Ensure that we have Event Scheduler thread, 3 ordinary user connections and
+# one extra super-user connection.
+SELECT user FROM information_schema.processlist ORDER BY id;
+
+--echo
+--echo # -- Resetting variables...
+
+--eval SET GLOBAL max_connections = $saved_max_connections
+SET GLOBAL event_scheduler = OFF;
+
+--echo
+--echo # -- That's it. Closing connections...
+--disconnect con_1
+--disconnect con_2
+--disconnect con_super_1
+
+--echo
+--echo # -- Restoring default connection...
+--connect (default,localhost,root,,test)
+
+--echo
+--echo # -- End of Bug#33507.
+--echo
+
+--echo # ------------------------------------------------------------------
+--echo # -- End of 5.1 tests
+--echo # ------------------------------------------------------------------
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index b52e5aa745c..de40004fc6d 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -974,8 +974,6 @@ void time_out_user_resource_limits(THD *thd, USER_CONN *uc);
 void decrease_user_connections(USER_CONN *uc);
 void thd_init_client_charset(THD *thd, uint cs_number);
 bool setup_connection_thread_globals(THD *thd);
-bool login_connection(THD *thd);
-void end_connection(THD *thd);
 
 bool mysql_create_db(THD *thd, char *db, HA_CREATE_INFO *create, bool silent);
 bool mysql_alter_db(THD *thd, const char *db, HA_CREATE_INFO *create);
@@ -1895,6 +1893,7 @@ extern bool opt_disable_networking, opt_skip_show_db;
 extern my_bool opt_character_set_client_handshake;
 extern bool volatile abort_loop, shutdown_in_progress;
 extern uint volatile thread_count, thread_running, global_read_lock;
+extern uint connection_count;
 extern my_bool opt_sql_bin_update, opt_safe_user_create, opt_no_mix_types;
 extern my_bool opt_safe_show_db, opt_local_infile, opt_myisam_use_mmap;
 extern my_bool opt_slave_compressed_protocol, use_temp_pool;
@@ -1933,7 +1932,7 @@ extern pthread_mutex_t LOCK_mysql_create_db,LOCK_Acl,LOCK_open, LOCK_lock_db,
        LOCK_slave_list, LOCK_active_mi, LOCK_manager, LOCK_global_read_lock,
        LOCK_global_system_variables, LOCK_user_conn,
        LOCK_prepared_stmt_count,
-       LOCK_bytes_sent, LOCK_bytes_received;
+       LOCK_bytes_sent, LOCK_bytes_received, LOCK_connection_count;
 #ifdef HAVE_OPENSSL
 extern pthread_mutex_t LOCK_des_key_file;
 #endif
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index cdd08be6573..fd9ce9e1cea 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -584,7 +584,8 @@ pthread_mutex_t LOCK_mysql_create_db, LOCK_Acl, LOCK_open, LOCK_thread_count,
 		LOCK_delayed_insert, LOCK_delayed_status, LOCK_delayed_create,
 		LOCK_crypt, LOCK_bytes_sent, LOCK_bytes_received,
 	        LOCK_global_system_variables,
-		LOCK_user_conn, LOCK_slave_list, LOCK_active_mi;
+		LOCK_user_conn, LOCK_slave_list, LOCK_active_mi,
+                LOCK_connection_count;
 /**
   The below lock protects access to two global server variables:
   max_prepared_stmt_count and prepared_stmt_count. These variables
@@ -720,6 +721,11 @@ char *des_key_file;
 struct st_VioSSLFd *ssl_acceptor_fd;
 #endif /* HAVE_OPENSSL */
 
+/**
+  Number of currently active user connections. The variable is protected by
+  LOCK_connection_count.
+*/
+uint connection_count= 0;
 
 /* Function declarations */
 
@@ -1341,6 +1347,7 @@ static void clean_up_mutexes()
   (void) pthread_mutex_destroy(&LOCK_bytes_sent);
   (void) pthread_mutex_destroy(&LOCK_bytes_received);
   (void) pthread_mutex_destroy(&LOCK_user_conn);
+  (void) pthread_mutex_destroy(&LOCK_connection_count);
   Events::destroy_mutexes();
 #ifdef HAVE_OPENSSL
   (void) pthread_mutex_destroy(&LOCK_des_key_file);
@@ -1783,6 +1790,11 @@ void unlink_thd(THD *thd)
   DBUG_ENTER("unlink_thd");
   DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd));
   thd->cleanup();
+
+  pthread_mutex_lock(&LOCK_connection_count);
+  --connection_count;
+  pthread_mutex_unlock(&LOCK_connection_count);
+
   (void) pthread_mutex_lock(&LOCK_thread_count);
   thread_count--;
   delete thd;
@@ -3453,6 +3465,7 @@ static int init_thread_environment()
   (void) pthread_mutex_init(&LOCK_global_read_lock, MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_prepared_stmt_count, MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_uuid_generator, MY_MUTEX_INIT_FAST);
+  (void) pthread_mutex_init(&LOCK_connection_count, MY_MUTEX_INIT_FAST);
 #ifdef HAVE_OPENSSL
   (void) pthread_mutex_init(&LOCK_des_key_file,MY_MUTEX_INIT_FAST);
 #ifndef HAVE_YASSL
@@ -4700,6 +4713,11 @@ void create_thread_to_handle_connection(THD *thd)
       thread_count--;
       thd->killed= THD::KILL_CONNECTION;			// Safety
       (void) pthread_mutex_unlock(&LOCK_thread_count);
+
+      pthread_mutex_lock(&LOCK_connection_count);
+      --connection_count;
+      pthread_mutex_unlock(&LOCK_connection_count);
+
       statistic_increment(aborted_connects,&LOCK_status);
       /* Can't use my_error() since store_globals has not been called. */
       my_snprintf(error_message_buff, sizeof(error_message_buff),
@@ -4739,15 +4757,31 @@ static void create_new_thread(THD *thd)
   if (protocol_version > 9)
     net->return_errno=1;
 
-  /* don't allow too many connections */
-  if (thread_count - delayed_insert_threads >= max_connections+1 || abort_loop)
+  /*
+    Don't allow too many connections. We roughly check here that we allow
+    only (max_connections + 1) connections.
+  */
+
+  pthread_mutex_lock(&LOCK_connection_count);
+
+  if (connection_count >= max_connections + 1 || abort_loop)
   {
+    pthread_mutex_unlock(&LOCK_connection_count);
+
     DBUG_PRINT("error",("Too many connections"));
     close_connection(thd, ER_CON_COUNT_ERROR, 1);
     delete thd;
     DBUG_VOID_RETURN;
   }
+
+  ++connection_count;
+
+  pthread_mutex_unlock(&LOCK_connection_count);
+
+  /* Start a new thread to handle connection. */
+
   pthread_mutex_lock(&LOCK_thread_count);
+
   /*
     The initialization of thread_id is done in create_embedded_thd() for
     the embedded library.
@@ -4755,13 +4789,13 @@ static void create_new_thread(THD *thd)
   */
   thd->thread_id= thd->variables.pseudo_thread_id= thread_id++;
 
-  /* Start a new thread to handle connection */
   thread_count++;
 
   if (thread_count - delayed_insert_threads > max_used_connections)
     max_used_connections= thread_count - delayed_insert_threads;
 
   thread_scheduler.add_connection(thd);
+
   DBUG_VOID_RETURN;
 }
 #endif /* EMBEDDED_LIBRARY */
diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc
index b22a33e3e92..6f8cd6494cd 100644
--- a/sql/sql_connect.cc
+++ b/sql/sql_connect.cc
@@ -402,10 +402,11 @@ check_user(THD *thd, enum enum_server_command command,
 
       if (check_count)
       {
-        VOID(pthread_mutex_lock(&LOCK_thread_count));
-        bool count_ok= thread_count <= max_connections + delayed_insert_threads
-                       || (thd->main_security_ctx.master_access & SUPER_ACL);
-        VOID(pthread_mutex_unlock(&LOCK_thread_count));
+        pthread_mutex_lock(&LOCK_connection_count);
+        bool count_ok= connection_count <= max_connections ||
+                       (thd->main_security_ctx.master_access & SUPER_ACL);
+        VOID(pthread_mutex_unlock(&LOCK_connection_count));
+
         if (!count_ok)
         {                                         // too many connections
           my_error(ER_CON_COUNT_ERROR, MYF(0));
@@ -930,7 +931,7 @@ bool setup_connection_thread_globals(THD *thd)
 */
 
 
-bool login_connection(THD *thd)
+static bool login_connection(THD *thd)
 {
   NET *net= &thd->net;
   int error;
@@ -968,7 +969,7 @@ bool login_connection(THD *thd)
     This mainly updates status variables
 */
 
-void end_connection(THD *thd)
+static void end_connection(THD *thd)
 {
   NET *net= &thd->net;
   plugin_thdvar_cleanup(thd);