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 13:29:22+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 and
  potentially can cause a segmentation fault in some rare cases.
  
  This patch move the reading of the above variables to occur inside
  terminate_slave_thread() guarded by the associated run_lock.

  sql/slave.cc@stripped, 2007-08-29 13:29:18+02:00, mats@stripped +21 -6
    The thread variable for each of the slave threads can change before
    acquiring the run_lock mutex inside terminate_slave_thread(). Hence
    the signature of the function was changed to take a pointer to the
    thd variable, an assertion removed, and code added to terminate_
    slave_thread() that makes calling the function with a NULL thread
    structure legal, and it will be checked once the mutex is acquired.

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 13:29:18 +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 **pthd,
                                   pthread_mutex_t* term_lock,
                                   pthread_cond_t* term_cond,
                                   volatile uint *slave_running,
@@ -322,7 +322,7 @@ int terminate_slave_threads(MASTER_INFO*
   {
     DBUG_PRINT("info",("Terminating IO thread"));
     mi->abort_slave=1;
-    if ((error=terminate_slave_thread(mi->io_thd,io_lock,
+    if ((error=terminate_slave_thread(&mi->io_thd,io_lock,
                                       &mi->stop_cond,
                                       &mi->slave_running,
                                       skip_lock)) &&
@@ -332,9 +332,8 @@ 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,
+    if ((error=terminate_slave_thread(&mi->rli.sql_thd,sql_lock,
                                       &mi->rli.stop_cond,
                                       &mi->rli.slave_running,
                                       skip_lock)) &&
@@ -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 **pthd,
                        pthread_mutex_t* term_lock,
                        pthread_cond_t* term_cond,
                        volatile uint *slave_running,
@@ -393,8 +392,24 @@ terminate_slave_thread(THD* thd,
       DBUG_RETURN(ER_SLAVE_NOT_RUNNING);
     }
   }
-  DBUG_ASSERT(thd != 0);
+  safe_mutex_assert_owner(term_lock);
+
+  /*
+    The value of the thread variable can change while the term_lock is
+    not held. Therefore, we do not check the value of the thread
+    variable until the term_lock has been acquired.
+  */
+  if (*pthd == 0)
+  {
+    if (!skip_lock)
+      pthread_mutex_unlock(term_lock);
+    DBUG_RETURN(ER_SLAVE_NOT_RUNNING);
+  }
+
+  THD *const thd= *pthd;
+
   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