Commit d51d6adc authored by unknown's avatar unknown

BUG#25306 (Race conditions during replication slave shutdown (valgrind stacks)):

The possibility of the race is removed by changing sequence of calls

  pthread_mutex_unlock(&mi->run_lock);
  pthread_cond_broadcast(&mi->stop_cond);  

into

  pthread_cond_broadcast(&mi->stop_cond);
  pthread_mutex_unlock(&mi->run_lock);

at the end of I/O thread (similar change at the end of SQL thread). This ensures 
that no thread waiting on the condition executes between the broadcast and the 
unlock and thus can't delete the mi structure which caused the bug.


sql/slave.cc:
  Change order of condition broadcast and mutex unlock at the end of slave's 
  I/O thread and SQL thread.
sql/sql_repl.cc:
  Add DBUG_ENTER() call
parent 501bf6de
...@@ -3758,8 +3758,13 @@ err: ...@@ -3758,8 +3758,13 @@ err:
mi->abort_slave= 0; mi->abort_slave= 0;
mi->slave_running= 0; mi->slave_running= 0;
mi->io_thd= 0; mi->io_thd= 0;
pthread_mutex_unlock(&mi->run_lock); /*
Note: the order of the two following calls (first broadcast, then unlock)
is important. Otherwise a killer_thread can execute between the calls and
delete the mi structure leading to a crash! (see BUG#25306 for details)
*/
pthread_cond_broadcast(&mi->stop_cond); // tell the world we are done pthread_cond_broadcast(&mi->stop_cond); // tell the world we are done
pthread_mutex_unlock(&mi->run_lock);
#ifndef DBUG_OFF #ifndef DBUG_OFF
if (abort_slave_event_count && !events_till_abort) if (abort_slave_event_count && !events_till_abort)
goto slave_begin; goto slave_begin;
...@@ -3977,8 +3982,13 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \ ...@@ -3977,8 +3982,13 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \
THD_CHECK_SENTRY(thd); THD_CHECK_SENTRY(thd);
delete thd; delete thd;
pthread_mutex_unlock(&LOCK_thread_count); pthread_mutex_unlock(&LOCK_thread_count);
pthread_cond_broadcast(&rli->stop_cond);
/*
Note: the order of the broadcast and unlock calls below (first broadcast, then unlock)
is important. Otherwise a killer_thread can execute between the calls and
delete the mi structure leading to a crash! (see BUG#25306 for details)
*/
pthread_cond_broadcast(&rli->stop_cond);
#ifndef DBUG_OFF #ifndef DBUG_OFF
/* /*
Bug #19938 Valgrind error (race) in handle_slave_sql() Bug #19938 Valgrind error (race) in handle_slave_sql()
...@@ -3986,9 +3996,8 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \ ...@@ -3986,9 +3996,8 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \
*/ */
const int eta= rli->events_till_abort; const int eta= rli->events_till_abort;
#endif #endif
pthread_mutex_unlock(&rli->run_lock); // tell the world we are done
// tell the world we are done
pthread_mutex_unlock(&rli->run_lock);
#ifndef DBUG_OFF // TODO: reconsider the code below #ifndef DBUG_OFF // TODO: reconsider the code below
if (abort_slave_event_count && !eta) if (abort_slave_event_count && !eta)
goto slave_begin; goto slave_begin;
......
...@@ -882,12 +882,14 @@ int start_slave(THD* thd , MASTER_INFO* mi, bool net_report) ...@@ -882,12 +882,14 @@ int start_slave(THD* thd , MASTER_INFO* mi, bool net_report)
int stop_slave(THD* thd, MASTER_INFO* mi, bool net_report ) int stop_slave(THD* thd, MASTER_INFO* mi, bool net_report )
{ {
DBUG_ENTER("stop_slave");
int slave_errno; int slave_errno;
if (!thd) if (!thd)
thd = current_thd; thd = current_thd;
if (check_access(thd, SUPER_ACL, any_db,0,0,0,0)) if (check_access(thd, SUPER_ACL, any_db,0,0,0,0))
return 1; DBUG_RETURN(1);
thd->proc_info = "Killing slave"; thd->proc_info = "Killing slave";
int thread_mask; int thread_mask;
lock_slave_threads(mi); lock_slave_threads(mi);
...@@ -921,12 +923,12 @@ int stop_slave(THD* thd, MASTER_INFO* mi, bool net_report ) ...@@ -921,12 +923,12 @@ int stop_slave(THD* thd, MASTER_INFO* mi, bool net_report )
{ {
if (net_report) if (net_report)
my_message(slave_errno, ER(slave_errno), MYF(0)); my_message(slave_errno, ER(slave_errno), MYF(0));
return 1; DBUG_RETURN(1);
} }
else if (net_report) else if (net_report)
send_ok(thd); send_ok(thd);
return 0; DBUG_RETURN(0);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment