MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:antony Date:January 23 2007 1:09pm
Subject:bk commit into 5.1 tree (acurtis:1.2379) BUG#25396
View as plain text  
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-23 05:09:14-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 causing a race in valgrind's pthread_cond_wait
  implementation.

  BUILD/compile-amd64-valgrind-max@stripped, 2007-01-23 05:09:11-08:00, acurtis@stripped +24 -0
    New BitKeeper file ``BUILD/compile-amd64-valgrind-max''

  BUILD/compile-amd64-valgrind-max@stripped, 2007-01-23 05:09:11-08:00, acurtis@stripped +0 -0

  sql/ha_ndbcluster.cc@stripped, 2007-01-23 05:09:11-08:00, acurtis@stripped +67 -46
    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@stripped, 2007-01-23 05:09:11-08:00, acurtis@stripped +7 -4
    Bug25386
    Valgrind 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-23 05:09:29 -08:00
+++ 1.604/sql/mysqld.cc	2007-01-23 05:09:29 -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/23 05:09:11
#! /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-23 05:09:29 -08:00
+++ 1.385/sql/ha_ndbcluster.cc	2007-01-23 05:09:29 -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;
@@ -159,6 +160,7 @@
 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 @@
 /* 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 @@
     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 @@
   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 @@
   
   (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 @@
     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 @@
   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 @@
   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 @@
   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 @@
 
   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 @@
   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 @@
   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 @@
   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 @@
 #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 @@
       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();
Thread
bk commit into 5.1 tree (acurtis:1.2379) BUG#25396antony23 Jan