Fixed errors and bugs found by valgrind:

- If run with valgrind, mysqltest will now wait longer when syncronizing slave with master
- Ensure that we wait with cleanup() until slave thread has stopped.
- Added signal_thd_deleted() to signal close_connections() that all THD's has been freed.
- Check in handle_fatal_signal() that we don't use variables that has been freed.
- Increased some timeouts when run with --valgrind

Other things:
- Fixed wrong test in one_thread_per_connection_end() if galera is used.
- Removed not needed calls to THD_CHECK_SENTRY() when we are calling 'delete thd'.

10 files changed, 97 insertions, 45 deletions

diff --git a/client/mysqltest.cc b/client/mysqltest.cc
index 6d9261fe906..c6144d9c5e4 100644
--- a/client/mysqltest.cc
+++ b/client/mysqltest.cc
@@ -33,7 +33,7 @@
   And many others
 */
 
-#define MTEST_VERSION "3.4"
+#define MTEST_VERSION "3.5"
 
 #include "client_priv.h"
 #include <mysql_version.h>
@@ -121,7 +121,7 @@ static my_bool tty_password= 0;
 static my_bool opt_mark_progress= 0;
 static my_bool ps_protocol= 0, ps_protocol_enabled= 0;
 static my_bool sp_protocol= 0, sp_protocol_enabled= 0;
-static my_bool view_protocol= 0, view_protocol_enabled= 0;
+static my_bool view_protocol= 0, view_protocol_enabled= 0, wait_longer= 0;
 static my_bool cursor_protocol= 0, cursor_protocol_enabled= 0;
 static my_bool parsing_disabled= 0;
 static my_bool display_result_vertically= FALSE, display_result_lower= FALSE,
@@ -4657,7 +4657,7 @@ void do_sync_with_master2(struct st_command *command, long offset,
   MYSQL_ROW row;
   MYSQL *mysql= cur_con->mysql;
   char query_buf[FN_REFLEN+128];
-  int timeout= 300; /* seconds */
+  int timeout= wait_longer ? 1500 : 300; /* seconds */
 
   if (!master_pos.file[0])
     die("Calling 'sync_with_master' without calling 'save_master_pos'");
@@ -5011,7 +5011,7 @@ static int my_kill(int pid, int sig)
 
 void do_shutdown_server(struct st_command *command)
 {
-  long timeout=60;
+  long timeout= wait_longer ? 60*5 : 60;
   int pid;
   DYNAMIC_STRING ds_pidfile_name;
   MYSQL* mysql = cur_con->mysql;
@@ -5080,7 +5080,6 @@ void do_shutdown_server(struct st_command *command)
   (void)my_kill(pid, 9);
 
   DBUG_VOID_RETURN;
-
 }
 
 
@@ -6953,6 +6952,9 @@ static struct my_option my_long_options[] =
    "Number of seconds before connection timeout.",
    &opt_connect_timeout, &opt_connect_timeout, 0, GET_UINT, REQUIRED_ARG,
    120, 0, 3600 * 12, 0, 0, 0},
+  {"wait-longer-for-timeouts", 0,
+   "Wait longer for timeouts. Useful when running under valgrind",
+   &wait_longer, &wait_longer, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
   {"plugin_dir", 0, "Directory for client-side plugins.",
     &opt_plugin_dir, &opt_plugin_dir, 0,
    GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
diff --git a/mysql-test/include/sync_with_master_gtid.inc b/mysql-test/include/sync_with_master_gtid.inc
index 97ada8eea29..777711b979c 100644
--- a/mysql-test/include/sync_with_master_gtid.inc
+++ b/mysql-test/include/sync_with_master_gtid.inc
@@ -34,6 +34,10 @@ let $_slave_timeout= $slave_timeout;
 if (!$_slave_timeout)
 {
   let $_slave_timeout= 120;
+  if ($VALGRIND_TEST)
+  {
+    let $_slave_timeout= 1200;
+  }
 }
 
 --let $_result= `SELECT master_gtid_wait('$master_pos', $_slave_timeout)`
diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
index 588b9122e7b..841e1d98b70 100755
--- a/mysql-test/mysql-test-run.pl
+++ b/mysql-test/mysql-test-run.pl
@@ -5472,6 +5472,12 @@ sub start_mysqltest ($) {
     mtr_add_arg($args, "--max-connections=%d", $opt_max_connections);
   }
 
+  if ( $opt_valgrind )
+  {
+    # Longer timeouts when running with valgrind
+    mtr_add_arg($args, "--wait-longer-for-timeouts");
+  }
+
   if ( $opt_embedded_server )
   {
 
diff --git a/mysql-test/r/max_statement_time.result b/mysql-test/r/max_statement_time.result
index e445907255b..30060e78aff 100644
--- a/mysql-test/r/max_statement_time.result
+++ b/mysql-test/r/max_statement_time.result
@@ -166,6 +166,7 @@ call pr();
 1
 1
 ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
+set max_statement_time = 0;
 drop procedure pr;
 create procedure pr()
 begin
diff --git a/mysql-test/t/max_statement_time.test b/mysql-test/t/max_statement_time.test
index cc8c0ae87a2..a25a97e1db3 100644
--- a/mysql-test/t/max_statement_time.test
+++ b/mysql-test/t/max_statement_time.test
@@ -1,9 +1,11 @@
 #
 # Test behavior of MAX_STATEMENT_TIME.
+# We can't do this under valgrind as valgrind interferes with thread scheduling
 #
 
 --source include/not_embedded.inc
 --source include/have_innodb.inc
+--source include/not_valgrind.inc
 
 --echo
 --echo # Test the MAX_STATEMENT_TIME option.
@@ -207,6 +209,7 @@ delimiter ;|
 set max_statement_time = 0.001;
 --error ER_STATEMENT_TIMEOUT
 call pr();
+set max_statement_time = 0;
 drop procedure pr;
 delimiter |;
 create procedure pr()
diff --git a/mysql-test/valgrind.supp b/mysql-test/valgrind.supp
index 2fca6ef90e8..53ddda21258 100644
--- a/mysql-test/valgrind.supp
+++ b/mysql-test/valgrind.supp
@@ -127,6 +127,15 @@
 }
 
 {
+   pthread memalign memory loss3
+   Memcheck:Leak
+   fun:memalign
+   fun:tls_get_addr_tail
+   ...
+   fun:*ha_finalize_handlerton*
+}
+
+{
    pthread pthread_key_create
    Memcheck:Leak
    fun:malloc
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 0586f1b9bf5..f36d1cee9bc 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -487,7 +487,7 @@ ulong delay_key_write_options;
 uint protocol_version;
 uint lower_case_table_names;
 ulong tc_heuristic_recover= 0;
-int32 thread_count;
+int32 thread_count, service_thread_count;
 int32 thread_running;
 int32 slave_open_temp_tables;
 ulong thread_created;
@@ -1753,7 +1753,7 @@ static void close_connections(void)
   /* All threads has now been aborted */
   DBUG_PRINT("quit",("Waiting for threads to die (count=%u)",thread_count));
   mysql_mutex_lock(&LOCK_thread_count);
-  while (thread_count)
+  while (thread_count || service_thread_count)
   {
     mysql_cond_wait(&COND_thread_count, &LOCK_thread_count);
     DBUG_PRINT("quit",("One thread died (count=%u)",thread_count));
@@ -2108,6 +2108,7 @@ void clean_up(bool print_message)
   xid_cache_free();
   tdc_deinit();
   mdl_destroy();
+  dflt_key_cache= 0;
   key_caches.delete_elements((void (*)(const char*, uchar*)) free_key_cache);
   wt_end();
   multi_keycache_free();
@@ -2150,6 +2151,7 @@ void clean_up(bool print_message)
     sql_print_information(ER_DEFAULT(ER_SHUTDOWN_COMPLETE),my_progname);
   cleanup_errmsgs();
   MYSQL_CALLBACK(thread_scheduler, end, ());
+  thread_scheduler= 0;
   mysql_library_end();
   finish_client_errs();
   (void) my_error_unregister(ER_ERROR_FIRST, ER_ERROR_LAST); // finish server errs
@@ -2838,8 +2840,26 @@ void delete_running_thd(THD *thd)
   delete thd;
   dec_thread_running();
   thread_safe_decrement32(&thread_count);
-  if (!thread_count)
+  signal_thd_deleted();
+}
+
+/*
+  Send a signal to unblock close_conneciton() if there is no more
+  threads running with a THD attached
+
+  It's safe to check for thread_count and service_thread_count outside
+  of a mutex as we are only interested to see if they where decremented
+  to 0 by a previous unlink_thd() call.
+
+  We should only signal COND_thread_count if both variables are 0,
+  false positives are ok.
+*/
+
+void signal_thd_deleted()
+{
+  if (!thread_count && ! service_thread_count)
   {
+    /* Signal close_connections() that all THD's are freed */
     mysql_mutex_lock(&LOCK_thread_count);
     mysql_cond_broadcast(&COND_thread_count);
     mysql_mutex_unlock(&LOCK_thread_count);
@@ -2993,22 +3013,10 @@ bool one_thread_per_connection_end(THD *thd, bool put_in_cache)
 
   unlink_thd(thd);
 
-  if (put_in_cache && cache_thread() && !wsrep_applier)
+  if (!wsrep_applier && put_in_cache && cache_thread())
     DBUG_RETURN(0);                             // Thread is reused
 
-  /*
-    It's safe to check for thread_count outside of the mutex
-    as we are only interested to see if it was counted to 0 by the
-    above unlink_thd() call. We should only signal COND_thread_count if
-    thread_count is likely to be 0. (false positives are ok)
-  */
-  if (!thread_count)
-  {
-    mysql_mutex_lock(&LOCK_thread_count);
-    DBUG_PRINT("signal", ("Broadcasting COND_thread_count"));
-    mysql_cond_broadcast(&COND_thread_count);
-    mysql_mutex_unlock(&LOCK_thread_count);
-  }
+  signal_thd_deleted();
   DBUG_LEAVE;                                   // Must match DBUG_ENTER()
 #if defined(HAVE_OPENSSL) && !defined(EMBEDDED_LIBRARY)
   ERR_remove_state(0);
@@ -8588,7 +8596,8 @@ static int mysql_init_variables(void)
   cleanup_done= 0;
   server_id_supplied= 0;
   test_flags= select_errors= dropping_tables= ha_open_options=0;
-  thread_count= thread_running= kill_cached_threads= wake_thread=0;
+  thread_count= thread_running= kill_cached_threads= wake_thread= 0;
+  service_thread_count= 0;
   slave_open_temp_tables= 0;
   cached_thread_count= 0;
   opt_endinfo= using_udf_functions= 0;
diff --git a/sql/mysqld.h b/sql/mysqld.h
index ebf55d8fd8a..e494b1abbe1 100644
--- a/sql/mysqld.h
+++ b/sql/mysqld.h
@@ -81,6 +81,7 @@ void close_connection(THD *thd, uint sql_errno= 0);
 void handle_connection_in_main_thread(THD *thd);
 void create_thread_to_handle_connection(THD *thd);
 void delete_running_thd(THD *thd);
+void signal_thd_deleted();
 void unlink_thd(THD *thd);
 bool one_thread_per_connection_end(THD *thd, bool put_in_cache);
 void flush_thread_cache();
@@ -566,7 +567,7 @@ extern mysql_cond_t COND_thread_count;
 extern mysql_cond_t COND_manager;
 extern mysql_cond_t COND_slave_init;
 extern int32 thread_running;
-extern int32 thread_count;
+extern int32 thread_count, service_thread_count;
 
 extern char *opt_ssl_ca, *opt_ssl_capath, *opt_ssl_cert, *opt_ssl_cipher,
   *opt_ssl_key, *opt_ssl_crl, *opt_ssl_crlpath;
diff --git a/sql/signal_handler.cc b/sql/signal_handler.cc
index 61e2830e82e..4490aae6b62 100644
--- a/sql/signal_handler.cc
+++ b/sql/signal_handler.cc
@@ -110,8 +110,9 @@ extern "C" sig_handler handle_fatal_signal(int sig)
   set_server_version();
   my_safe_printf_stderr("Server version: %s\n", server_version);
 
-  my_safe_printf_stderr("key_buffer_size=%lu\n",
-                        (ulong) dflt_key_cache->key_cache_mem_size);
+  if (dflt_key_cache)
+    my_safe_printf_stderr("key_buffer_size=%lu\n",
+                          (ulong) dflt_key_cache->key_cache_mem_size);
 
   my_safe_printf_stderr("read_buffer_size=%ld\n",
                         (long) global_system_variables.read_buff_size);
@@ -119,24 +120,30 @@ extern "C" sig_handler handle_fatal_signal(int sig)
   my_safe_printf_stderr("max_used_connections=%lu\n",
                         (ulong) max_used_connections);
 
-  my_safe_printf_stderr("max_threads=%u\n",
-                        (uint) thread_scheduler->max_threads +
-                        (uint) extra_max_connections);
+  if (thread_scheduler)
+    my_safe_printf_stderr("max_threads=%u\n",
+                          (uint) thread_scheduler->max_threads +
+                          (uint) extra_max_connections);
 
   my_safe_printf_stderr("thread_count=%u\n", (uint) thread_count);
 
-  my_safe_printf_stderr("It is possible that mysqld could use up to \n"
-                        "key_buffer_size + "
-                        "(read_buffer_size + sort_buffer_size)*max_threads = "
-                        "%lu K  bytes of memory\n",
-                        (ulong)(dflt_key_cache->key_cache_mem_size +
-                         (global_system_variables.read_buff_size +
-                          global_system_variables.sortbuff_size) *
-                         (thread_scheduler->max_threads + extra_max_connections) +
-                         (max_connections + extra_max_connections)* sizeof(THD)) / 1024);
-
-  my_safe_printf_stderr("%s",
-    "Hope that's ok; if not, decrease some variables in the equation.\n\n");
+  if (dflt_key_cache && thread_scheduler)
+  {
+    my_safe_printf_stderr("It is possible that mysqld could use up to \n"
+                          "key_buffer_size + "
+                          "(read_buffer_size + sort_buffer_size)*max_threads = "
+                          "%lu K  bytes of memory\n",
+                          (ulong)
+                          (dflt_key_cache->key_cache_mem_size +
+                           (global_system_variables.read_buff_size +
+                            global_system_variables.sortbuff_size) *
+                           (thread_scheduler->max_threads + extra_max_connections) +
+                           (max_connections + extra_max_connections) *
+                           sizeof(THD)) / 1024);
+    my_safe_printf_stderr("%s",
+                          "Hope that's ok; if not, decrease some variables in "
+                          "the equation.\n\n");
+  }
 
 #ifdef HAVE_STACKTRACE
   thd= current_thd;
diff --git a/sql/slave.cc b/sql/slave.cc
index 07b0dbacc82..3bbc7610649 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -299,6 +299,7 @@ handle_slave_init(void *arg __attribute__((unused)))
   thd->thread_id= thread_id++;
   mysql_mutex_unlock(&LOCK_thread_count);
   thd->system_thread = SYSTEM_THREAD_SLAVE_INIT;
+  thread_safe_increment32(&service_thread_count);
   thd->store_globals();
   thd->security_ctx->skip_grants();
   thd->set_command(COM_DAEMON);
@@ -314,6 +315,8 @@ handle_slave_init(void *arg __attribute__((unused)))
   mysql_mutex_lock(&LOCK_thread_count);
   delete thd;
   mysql_mutex_unlock(&LOCK_thread_count);
+  thread_safe_decrement32(&service_thread_count);
+  signal_thd_deleted();
   my_thread_end();
 
   mysql_mutex_lock(&LOCK_slave_init);
@@ -3059,6 +3062,11 @@ static int init_slave_thread(THD* thd, Master_info *mi,
                   simulate_error|= (1 << SLAVE_THD_IO););
   DBUG_EXECUTE_IF("simulate_sql_slave_error_on_init",
                   simulate_error|= (1 << SLAVE_THD_SQL););
+
+  thd->system_thread = (thd_type == SLAVE_THD_SQL) ?
+    SYSTEM_THREAD_SLAVE_SQL : SYSTEM_THREAD_SLAVE_IO;
+  thread_safe_increment32(&service_thread_count);
+
   /* We must call store_globals() before doing my_net_init() */
   if (init_thr_lock() || thd->store_globals() ||
       my_net_init(&thd->net, 0, thd, MYF(MY_THREAD_SPECIFIC)) ||
@@ -3068,8 +3076,6 @@ static int init_slave_thread(THD* thd, Master_info *mi,
     DBUG_RETURN(-1);
   }
 
-  thd->system_thread = (thd_type == SLAVE_THD_SQL) ?
-    SYSTEM_THREAD_SLAVE_SQL : SYSTEM_THREAD_SLAVE_IO;
   thd->security_ctx->skip_grants();
   thd->slave_thread= 1;
   thd->connection_name= mi->connection_name;
@@ -4313,11 +4319,14 @@ err_during_init:
   mi->rli.relay_log.description_event_for_queue= 0;
   // TODO: make rpl_status part of Master_info
   change_rpl_status(RPL_ACTIVE_SLAVE,RPL_IDLE_SLAVE);
+
   mysql_mutex_lock(&LOCK_thread_count);
   thd->unlink();
   mysql_mutex_unlock(&LOCK_thread_count);
-  THD_CHECK_SENTRY(thd);
   delete thd;
+  thread_safe_decrement32(&service_thread_count);
+  signal_thd_deleted();
+
   mi->abort_slave= 0;
   mi->slave_running= MYSQL_SLAVE_NOT_RUN;
   mi->io_thd= 0;
@@ -4970,9 +4979,10 @@ err_during_init:
   mysql_mutex_unlock(&LOCK_active_mi);
 
   mysql_mutex_lock(&LOCK_thread_count);
-  THD_CHECK_SENTRY(thd);
   delete thd;
   mysql_mutex_unlock(&LOCK_thread_count);
+  thread_safe_decrement32(&service_thread_count);
+  signal_thd_deleted();
 
   DBUG_LEAVE;                                   // Must match DBUG_ENTER()
   my_thread_end();