WL#6407 Code reorganization to avoid race condition between

server main thread and the kill server thread

New class Global_THD_manager is added which encapsulates
global_thread_list and synchronization primitives related to
this data structures such as LOCK_thread_count and COND_thread_count.
It provides method to add/remove/find/count/do_func/traverse
global_thread_list and guards it with LOCK_thread_count internally.

Removed ready_to_exit flag and joining signal_hand thread
with the main mysqld thread during server shutdown. Removed
kill_server_thread and calling clean_up() only from the
main mysqld thread. For Windows, shutdown handler thread
is joined with main mysqld thread during server shutdown.

Removed assert in storage/perfschema/pfs_lock.h which relaxes
double free when ready_to_exit flag is set. Also enabled
commented out code in performance schema cleanup.

Author: prabakaran thirumalai

include/my_pthread.h

@@ -111,6 +111,16 @@ struct timespec {
   ((TS1.tv.i64 - TS2.tv.i64) * 100)
 
 int win_pthread_mutex_trylock(pthread_mutex_t *mutex);
+/*
+  Existing mysql_thread_create() or pthread_create() does not work well
+  in windows platform when threads are joined because
+  A)during thread creation, thread handle is not stored.
+  B)during thread join, thread handle is retrieved using OpenThread().
+    OpenThread() does not behave properly when thread to be joined is already
+    exited.
+  Use pthread_create_get_handle() and pthread_join_with_handle() function
+  instead of mysql_thread_create() function for windows joinable threads.
+*/
 int pthread_create(pthread_t *, const pthread_attr_t *, pthread_handler, void *);
 int pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *attr);
 int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex);
@@ -127,7 +137,53 @@ int my_pthread_once(my_pthread_once_t *once_control,void (*init_routine)(void));
 struct tm *localtime_r(const time_t *timep,struct tm *tmp);
 struct tm *gmtime_r(const time_t *timep,struct tm *tmp);
 
+/**
+  Create thread.
+
+  Existing mysql_thread_create does not work well in windows platform
+  when threads are joined. Use pthread_create_get_handle() and
+  pthread_join_with_handle() function instead of mysql_thread_create()
+  function for windows.
+
+  @param thread_id    reference to pthread object
+  @param attr         reference to pthread attribute
+  @param func         pthread handler function
+  @param param        parameters to pass to newly created thread
+  @param out_handle   output parameter to get newly created thread handle
+
+  @return int
+    @retval 0 success
+    @retval 1 failure
+*/
+int pthread_create_get_handle(pthread_t *thread_id,
+                              const pthread_attr_t *attr,
+                              pthread_handler func, void *param,
+                              HANDLE *out_handle);
+
+/**
+  Wait for thread termination.
+
+  @param handle       handle of the thread to wait for
+
+  @return  int
+    @retval 0 success
+    @retval 1 failure
+*/
+int pthread_join_with_handle(HANDLE handle);
+
 void pthread_exit(void *a);
+
+/*
+  Existing pthread_join() does not work well in windows platform when
+  threads are joined because
+  A)during thread creation thread handle is not stored.
+  B)during thread join, thread handle is retrieved using OpenThread().
+    OpenThread() does not behave properly when thread to be joined is already
+    exited.
+
+  Use pthread_create_get_handle() and pthread_join_with_handle()
+  function instead for windows joinable threads.
+*/
 int pthread_join(pthread_t thread, void **value_ptr);
 int pthread_cancel(pthread_t thread);
 extern int pthread_dummy(int);

include/my_sys.h

@@ -253,12 +253,6 @@ extern ulong	my_file_opened,my_stream_opened, my_tmp_file_created;
 extern ulong    my_file_total_opened;
 extern my_bool	my_init_done;
 
-					/* Point to current my_message() */
-extern void (*my_sigtstp_cleanup)(void),
-					/* Executed before jump to shell */
-	    (*my_sigtstp_restart)(void),
-	    (*my_abort_hook)(int);
-					/* Executed when comming from shell */
 extern MYSQL_PLUGIN_IMPORT int my_umask;		/* Default creation mask  */
 extern int my_umask_dir,
 	   my_recived_signals,	/* Signals we have got */

include/mysql/thread_pool_priv.h

@@ -38,8 +38,6 @@
 #include <table.h>
 #include <set>
 
-typedef std::set<THD*>::iterator Thread_iterator;
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -92,10 +90,22 @@ void dec_connection_count();
 */
 void inc_thread_created();
 
+void thd_lock_thread_count(THD *thd);
+void thd_unlock_thread_count(THD *thd);
+/* Remove the THD from the set of global threads. */
+void remove_global_thread(THD *thd);
+
 #ifdef __cplusplus
 }
 #endif
 
+/*
+  Interface to global thread list iterator functions.
+  Executes a function with signature 'void f(THD*, uint64)' for all THDs.
+*/
+typedef void (do_thd_impl_uint64)(THD*, uint64);
+void do_for_all_thd(do_thd_impl_uint64, uint64);
+
 /* Needed to get access to scheduler variables */
 void* thd_get_scheduler_data(THD *thd);
 void thd_set_scheduler_data(THD *thd, void *data);
@@ -106,8 +116,6 @@ void thd_set_psi(THD *thd, PSI_thread *psi);
 void thd_set_killed(THD *thd);
 void thd_clear_errors(THD *thd);
 void thd_set_thread_stack(THD *thd, char *stack_start);
-void thd_lock_thread_count(THD *thd);
-void thd_unlock_thread_count(THD *thd);
 void thd_close_connection(THD *thd);
 THD *thd_get_current_thd();
 void thd_new_connection_setup(THD *thd, char *stack_start);
@@ -122,10 +130,6 @@ ulong  thd_get_net_wait_timeout(THD *thd);
 my_socket thd_get_fd(THD *thd);
 int thd_store_globals(THD* thd);
 
-/* Interface to global thread list iterator functions */
-Thread_iterator thd_get_global_thread_list_begin();
-Thread_iterator thd_get_global_thread_list_end();
-
 /* Print to the MySQL error log */
 void sql_print_error(const char *format, ...);
 void sql_print_warning(const char *format, ...);
@@ -167,8 +171,6 @@ void thd_release_resources(THD *thd);
 void restore_globals(THD *thd);
 /* Destroy THD object */
 void destroy_thd(THD *thd);
-/* Remove the THD from the set of global threads. */
-void remove_global_thread(THD *thd);
 
 /*
   max_connections is needed to calculate the maximum number of threads

libmysqld/CMakeLists.txt

@@ -62,6 +62,7 @@ SET(SQL_EMBEDDED_SOURCES
   ../sql/event_parse_data.cc
   ../sql/hash_filo.cc
   ../sql/log_event.cc
+  ../sql/mysqld_thd_manager.cc
   ../sql/rpl_filter.cc
   ../sql/rpl_injector.cc
   ../sql/rpl_record.cc

libmysqld/lib_sql.cc

@@ -1,5 +1,5 @@
 /*
- * Copyright (c)  2000
+ * Copyright (c)  2000, 2013
  * SWsoft  company
  *
  * This material is provided "as is", with absolutely no warranty expressed
@@ -424,9 +424,7 @@ static void emb_free_embedded_thd(MYSQL *mysql)
   thd->clear_data_list();
   thd->store_globals();
   thd->release_resources();
-
-  remove_global_thread(thd);
-
+  Global_THD_manager::get_instance()->remove_thd(thd);
   delete thd;
   my_pthread_setspecific_ptr(THR_THD,  0);
   mysql->thd=0;
@@ -704,7 +702,9 @@ void init_embedded_mysql(MYSQL *mysql, int client_flag)
 void *create_embedded_thd(int client_flag)
 {
   THD * thd= new THD;
-  thd->thread_id= thd->variables.pseudo_thread_id= thread_id++;
+  Global_THD_manager *thd_manager= Global_THD_manager::get_instance();
+  thd->variables.pseudo_thread_id= thd_manager->get_inc_thread_id();
+  thd->thread_id= thd->variables.pseudo_thread_id;
 
   thd->thread_stack= (char*) &thd;
   if (thd->store_globals())
@@ -735,8 +735,7 @@ void *create_embedded_thd(int client_flag)
   thd->first_data= 0;
   thd->data_tail= &thd->first_data;
   memset(&thd->net, 0, sizeof(thd->net));
-
-  add_global_thread(thd);
+  thd_manager->add_thd(thd);
   thd->mysys_var= 0;
   return thd;
 err:

mysql-test/suite/binlog/t/binlog_reset_master.test

@@ -12,7 +12,7 @@
 
 #######################################################################
 # BUG#12574820: binlog.binlog_tmp_table timing out in daily and weekly trunk run
-# Problem: MYSQL_BIN_LOG::reset_logs acquired LOCK_thread_count and
+# Problem: MYSQL_BIN_LOG::reset_logs acquired LOCK_thd_count and
 # LOCK_log in the wrong order. This could cause a deadlock when
 # RESET MASTER was run concurrently with a disconnecting thread.
 #######################################################################

mysql-test/suite/perfschema/r/server_init.result

@@ -36,7 +36,7 @@ where name like "wait/synch/mutex/sql/LOCK_open";
 count(name)
 1
 select count(name) from mutex_instances
-where name like "wait/synch/mutex/sql/LOCK_thread_count";
+where name like "wait/synch/mutex/sql/LOCK_thd_count";
 count(name)
 1
 select count(name) from mutex_instances

mysql-test/suite/perfschema/r/setup_instruments_defaults.result

@@ -15,10 +15,10 @@ wait/synch/mutex/sql/LOCK_user_conn	YES	NO
 wait/synch/mutex/sql/LOCK_uuid_generator	YES	NO
 wait/synch/mutex/sql/LOCK_xid_cache	YES	NO
 SELECT * FROM performance_schema.setup_instruments
-WHERE name = 'wait/synch/mutex/sql/LOCK_thread_count'
+WHERE name = 'wait/synch/mutex/sql/LOCK_thd_count'
 AND enabled = 'no' AND timed = 'no';
 NAME	ENABLED	TIMED
-wait/synch/mutex/sql/LOCK_thread_count	NO	NO
+wait/synch/mutex/sql/LOCK_thd_count	NO	NO
 SELECT * FROM performance_schema.setup_instruments
 WHERE name IN (
 'wait/synch/mutex/sql/LOG_INFO::lock',

mysql-test/suite/perfschema/t/server_init.test

@@ -47,7 +47,7 @@ select count(name) from mutex_instances
  where name like "wait/synch/mutex/sql/LOCK_open";
 
 select count(name) from mutex_instances
- where name like "wait/synch/mutex/sql/LOCK_thread_count";
+ where name like "wait/synch/mutex/sql/LOCK_thd_count";
 
 select count(name) from mutex_instances
  where name like "wait/synch/mutex/sql/LOCK_log_throttle_qni";

mysql-test/suite/perfschema/t/setup_instruments_defaults-master.opt

@@ -10,7 +10,7 @@
 --loose-performance-schema-instrument='  wait/synch/mutex/sql/%   =  OFF  '
 --loose-performance-schema-instrument='wait/synch/mutex/sql/% = OFF '
 --loose-performance-schema-instrument='wait/synch/mutex/sql/LOCK% = ON'
---loose-performance-schema-instrument='wait/synch/mutex/sql/LOCK_thread_count=OFF'
+--loose-performance-schema-instrument='wait/synch/mutex/sql/LOCK_thd_count=OFF'
 --loose-performance-schema-instrument=' wait/synch/mutex/sql/LOCK_user_conn    = COUNTED'
 --loose-performance-schema-instrument='wait%/synch/mutex/sql/LOCK_uu%_genera%/= COUNTED'
 --loose-performance-schema-instrument='%%wait/synch/mutex/sql/LOCK_xid_cache=COUNTED'

mysql-test/suite/perfschema/t/setup_instruments_defaults.test

@@ -21,7 +21,7 @@ AND enabled = 'yes' AND timed = 'no'
 ORDER BY name;
 
 SELECT * FROM performance_schema.setup_instruments
-WHERE name = 'wait/synch/mutex/sql/LOCK_thread_count'
+WHERE name = 'wait/synch/mutex/sql/LOCK_thd_count'
 AND enabled = 'no' AND timed = 'no';
 
 SELECT * FROM performance_schema.setup_instruments

mysql-test/valgrind.supp

@@ -710,20 +710,11 @@
 }
 
 #
-# See related Bug#56666
-# Race condition between the server main thread and the kill server thread.
-#
-# Because of this race condition, the call to shutdown_performance_schema()
-# was commented in sql/mysqld.cc, causing the reported leaks.
-#
-
-{
-   missing shutdown_performance_schema 1a
-   Memcheck:Leak
-   fun:malloc
-   fun:_Z10pfs_mallocmi
-}
-
+# Bug#56666 is fixed by WL#6407
+# Also, as part of WL#6407, call to shutdown_performance_schema()
+# is uncommented in sql/mysqld.cc. As this call was commented out
+# for long time, there are still some leaks which needs to be
+# addressed by pfs.
 {
    missing shutdown_performance_schema 1b
    Memcheck:Leak
@@ -943,57 +934,6 @@
    fun:_Z11mysqld_mainiPPc
 }
 
-{
-   missing shutdown_performance_schema 9a-mem
-   Memcheck:Leak
-   fun:my_malloc
-   fun:_Z22add_pfs_instr_to_arrayPKcS0_
-   fun:mysqld_get_one_option
-   fun:my_handle_options
-   fun:_Z20handle_early_optionsv
-   fun:_Z11mysqld_mainiPPc
-}
-
-# Same as shutdown_performance_schema 9,
-# but the compiler/linker can sometime change the
-# calls from:
-# main()
-#   --> init_pfs_instrument_array()
-#     --> init_dynamic_array2()
-# to:
-# main()
-#   --> init_dynamic_array2()
-# when building with optimizations.
-
-{
-   missing shutdown_performance_schema 10
-   Memcheck:Leak
-   fun:malloc
-   fun:my_malloc
-   fun:init_dynamic_array2
-   fun:_Z11mysqld_mainiPPc
-   fun:main
-}
-
-{
-   missing shutdown_performance_schema 10-mem
-   Memcheck:Leak
-   fun:my_malloc
-   fun:init_dynamic_array2
-   fun:_Z11mysqld_mainiPPc
-   fun:main
-}
-
-{
-   missing shutdown_performance_schema 11
-   Memcheck:Leak
-   fun:malloc
-   fun:my_malloc
-   fun:init_dynamic_array2
-   fun:_Z11mysqld_mainiPPc
-   fun:(below main)
-}
-
 {
    missing shutdown_performance_schema 11-mem
    Memcheck:Leak
@@ -1265,9 +1205,6 @@
    fun:_ZL12reap_pluginsv
    fun:_Z15plugin_shutdownv
    fun:_ZL8clean_upb
-   fun:_Z10unireg_endv
-   fun:_ZL11kill_serverPv
-   fun:kill_server_thread
 }
 
 #Suppress warnings from glibc implementation of 'malloc_info' 

mysys/my_static.c

@@ -96,7 +96,6 @@ int volatile my_have_got_alarm=0;	/* declare variable to reset */
 ulong my_time_to_wait_for_lock=2;	/* In seconds */
 
 	/* from errors.c */
-void (*my_abort_hook)(int) = (void(*)(int)) exit;
 void (*error_handler_hook)(uint error, const char *str, myf MyFlags)=
   my_message_stderr;
 void (*fatal_error_handler_hook)(uint error, const char *str, myf MyFlags)=