bk commit into 5.1 tree (mats:1.2556) BUG#29968

[Mats Kindahl]

Below is the list of changes that have just been committed into a local
5.1 repository of mats. When mats 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-08-29 15:11:33+02:00, mats@stripped +1 -0
  BUG#29968 (rpl_ndb_circular.test and rpl_ndb_log.test fail):
  
  The fields slave_running, io_thd, and sql_thread are guarded by an
  associated run_lock. A read of these fields were not guarded inside
  terminate_slave_threads(), which caused an assertion to fire.
  
  This patch removes that assertion and reorganizes the code in
  terminate_slave_thread() slightly.

  sql/slave.cc@stripped, 2007-08-29 15:11:30+02:00, mats@stripped +11 -8
    The thread variable for each of the slave threads can change before
    acquiring the run_lock mutex inside terminate_slave_thread(). Hence
    an assertion was removed that read the variable without guarding it 
    with run_lock.
    
    Code that checked *slave_running status inside terminate_slave_thread()
    was reorganized slightly.

diff -Nrup a/sql/slave.cc b/sql/slave.cc
--- a/sql/slave.cc	2007-08-29 09:14:51 +02:00
+++ b/sql/slave.cc	2007-08-29 15:11:30 +02:00
@@ -132,7 +132,7 @@ static int create_table_from_dump(THD* t
                                   const char* table_name, bool overwrite);
 static int get_master_version_and_clock(MYSQL* mysql, MASTER_INFO* mi);
 static Log_event* next_event(RELAY_LOG_INFO* rli);
-static int terminate_slave_thread(THD* thd,
+static int terminate_slave_thread(THD *thd,
                                   pthread_mutex_t* term_lock,
                                   pthread_cond_t* term_cond,
                                   volatile uint *slave_running,
@@ -332,7 +332,6 @@ int terminate_slave_threads(MASTER_INFO*
   if ((thread_mask & (SLAVE_SQL|SLAVE_FORCE_ALL)))
   {
     DBUG_PRINT("info",("Terminating SQL thread"));
-    DBUG_ASSERT(mi->rli.sql_thd != 0) ;
     mi->rli.abort_slave=1;
     if ((error=terminate_slave_thread(mi->rli.sql_thd,sql_lock,
                                       &mi->rli.stop_cond,
@@ -375,7 +374,7 @@ int terminate_slave_threads(MASTER_INFO*
    @retval 0 All OK
  */
 static int
-terminate_slave_thread(THD* thd,
+terminate_slave_thread(THD *thd,
                        pthread_mutex_t* term_lock,
                        pthread_cond_t* term_cond,
                        volatile uint *slave_running,
@@ -384,17 +383,21 @@ terminate_slave_thread(THD* thd,
   int error;
 
   DBUG_ENTER("terminate_slave_thread");
+
   if (!skip_lock)
-  {
     pthread_mutex_lock(term_lock);
-    if (!*slave_running)
-    {
+
+  safe_mutex_assert_owner(term_lock);
+
+  if (!*slave_running)
+  {
+    if (!skip_lock)
       pthread_mutex_unlock(term_lock);
-      DBUG_RETURN(ER_SLAVE_NOT_RUNNING);
-    }
+    DBUG_RETURN(ER_SLAVE_NOT_RUNNING);
   }
   DBUG_ASSERT(thd != 0);
   THD_CHECK_SENTRY(thd);
+
   /*
     Is is critical to test if the slave is running. Otherwise, we might
     be referening freed memory trying to kick it