Fix for:

Bug #4810 "deadlock with KILL when the victim was in a wait state"
(I included mutex unlock into exit_cond() for future safety)
and BUG#4827 "KILL while START SLAVE may lead to replication slave crash"


sql/lock.cc:
  we did exit_cond() before unlock(LOCK_open), which led to deadlocks with THD::awake(). Fixing this.
sql/log.cc:
  mutex unlock is now included in exit_cond()
sql/repl_failsafe.cc:
  we did exit_cond() before unlock(LOCK_rpl_status), which led to deadlocks with THD::awake(). Fixing this.
sql/slave.cc:
  we did exit_cond() before unlock(cond_lock), which led to deadlocks with THD::awake(). Fixing this.
  Fixing also that if killed while waiting for slave thread to start, we don't release the mutex
  (that caused a double release of the mutex => crash).
sql/sql_class.h:
  comments about exit_cond()/enter_cond().
  Mutex unlock is now included in exit_cond() so that it's always done in the good order.
sql/sql_table.cc:
  unlock is now included in exit_cond().

1 files changed, 7 insertions, 11 deletions

diff --git a/sql/slave.cc b/sql/slave.cc
index 2269fc8d8cf..4416a2544ef 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -582,7 +582,7 @@ int start_slave_thread(pthread_handler h_func, pthread_mutex_t *start_lock,
       pthread_mutex_unlock(start_lock);
     DBUG_RETURN(ER_SLAVE_THREAD);
   }
-  if (start_cond && cond_lock)
+  if (start_cond && cond_lock) // caller has cond_lock
   {
     THD* thd = current_thd;
     while (start_id == *slave_run_id)
@@ -592,11 +592,9 @@ int start_slave_thread(pthread_handler h_func, pthread_mutex_t *start_lock,
 					    "Waiting for slave thread to start");
       pthread_cond_wait(start_cond,cond_lock);
       thd->exit_cond(old_msg);
+      pthread_mutex_lock(cond_lock); // re-acquire it as exit_cond() released
       if (thd->killed)
-      {
-	pthread_mutex_unlock(cond_lock);
 	DBUG_RETURN(ER_SERVER_SHUTDOWN);
-      }
     }
   }
   if (start_lock)
@@ -1561,7 +1559,6 @@ thread to free enough relay log space");
          !rli->ignore_log_space_limit)
     pthread_cond_wait(&rli->log_space_cond, &rli->log_space_lock);
   thd->exit_cond(save_proc_info);
-  pthread_mutex_unlock(&rli->log_space_lock);
   DBUG_RETURN(slave_killed);
 }
 
@@ -1965,6 +1962,9 @@ int st_relay_log_info::wait_for_pos(THD* thd, String* log_name,
                       (long) timeout));
 
   pthread_mutex_lock(&data_lock);
+  const char *msg= thd->enter_cond(&data_cond, &data_lock,
+                                   "Waiting for the SQL slave thread to "
+                                   "advance position");
   /* 
      This function will abort when it notices that some CHANGE MASTER or
      RESET MASTER has changed the master info.
@@ -2063,9 +2063,6 @@ int st_relay_log_info::wait_for_pos(THD* thd, String* log_name,
     //wait for master update, with optional timeout.
     
     DBUG_PRINT("info",("Waiting for master update"));
-    const char* msg = thd->enter_cond(&data_cond, &data_lock,
-                                      "Waiting for the SQL slave thread to \
-advance position");
     /*
       We are going to pthread_cond_(timed)wait(); if the SQL thread stops it
       will wake us up.
@@ -2087,8 +2084,7 @@ advance position");
     }
     else
       pthread_cond_wait(&data_cond, &data_lock);
-    DBUG_PRINT("info",("Got signal of master update"));
-    thd->exit_cond(msg);
+    DBUG_PRINT("info",("Got signal of master update or timed out"));
     if (error == ETIMEDOUT || error == ETIME)
     {
       error= -1;
@@ -2100,7 +2096,7 @@ advance position");
   }
 
 err:
-  pthread_mutex_unlock(&data_lock);
+  thd->exit_cond(msg);
   DBUG_PRINT("exit",("killed: %d  abort: %d  slave_running: %d \
 improper_arguments: %d  timed_out: %d",
                      (int) thd->killed,