Thread_cache::unregistred_connections()bb-10.5-svoj

There is a gap between connection spawn time and connection becoming
killable time (registration in server_threads). If shutdown is initiated
when such connection is being executed through this gap, connection may
access destroyed data or cause shutdown hang.

Fixed by adding a counter of connections going through this gap and
waiting until this counter drops down to 0 on shutdown.

Part of
MDEV-18353 - Shutdown may miss to wait for connection thread

3 files changed, 19 insertions, 0 deletions

diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index e253b862700..63a24f0547d 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -5957,6 +5957,7 @@ void create_thread_to_handle_connection(CONNECT *connect)
                 ER_DEFAULT(ER_CANT_CREATE_THREAD), error);
     connect->close_with_error(ER_CANT_CREATE_THREAD, error_message_buff,
                               ER_OUT_OF_RESOURCES);
+    thread_cache.reset_unregistered();
     DBUG_VOID_RETURN;
     /* purecov: end */
   }
diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc
index 420a054a1b2..b050e6edd43 100644
--- a/sql/sql_connect.cc
+++ b/sql/sql_connect.cc
@@ -1377,6 +1377,8 @@ void do_handle_one_connection(CONNECT *connect, bool put_in_cache)
 
   /* Make THD visible in show processlist */
   server_threads.insert(thd);
+  if (put_in_cache)
+    thread_cache.reset_unregistered();
   
   thd->thr_create_utime= thr_create_utime;
   /* We need to set this because of time_out_user_resource_limits */
@@ -1448,6 +1450,7 @@ end_thread:
     thd->start_utime= thd->thr_create_utime;
 
     server_threads.insert(thd);
+    thread_cache.reset_unregistered();
   }
   delete thd;
 }
diff --git a/sql/thread_cache.h b/sql/thread_cache.h
index 5cb6c0fe85c..c9343587a9f 100644
--- a/sql/thread_cache.h
+++ b/sql/thread_cache.h
@@ -33,6 +33,8 @@ class Thread_cache
   ulong cached_thread_count;
   /** Number of active flush requests. */
   uint32_t kill_cached_threads;
+  /** Number of connections that haven't registered in server_threads. */
+  Atomic_counter<uint32_t> unregistered_connections;
   /**
     PFS stuff, only used during initialization.
     Unfortunately needs to survive till destruction.
@@ -64,6 +66,7 @@ public:
     list.empty();
     kill_cached_threads= 0;
     cached_thread_count= 0;
+    unregistered_connections= 0;
   }
 
 
@@ -71,6 +74,7 @@ public:
   {
     DBUG_ASSERT(cached_thread_count == 0);
     DBUG_ASSERT(list.is_empty());
+    DBUG_ASSERT(unregistered_connections == 0);
     mysql_cond_destroy(&COND_flush_thread_cache);
     mysql_cond_destroy(&COND_thread_cache);
     mysql_mutex_destroy(&LOCK_thread_cache);
@@ -106,6 +110,8 @@ public:
   {
     kill_cached_threads++;
     flush();
+    while (unregistered_connections)
+      my_sleep(1010);
   }
 
 
@@ -118,6 +124,7 @@ public:
   */
   bool enqueue(CONNECT *connect)
   {
+    unregistered_connections++;
     mysql_mutex_lock(&LOCK_thread_cache);
     if (cached_thread_count)
     {
@@ -205,6 +212,14 @@ public:
     mysql_mutex_unlock(&LOCK_thread_cache);
     return r;
   }
+
+
+  /**
+    Informs thread cache that connection reached killable state.
+
+    In other words THD was added to the server_threads list.
+  */
+  void reset_unregistered() { unregistered_connections--; }
 };
 
 extern Thread_cache thread_cache;