From d48e864f2faf06867988e5b9b9346a56fb86b818 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 23 Jan 2007 05:09:14 -0800 Subject: [PATCH] Bug#25396 "Valgrind leak in closecon_handlerton" plugin_shutdown() calls plugin_deinitialize() which calls ha_finalize_handlerton(). ndbcluster_end() fails to wait for the ndb utility thread to exit which results in the handlerton struct being freed before the ndb utility thread has destroyed it's THD but before the plugin has been marked as UNINITIALIZED Bug is caused by misuse of abort_loops variable and not locking mutex during calls to pthread condition variable functions causing a race in valgrind's pthread_cond_wait implementation. sql/ha_ndbcluster.cc: Bug25396 Valgrind requires that mutex be held during call to pthread_cond_signal. Change pthread_cond_timedwait() to pthread_cond_wait() where the timeout is not needed. Ensure that appropiate variables are protected by mutex. Remove use of abort_loop global variable. Ensure that ndbcluster_end waits for util thread to exit. Add an extra cond_var as insurance against non-conforming pthreads implementations. sql/mysqld.cc: Bug25386 Valgrind requires that mutex be held during call to pthread_cond_signal. BUILD/compile-amd64-valgrind-max: New BitKeeper file ``BUILD/compile-amd64-valgrind-max'' --- BUILD/compile-amd64-valgrind-max | 24 +++++++ sql/ha_ndbcluster.cc | 113 ++++++++++++++++++------------- sql/mysqld.cc | 11 +-- 3 files changed, 98 insertions(+), 50 deletions(-) create mode 100755 BUILD/compile-amd64-valgrind-max diff --git a/BUILD/compile-amd64-valgrind-max b/BUILD/compile-amd64-valgrind-max new file mode 100755 index 00000000000..962d0f17b04 --- /dev/null +++ b/BUILD/compile-amd64-valgrind-max @@ -0,0 +1,24 @@ +#! /bin/sh + +path=`dirname $0` +. "$path/SETUP.sh" + +extra_flags="$amd64_cflags $debug_cflags $valgrind_flags" +extra_configs="$amd64_configs $debug_configs $max_configs" + +. "$path/FINISH.sh" + +if test -z "$just_print" +then + set +v +x + echo "\ +****************************************************************************** +Note that by default BUILD/compile-pentium-valgrind-max calls 'configure' with +--enable-assembler. When Valgrind detects an error involving an assembly +function (for example an uninitialized value used as an argument of an +assembly function), Valgrind will not print the stacktrace and 'valgrind +--gdb-attach=yes' will not work either. If you need a stacktrace in those +cases, you have to run BUILD/compile-pentium-valgrind-max with the +--disable-assembler argument. +******************************************************************************" +fi diff --git a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc index 5614cc3ecd8..07887c2ac1f 100644 --- a/sql/ha_ndbcluster.cc +++ b/sql/ha_ndbcluster.cc @@ -133,6 +133,7 @@ static uint ndbcluster_alter_table_flags(uint flags) } static int ndbcluster_inited= 0; +static int ndbcluster_terminating= 0; static Ndb* g_ndb= NULL; Ndb_cluster_connection* g_ndb_cluster_connection= NULL; @@ -159,6 +160,7 @@ pthread_t ndb_util_thread; int ndb_util_thread_running= 0; pthread_mutex_t LOCK_ndb_util_thread; pthread_cond_t COND_ndb_util_thread; +pthread_cond_t COND_ndb_util_ready; pthread_handler_t ndb_util_thread_func(void *arg); ulong ndb_cache_check_time; @@ -6588,6 +6590,7 @@ int ndbcluster_find_files(handlerton *hton, THD *thd, /* Call back after cluster connect */ static int connect_callback() { + pthread_mutex_lock(&LOCK_ndb_util_thread); update_status_variables(g_ndb_cluster_connection); uint node_id, i= 0; @@ -6597,6 +6600,7 @@ static int connect_callback() g_node_id_map[node_id]= i++; pthread_cond_signal(&COND_ndb_util_thread); + pthread_mutex_unlock(&LOCK_ndb_util_thread); return 0; } @@ -6607,6 +6611,15 @@ static int ndbcluster_init(void *p) int res; DBUG_ENTER("ndbcluster_init"); + if (ndbcluster_inited) + DBUG_RETURN(FALSE); + + pthread_mutex_init(&ndbcluster_mutex,MY_MUTEX_INIT_FAST); + pthread_mutex_init(&LOCK_ndb_util_thread, MY_MUTEX_INIT_FAST); + pthread_cond_init(&COND_ndb_util_thread, NULL); + pthread_cond_init(&COND_ndb_util_ready, NULL); + ndb_util_thread_running= -1; + ndbcluster_terminating= 0; ndb_dictionary_is_mysqld= 1; ndbcluster_hton= (handlerton *)p; @@ -6705,17 +6718,12 @@ static int ndbcluster_init(void *p) (void) hash_init(&ndbcluster_open_tables,system_charset_info,32,0,0, (hash_get_key) ndbcluster_get_key,0,0); - pthread_mutex_init(&ndbcluster_mutex,MY_MUTEX_INIT_FAST); #ifdef HAVE_NDB_BINLOG /* start the ndb injector thread */ if (ndbcluster_binlog_start()) goto ndbcluster_init_error; #endif /* HAVE_NDB_BINLOG */ - pthread_mutex_init(&LOCK_ndb_util_thread, MY_MUTEX_INIT_FAST); - pthread_cond_init(&COND_ndb_util_thread, NULL); - - ndb_cache_check_time = opt_ndb_cache_check_time; // Create utility thread pthread_t tmp; @@ -6726,14 +6734,26 @@ static int ndbcluster_init(void *p) pthread_mutex_destroy(&ndbcluster_mutex); pthread_mutex_destroy(&LOCK_ndb_util_thread); pthread_cond_destroy(&COND_ndb_util_thread); + pthread_cond_destroy(&COND_ndb_util_ready); goto ndbcluster_init_error; } /* Wait for the util thread to start */ pthread_mutex_lock(&LOCK_ndb_util_thread); - while (!ndb_util_thread_running) - pthread_cond_wait(&COND_ndb_util_thread, &LOCK_ndb_util_thread); + while (ndb_util_thread_running < 0) + pthread_cond_wait(&COND_ndb_util_ready, &LOCK_ndb_util_thread); pthread_mutex_unlock(&LOCK_ndb_util_thread); + + if (!ndb_util_thread_running) + { + DBUG_PRINT("error", ("ndb utility thread exited prematurely")); + hash_free(&ndbcluster_open_tables); + pthread_mutex_destroy(&ndbcluster_mutex); + pthread_mutex_destroy(&LOCK_ndb_util_thread); + pthread_cond_destroy(&COND_ndb_util_thread); + pthread_cond_destroy(&COND_ndb_util_ready); + goto ndbcluster_init_error; + } ndbcluster_inited= 1; DBUG_RETURN(FALSE); @@ -6760,22 +6780,12 @@ static int ndbcluster_end(handlerton *hton, ha_panic_function type) ndbcluster_inited= 0; /* wait for util thread to finish */ + sql_print_information("Stopping Cluster Utility thread"); pthread_mutex_lock(&LOCK_ndb_util_thread); - if (ndb_util_thread_running > 0) - { - pthread_cond_signal(&COND_ndb_util_thread); - pthread_mutex_unlock(&LOCK_ndb_util_thread); - - pthread_mutex_lock(&LOCK_ndb_util_thread); - while (ndb_util_thread_running > 0) - { - struct timespec abstime; - set_timespec(abstime, 1); - pthread_cond_timedwait(&COND_ndb_util_thread, - &LOCK_ndb_util_thread, - &abstime); - } - } + ndbcluster_terminating= 1; + pthread_cond_signal(&COND_ndb_util_thread); + while (ndb_util_thread_running > 0) + pthread_cond_wait(&COND_ndb_util_ready, &LOCK_ndb_util_thread); pthread_mutex_unlock(&LOCK_ndb_util_thread); @@ -6824,6 +6834,7 @@ static int ndbcluster_end(handlerton *hton, ha_panic_function type) pthread_mutex_destroy(&ndbcluster_mutex); pthread_mutex_destroy(&LOCK_ndb_util_thread); pthread_cond_destroy(&COND_ndb_util_thread); + pthread_cond_destroy(&COND_ndb_util_ready); DBUG_RETURN(0); } @@ -8357,6 +8368,8 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) my_thread_init(); DBUG_ENTER("ndb_util_thread"); DBUG_PRINT("enter", ("ndb_cache_check_time: %lu", ndb_cache_check_time)); + + pthread_mutex_lock(&LOCK_ndb_util_thread); thd= new THD; /* note that contructor of THD uses DBUG_ */ THD_CHECK_SENTRY(thd); @@ -8366,12 +8379,7 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) thd->thread_stack= (char*)&thd; /* remember where our stack is */ if (thd->store_globals()) - { - thd->cleanup(); - delete thd; - ndb_util_thread_running= 0; - DBUG_RETURN(NULL); - } + goto ndb_util_thread_fail; thd->init_for_queries(); thd->version=refresh_version; thd->set_time(); @@ -8382,15 +8390,27 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) thd->main_security_ctx.priv_user = 0; thd->current_stmt_binlog_row_based= TRUE; // If in mixed mode + /* Signal successful initialization */ ndb_util_thread_running= 1; - pthread_cond_signal(&COND_ndb_util_thread); + pthread_cond_signal(&COND_ndb_util_ready); + pthread_mutex_unlock(&LOCK_ndb_util_thread); /* wait for mysql server to start */ pthread_mutex_lock(&LOCK_server_started); while (!mysqld_server_started) - pthread_cond_wait(&COND_server_started, &LOCK_server_started); + { + set_timespec(abstime, 1); + pthread_cond_timedwait(&COND_server_started, &LOCK_server_started, + &abstime); + if (ndbcluster_terminating) + { + pthread_mutex_unlock(&LOCK_server_started); + pthread_mutex_lock(&LOCK_ndb_util_thread); + goto ndb_util_thread_end; + } + } pthread_mutex_unlock(&LOCK_server_started); /* @@ -8400,15 +8420,9 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) while (!ndb_cluster_node_id && (ndbcluster_hton->slot != ~(uint)0)) { /* ndb not connected yet */ - set_timespec(abstime, 1); - pthread_cond_timedwait(&COND_ndb_util_thread, - &LOCK_ndb_util_thread, - &abstime); - if (abort_loop) - { - pthread_mutex_unlock(&LOCK_ndb_util_thread); + pthread_cond_wait(&COND_ndb_util_thread, &LOCK_ndb_util_thread); + if (ndbcluster_terminating) goto ndb_util_thread_end; - } } pthread_mutex_unlock(&LOCK_ndb_util_thread); @@ -8416,6 +8430,7 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) if (!(thd_ndb= ha_ndbcluster::seize_thd_ndb())) { sql_print_error("Could not allocate Thd_ndb object"); + pthread_mutex_lock(&LOCK_ndb_util_thread); goto ndb_util_thread_end; } set_thd_ndb(thd, thd_ndb); @@ -8434,19 +8449,20 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) #endif set_timespec(abstime, 0); - for (;!abort_loop;) + for (;;) { pthread_mutex_lock(&LOCK_ndb_util_thread); - pthread_cond_timedwait(&COND_ndb_util_thread, - &LOCK_ndb_util_thread, - &abstime); + if (!ndbcluster_terminating) + pthread_cond_timedwait(&COND_ndb_util_thread, + &LOCK_ndb_util_thread, + &abstime); + if (ndbcluster_terminating) /* Shutting down server */ + goto ndb_util_thread_end; pthread_mutex_unlock(&LOCK_ndb_util_thread); #ifdef NDB_EXTRA_DEBUG_UTIL_THREAD DBUG_PRINT("ndb_util_thread", ("Started, ndb_cache_check_time: %lu", ndb_cache_check_time)); #endif - if (abort_loop) - break; /* Shutting down server */ #ifdef HAVE_NDB_BINLOG /* @@ -8569,13 +8585,18 @@ pthread_handler_t ndb_util_thread_func(void *arg __attribute__((unused))) abstime.tv_nsec-= 1000000000; } } + + pthread_mutex_lock(&LOCK_ndb_util_thread); + ndb_util_thread_end: - sql_print_information("Stopping Cluster Utility thread"); net_end(&thd->net); +ndb_util_thread_fail: thd->cleanup(); delete thd; - pthread_mutex_lock(&LOCK_ndb_util_thread); + + /* signal termination */ ndb_util_thread_running= 0; + pthread_cond_signal(&COND_ndb_util_ready); pthread_mutex_unlock(&LOCK_ndb_util_thread); DBUG_PRINT("exit", ("ndb_util_thread")); my_thread_end(); diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 72add4d3aa4..2be5c2b342b 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3706,15 +3706,18 @@ we force server id to 2, but this MySQL server will not act as a slave."); mysqld_port, MYSQL_COMPILATION_COMMENT); - // Signal threads waiting for server to be started - mysqld_server_started= 1; - pthread_cond_signal(&COND_server_started); - if (!opt_noacl) { if (Events::get_instance()->init()) unireg_abort(1); } + + /* Signal threads waiting for server to be started */ + pthread_mutex_lock(&LOCK_server_started); + mysqld_server_started= 1; + pthread_cond_signal(&COND_server_started); + pthread_mutex_unlock(&LOCK_server_started); + #if defined(__NT__) || defined(HAVE_SMEM) handle_connections_methods(); #else