List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:June 20 2012 8:07am
Subject:bzr push into mysql-trunk branch (tor.didriksen:4028 to 4029) Bug#14100495
View as plain text  
 4029 Tor Didriksen	2012-06-20
      Bug#14100495 VALGRIND: INVALID READ OF SIZE 4 IN CLOSECON_HANDLERTON AND THD_HA_DATA
      
      Disconnect THD from global resources (handlers etc.) before
      signalling that the thread is done.
      This is done to avoid possible race conditions during shutdown of the server.

    modified:
      include/mysql/thread_pool_priv.h
      libmysqld/lib_sql.cc
      sql/event_scheduler.cc
      sql/mysqld.cc
      sql/mysqld.h
      sql/rpl_slave.cc
      sql/scheduler.cc
      sql/sql_class.cc
      sql/sql_class.h
      sql/sql_insert.cc
      sql/sql_parse.cc
 4028 Sunny Bains	2012-06-20
      Bug#14146981 - AT STARTUP: INNODB: ERROR: A RECORD LOCK WAIT HAPPENS IN A DICTIONARY OPERATION!
      
      The transaction that updates the system tables needs to be tagged as
      TRX_DICT_OP_TABLE.
      
      Approved by Inaam Rana rb://1104

    modified:
      mysql-test/suite/innodb/r/innodb-wl5522-debug.result
      mysql-test/suite/innodb/t/innodb-wl5522-debug.test
      storage/innobase/row/row0mysql.cc
=== modified file 'include/mysql/thread_pool_priv.h'
--- a/include/mysql/thread_pool_priv.h	2012-04-20 08:55:22 +0000
+++ b/include/mysql/thread_pool_priv.h	2012-06-20 07:39:11 +0000
@@ -94,8 +94,8 @@ bool thd_is_connection_alive(THD *thd);
 void close_connection(THD *thd, uint errcode);
 /* End the connection before closing it */
 void end_connection(THD *thd);
-/* Cleanup the THD object */
-void thd_cleanup(THD *thd);
+/* Release resources of the THD object */
+void thd_release_resources(THD *thd);
 /* Decrement connection counter */
 void dec_connection_count();
 /* Destroy THD object */

=== modified file 'libmysqld/lib_sql.cc'
--- a/libmysqld/lib_sql.cc	2012-05-29 23:35:24 +0000
+++ b/libmysqld/lib_sql.cc	2012-06-20 07:39:11 +0000
@@ -423,6 +423,7 @@ static void emb_free_embedded_thd(MYSQL 
   THD *thd= (THD*)mysql->thd;
   thd->clear_data_list();
   thd->store_globals();
+  thd->release_resources();
 
   mysql_mutex_lock(&LOCK_thread_count);
   remove_global_thread(thd);

=== modified file 'sql/event_scheduler.cc'
--- a/sql/event_scheduler.cc	2012-05-17 12:55:26 +0000
+++ b/sql/event_scheduler.cc	2012-06-20 07:39:11 +0000
@@ -130,13 +130,12 @@ post_init_event_thread(THD *thd)
   (void) init_new_connection_handler_thread();
   if (init_thr_lock() || thd->store_globals())
   {
-    thd->cleanup();
     return TRUE;
   }
 
+  inc_thread_running();
   mysql_mutex_lock(&LOCK_thread_count);
   add_global_thread(thd);
-  inc_thread_running();
   mysql_mutex_unlock(&LOCK_thread_count);
   return FALSE;
 }
@@ -157,9 +156,11 @@ deinit_event_thread(THD *thd)
   DBUG_ASSERT(thd->net.buff != 0);
   net_end(&thd->net);
   DBUG_PRINT("exit", ("Event thread finishing"));
+
+  dec_thread_running();
+  thd->release_resources();
   mysql_mutex_lock(&LOCK_thread_count);
   remove_global_thread(thd);
-  dec_thread_running();
   mysql_mutex_unlock(&LOCK_thread_count);
   delete thd;
 }
@@ -431,9 +432,11 @@ Event_scheduler::start()
     new_thd->proc_info= "Clearing";
     DBUG_ASSERT(new_thd->net.buff != 0);
     net_end(&new_thd->net);
+
+    dec_thread_running();
+    new_thd->release_resources();
     mysql_mutex_lock(&LOCK_thread_count);
     remove_global_thread(new_thd);
-    dec_thread_running();
     mysql_mutex_unlock(&LOCK_thread_count);
     delete new_thd;
   }
@@ -564,9 +567,11 @@ error:
     new_thd->proc_info= "Clearing";
     DBUG_ASSERT(new_thd->net.buff != 0);
     net_end(&new_thd->net);
+
+    dec_thread_running();
+    new_thd->release_resources();
     mysql_mutex_lock(&LOCK_thread_count);
     remove_global_thread(new_thd);
-    dec_thread_running();
     mysql_mutex_unlock(&LOCK_thread_count);
     delete new_thd;
   }

=== modified file 'sql/mysqld.cc'
--- a/sql/mysqld.cc	2012-06-12 13:31:36 +0000
+++ b/sql/mysqld.cc	2012-06-20 07:39:11 +0000
@@ -491,7 +491,7 @@ ulong delay_key_write_options;
 uint protocol_version;
 uint lower_case_table_names;
 ulong tc_heuristic_recover= 0;
-int32 thread_running;
+int32 num_thread_running;
 ulong thread_created;
 ulong back_log, connect_timeout, concurrency, server_id;
 ulong table_cache_size, table_def_size;
@@ -814,6 +814,7 @@ void remove_global_thread(THD *thd)
   DBUG_PRINT("info", ("remove_global_thread %p current_linfo %p",
                       thd, thd->current_linfo));
   mysql_mutex_assert_owner(&LOCK_thread_count);
+  DBUG_ASSERT(thd->release_resources_done());
 
   const size_t num_erased= global_thread_list->erase(thd);
   if (num_erased == 1)
@@ -2364,16 +2365,16 @@ extern "C" sig_handler end_thread_signal
 
 
 /*
-  Cleanup THD object
+  Rlease resources of the THD, prior to destruction.
 
   SYNOPSIS
-    thd_cleanup()
+    thd_release_resources()
     thd    Thread handler
 */
 
-void thd_cleanup(THD *thd)
+void thd_release_resources(THD *thd)
 {
-  thd->cleanup();
+  thd->release_resources();
 }
 
 /*
@@ -2493,7 +2494,7 @@ bool one_thread_per_connection_end(THD *
   DBUG_ENTER("one_thread_per_connection_end");
   DBUG_PRINT("info", ("thd %p block_pthread %d", thd, (int) block_pthread));
 
-  thd->cleanup();
+  thd->release_resources();
   dec_connection_count();
 
   mysql_mutex_lock(&LOCK_thread_count);
@@ -5702,7 +5703,6 @@ void create_thread_to_handle_connection(
     /* Create new thread to handle connection */
     int error;
     thread_created++;
-    add_global_thread(thd);
     DBUG_PRINT("info",(("creating thread %lu"), thd->thread_id));
     thd->prior_thr_create_utime= thd->start_utime= my_micro_time();
     if ((error= mysql_thread_create(key_thread_one_connection,
@@ -5714,7 +5714,6 @@ void create_thread_to_handle_connection(
       DBUG_PRINT("error",
                  ("Can't create thread to handle request (error %d)",
                   error));
-      remove_global_thread(thd);
       thd->killed= THD::KILL_CONNECTION;      // Safety
       mysql_mutex_unlock(&LOCK_thread_count);
 
@@ -5733,6 +5732,7 @@ void create_thread_to_handle_connection(
       return;
       /* purecov: end */
     }
+    add_global_thread(thd);
   }
   mysql_mutex_unlock(&LOCK_thread_count);
   DBUG_PRINT("info",("Thread created"));
@@ -7572,7 +7572,7 @@ SHOW_VAR status_vars[]= {
   {"Threads_cached",           (char*) &blocked_pthread_count,    SHOW_LONG_NOFLUSH},
   {"Threads_connected",        (char*) &connection_count,       SHOW_INT},
   {"Threads_created",        (char*) &thread_created,   SHOW_LONG_NOFLUSH},
-  {"Threads_running",          (char*) &thread_running,         SHOW_INT},
+  {"Threads_running",          (char*) &num_thread_running,     SHOW_INT},
   {"Uptime",                   (char*) &show_starttime,         SHOW_FUNC},
 #ifdef ENABLED_PROFILING
   {"Uptime_since_flush_status",(char*) &show_flushstatustime,   SHOW_FUNC},
@@ -7732,7 +7732,7 @@ static int mysql_init_variables(void)
   cleanup_done= 0;
   server_id_supplied= 0;
   test_flags= select_errors= dropping_tables= ha_open_options=0;
-  global_thread_count= thread_running= kill_blocked_pthreads_flag= wake_pthread=0;
+  global_thread_count= num_thread_running= kill_blocked_pthreads_flag= wake_pthread=0;
   slave_open_temp_tables= 0;
   blocked_pthread_count= 0;
   opt_endinfo= using_udf_functions= 0;

=== modified file 'sql/mysqld.h'
--- a/sql/mysqld.h	2012-06-12 13:31:36 +0000
+++ b/sql/mysqld.h	2012-06-20 07:39:11 +0000
@@ -687,7 +687,7 @@ inc_thread_running()
 {
   int32 num_thread_running;
   my_atomic_rwlock_wrlock(&thread_running_lock);
-  num_thread_running= my_atomic_add32(&thread_running, 1);
+  num_thread_running= my_atomic_add32(&num_thread_running, 1);
   my_atomic_rwlock_wrunlock(&thread_running_lock);
   return (num_thread_running+1);
 }
@@ -697,21 +697,11 @@ dec_thread_running()
 {
   int32 num_thread_running;
   my_atomic_rwlock_wrlock(&thread_running_lock);
-  num_thread_running= my_atomic_add32(&thread_running, -1);
+  num_thread_running= my_atomic_add32(&num_thread_running, -1);
   my_atomic_rwlock_wrunlock(&thread_running_lock);
   return (num_thread_running-1);
 }
 
-inline int32
-get_thread_running()
-{
-  int32 num_thread_running;
-  my_atomic_rwlock_wrlock(&thread_running_lock);
-  num_thread_running= my_atomic_load32(&thread_running);
-  my_atomic_rwlock_wrunlock(&thread_running_lock);
-  return num_thread_running;
-}
-
 #if defined(MYSQL_DYNAMIC_PLUGIN) && defined(_WIN32)
 extern "C" THD *_current_thd_noinline();
 #define _current_thd() _current_thd_noinline()

=== modified file 'sql/rpl_slave.cc'
--- a/sql/rpl_slave.cc	2012-06-15 16:37:59 +0000
+++ b/sql/rpl_slave.cc	2012-06-20 07:39:11 +0000
@@ -2825,7 +2825,6 @@ static int init_slave_thread(THD* thd, S
   if (init_thr_lock() || thd->store_globals())
 #endif
   {
-    thd->cleanup();
     DBUG_RETURN(-1);
   }
 
@@ -4251,12 +4250,15 @@ err:
 
   DBUG_ASSERT(thd->net.buff != 0);
   net_end(&thd->net); // destructor will not free it, because net.vio is 0
+
+  thd->release_resources();
   mysql_mutex_lock(&LOCK_thread_count);
   THD_CHECK_SENTRY(thd);
   if (thd_added)
     remove_global_thread(thd);
   mysql_mutex_unlock(&LOCK_thread_count);
   delete thd;
+
   mi->abort_slave= 0;
   mi->slave_running= 0;
   mi->info_thd= 0;
@@ -4447,13 +4449,15 @@ err:
     DBUG_ASSERT(thd->net.buff != 0);
     net_end(&thd->net);
 
-    mysql_mutex_lock(&LOCK_thread_count);
-    THD_CHECK_SENTRY(thd);
     /*
       to avoid close_temporary_tables() closing temp tables as those
       are Coordinator's burden.
     */
     thd->system_thread= NON_SYSTEM_THREAD;
+    thd->release_resources();
+
+    mysql_mutex_lock(&LOCK_thread_count);
+    THD_CHECK_SENTRY(thd);
     if (thd_added)
       remove_global_thread(thd);
     mysql_mutex_unlock(&LOCK_thread_count);
@@ -5575,6 +5579,8 @@ llstr(rli->get_group_master_log_pos(), l
   THD_CHECK_SENTRY(thd);
   rli->info_thd= 0;
   set_thd_in_use_temporary_tables(rli);  // (re)set info_thd in use for saved temp tables
+
+  thd->release_resources();
   mysql_mutex_lock(&LOCK_thread_count);
   THD_CHECK_SENTRY(thd);
   if (thd_added)

=== modified file 'sql/scheduler.cc'
--- a/sql/scheduler.cc	2012-04-20 08:55:22 +0000
+++ b/sql/scheduler.cc	2012-06-20 07:39:11 +0000
@@ -32,7 +32,7 @@
 
 static bool no_threads_end(THD *thd, bool put_in_cache)
 {
-  thd_cleanup(thd);
+  thd_release_resources(thd);
   dec_connection_count();
 
   // THD is an incomplete type here, so use destroy_thd() to delete it.

=== modified file 'sql/sql_class.cc'
--- a/sql/sql_class.cc	2012-06-19 09:26:13 +0000
+++ b/sql/sql_class.cc	2012-06-20 07:39:11 +0000
@@ -933,6 +933,7 @@ THD::THD(bool enable_plugins)
   ull=0;
   system_thread= NON_SYSTEM_THREAD;
   cleanup_done= abort_on_warning= 0;
+  m_release_resources_done= false;
   peer_port= 0;					// For SHOW PROCESSLIST
   transaction.m_pending_rows_event= 0;
   transaction.flags.enabled= true;
@@ -1403,8 +1404,10 @@ void THD::change_user(void)
 }
 
 
-/* Do operations that may take a long time */
-
+/*
+  Do what's needed when one invokes change user.
+  Also used during THD::release_resources, i.e. prior to THD destruction.
+*/
 void THD::cleanup(void)
 {
   DBUG_ENTER("THD::cleanup");
@@ -1474,12 +1477,13 @@ void THD::cleanup(void)
 }
 
 
-THD::~THD()
+/**
+  Release most resources, prior to THD destruction.
+ */
+void THD::release_resources()
 {
   mysql_mutex_assert_not_owner(&LOCK_thread_count);
-  THD_CHECK_SENTRY(this);
-  DBUG_ENTER("~THD()");
-  DBUG_PRINT("info", ("THD dtor, this %p", this));
+  DBUG_ASSERT(m_release_resources_done == false);
 
   /* Ensure that no one is using THD */
   mysql_mutex_lock(&LOCK_thd_data);
@@ -1507,6 +1511,20 @@ THD::~THD()
   if (m_enable_plugins)
     plugin_thdvar_cleanup(this);
 
+  m_release_resources_done= true;
+}
+
+
+THD::~THD()
+{
+  mysql_mutex_assert_not_owner(&LOCK_thread_count);
+  THD_CHECK_SENTRY(this);
+  DBUG_ENTER("~THD()");
+  DBUG_PRINT("info", ("THD dtor, this %p", this));
+
+  if (!m_release_resources_done)
+    release_resources();
+
   clear_next_event_pos();
 
   DBUG_PRINT("info", ("freeing security context"));

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2012-06-19 09:26:13 +0000
+++ b/sql/sql_class.h	2012-06-20 07:39:11 +0000
@@ -2835,8 +2835,8 @@ public:
     it returned an error on master, and this is OK on the slave.
   */
   bool       is_slave_error;
-  bool       bootstrap, cleanup_done;
-  
+  bool       bootstrap;
+
   /**  is set if some thread specific value(s) used in a statement. */
   bool       thread_specific_used;
   /**  
@@ -2939,8 +2939,31 @@ public:
   bool m_enable_plugins;
 
   THD(bool enable_plugins= true);
+
+  /*
+    The THD dtor is effectively split in two:
+      THD::release_resources() and ~THD().
+
+    We want to minimize the time we hold LOCK_thread_count,
+    so when destroying a global thread, do:
+
+    thd->release_resources()
+    mysql_mutex_lock(&LOCK_thread_count);
+    remove_global_thread(thd);
+    mysql_mutex_unlock(&LOCK_thread_count);
+    delete thd;
+   */
   ~THD();
 
+  void release_resources();
+  bool release_resources_done() const { return m_release_resources_done; }
+
+private:
+  bool m_release_resources_done;
+  bool cleanup_done;
+  void cleanup(void);
+
+public:
   void init(void);
   /*
     Initialize memory roots necessary for query processing and (!)
@@ -2953,7 +2976,6 @@ public:
   */
   void init_for_queries(Relay_log_info *rli= NULL);
   void change_user(void);
-  void cleanup(void);
   void cleanup_after_query();
   bool store_globals();
   bool restore_globals();

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2012-06-11 11:30:25 +0000
+++ b/sql/sql_insert.cc	2012-06-20 07:39:11 +0000
@@ -2178,6 +2178,7 @@ public:
       close_thread_tables(&thd);
       thd.mdl_context.release_transactional_locks();
     }
+    thd.release_resources();
     mysql_mutex_lock(&LOCK_thread_count);
     mysql_mutex_destroy(&mutex);
     mysql_cond_destroy(&cond);

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2012-06-18 14:03:38 +0000
+++ b/sql/sql_parse.cc	2012-06-20 07:39:11 +0000
@@ -837,7 +837,7 @@ void do_handle_bootstrap(THD *thd)
 
 end:
   net_end(&thd->net);
-  thd->cleanup();
+  thd->release_resources();
 
   if (thd_added)
   {
@@ -846,9 +846,8 @@ end:
     mysql_mutex_unlock(&LOCK_thread_count);
   }
   /*
-    We need to delete the thd before signalling that bootstrap is done.
-    The reason is that we have to call ha_close_connection(thd)
-    before shutting down InnoDB (this is done by THD::~THD())
+    For safety we delete the thd before signalling that bootstrap is done,
+    since the server will be taken down immediately.
   */
   delete thd;
 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk branch (tor.didriksen:4028 to 4029) Bug#14100495Tor Didriksen20 Jun