From: Date: January 11 2007 11:31pm Subject: bk commit into 5.1 tree (acurtis:1.2379) BUG#25396 List-Archive: http://lists.mysql.com/commits/18001 X-Bug: 25396 Message-Id: <200701112231.l0BMVa9k011220@mail.mysql.com> Below is the list of changes that have just been committed into a local 5.1 repository of antony. When antony does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet@stripped, 2007-01-11 14:31:20-08:00, acurtis@stripped +3 -0 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. BUILD/compile-amd64-valgrind-max@stripped, 2007-01-11 14:31:14-08:00, acurtis@stripped +24 -0 New BitKeeper file ``BUILD/compile-amd64-valgrind-max'' BUILD/compile-amd64-valgrind-max@stripped, 2007-01-11 14:31:14-08:00, acurtis@stripped +0 -0 sql/ha_ndbcluster.cc@stripped, 2007-01-11 14:31:14-08:00, acurtis@stripped +62 -44 Bug25396 POSIX requires that mutex be held during call to pthread_cond_signal. 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. sql/mysqld.cc@stripped, 2007-01-11 14:31:14-08:00, acurtis@stripped +7 -4 Bug25386 POSIX requires that mutex be held during call to pthread_cond_signal. # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: acurtis # Host: ltamd64.xiphis.org # Root: /home/antony/work2/p1-bug25396.4 --- 1.603/sql/mysqld.cc 2007-01-11 14:31:35 -08:00 +++ 1.604/sql/mysqld.cc 2007-01-11 14:31:35 -08:00 @@ -3706,15 +3706,18 @@ 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 --- New file --- +++ BUILD/compile-amd64-valgrind-max 07/01/11 14:31:14 #! /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 --- 1.384/sql/ha_ndbcluster.cc 2007-01-11 14:31:35 -08:00 +++ 1.385/sql/ha_ndbcluster.cc 2007-01-11 14:31:35 -08:00 @@ -133,6 +133,7 @@ } static int ndbcluster_inited= 0; +static int ndbcluster_terminating= 0; static Ndb* g_ndb= NULL; Ndb_cluster_connection* g_ndb_cluster_connection= NULL; @@ -6588,6 +6589,7 @@ /* 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 +6599,7 @@ 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 +6610,14 @@ 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); + ndb_util_thread_running= -1; + ndbcluster_terminating= 0; ndb_dictionary_is_mysqld= 1; ndbcluster_hton= (handlerton *)p; @@ -6705,17 +6716,12 @@ (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; @@ -6731,9 +6737,19 @@ /* Wait for the util thread to start */ pthread_mutex_lock(&LOCK_ndb_util_thread); - while (!ndb_util_thread_running) + while (ndb_util_thread_running < 0) pthread_cond_wait(&COND_ndb_util_thread, &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); + goto ndbcluster_init_error; + } ndbcluster_inited= 1; DBUG_RETURN(FALSE); @@ -6760,22 +6776,12 @@ 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_thread, &LOCK_ndb_util_thread); pthread_mutex_unlock(&LOCK_ndb_util_thread); @@ -8357,6 +8363,8 @@ 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 +8374,7 @@ 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 +8385,27 @@ 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_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 +8415,9 @@ 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 +8425,7 @@ 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 +8444,22 @@ #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); + do + { + if (ndbcluster_terminating) /* Shutting down server */ + goto ndb_util_thread_end; + } + while (!pthread_cond_timedwait(&COND_ndb_util_thread, + &LOCK_ndb_util_thread, + &abstime)); 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 +8582,18 @@ 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_thread); pthread_mutex_unlock(&LOCK_ndb_util_thread); DBUG_PRINT("exit", ("ndb_util_thread")); my_thread_end();