Skip to content

Commit

Permalink
MDEV-11221 - main.events_restart failed in bb
Browse files Browse the repository at this point in the history
Applied lost in a merge revision 7f38a07:

MDEV-10043 - main.events_restart fails sporadically in buildbot (crashes upon
             shutdown)

There was race condition between shutdown thread and event worker threads.

Shutdown thread waits for thread_count to become 0 in close_connections(). It
may happen so that event worker thread was started but didn't increment
thread_count by this time. In this case shutdown thread may miss wait for this
working thread and continue deinitialization. Worker thread in turn may continue
execution and crash on deinitialized data.

Fixed by incrementing thread_count before thread is actually created like it is
done for connection threads.

Also let event scheduler not to inc/dec running threads counter for symmetry
with other "service" threads.
  • Loading branch information
Sergey Vojtovich committed Mar 1, 2017
1 parent cc413ce commit e9ad4bd
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 44 deletions.
40 changes: 14 additions & 26 deletions sql/event_scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,6 @@ post_init_event_thread(THD *thd)
thd->cleanup();
return TRUE;
}

thread_safe_increment32(&thread_count, &thread_count_lock);
mysql_mutex_lock(&LOCK_thread_count);
threads.append(thd);
mysql_mutex_unlock(&LOCK_thread_count);
inc_thread_running();
return FALSE;
}

Expand All @@ -157,7 +151,13 @@ deinit_event_thread(THD *thd)
thd->proc_info= "Clearing";
DBUG_PRINT("exit", ("Event thread finishing"));

delete_running_thd(thd);
mysql_mutex_lock(&LOCK_thread_count);
thd->unlink();
mysql_mutex_unlock(&LOCK_thread_count);

delete thd;
thread_safe_decrement32(&thread_count, &thread_count_lock);
signal_thd_deleted();
}


Expand Down Expand Up @@ -191,8 +191,10 @@ pre_init_event_thread(THD* thd)
thd->net.read_timeout= slave_net_timeout;
thd->variables.option_bits|= OPTION_AUTO_IS_NULL;
thd->client_capabilities|= CLIENT_MULTI_RESULTS;
thread_safe_increment32(&thread_count, &thread_count_lock);
mysql_mutex_lock(&LOCK_thread_count);
thd->thread_id= thd->variables.pseudo_thread_id= thread_id++;
threads.append(thd);
mysql_mutex_unlock(&LOCK_thread_count);

/*
Expand Down Expand Up @@ -240,13 +242,8 @@ event_scheduler_thread(void *arg)
my_free(arg);
if (!res)
scheduler->run(thd);
else
{
thd->proc_info= "Clearing";
net_end(&thd->net);
delete thd;
}

deinit_event_thread(thd);
DBUG_LEAVE; // Against gcc warnings
my_thread_end();
return 0;
Expand Down Expand Up @@ -310,6 +307,7 @@ Event_worker_thread::run(THD *thd, Event_queue_element_for_exec *event)
DBUG_ENTER("Event_worker_thread::run");
DBUG_PRINT("info", ("Time is %ld, THD: 0x%lx", (long) my_time(0), (long) thd));

inc_thread_running();
if (res)
goto end;

Expand Down Expand Up @@ -338,6 +336,7 @@ Event_worker_thread::run(THD *thd, Event_queue_element_for_exec *event)
event->name.str));

delete event;
dec_thread_running();
deinit_event_thread(thd);

DBUG_VOID_RETURN;
Expand Down Expand Up @@ -442,13 +441,9 @@ Event_scheduler::start(int *err_no)
" Can not create thread for event scheduler (errno=%d)",
*err_no);

new_thd->proc_info= "Clearing";
DBUG_ASSERT(new_thd->net.buff != 0);
net_end(&new_thd->net);

state= INITIALIZED;
scheduler_thd= NULL;
delete new_thd;
deinit_event_thread(new_thd);

delete scheduler_param_value;
ret= true;
Expand Down Expand Up @@ -515,7 +510,6 @@ Event_scheduler::run(THD *thd)
}

LOCK_DATA();
deinit_event_thread(thd);
scheduler_thd= NULL;
state= INITIALIZED;
DBUG_PRINT("info", ("Broadcasting COND_state back to the stoppers"));
Expand Down Expand Up @@ -575,10 +569,7 @@ Event_scheduler::execute_top(Event_queue_element_for_exec *event_name)
sql_print_error("Event_scheduler::execute_top: Can not create event worker"
" thread (errno=%d). Stopping event scheduler", res);

new_thd->proc_info= "Clearing";
DBUG_ASSERT(new_thd->net.buff != 0);
net_end(&new_thd->net);

deinit_event_thread(new_thd);
goto error;
}

Expand All @@ -590,9 +581,6 @@ Event_scheduler::execute_top(Event_queue_element_for_exec *event_name)

error:
DBUG_PRINT("error", ("Event_scheduler::execute_top() res: %d", res));
if (new_thd)
delete new_thd;

delete event_name;
DBUG_RETURN(TRUE);
}
Expand Down
17 changes: 0 additions & 17 deletions sql/mysqld.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2647,23 +2647,6 @@ void dec_connection_count(THD *thd)
}


/*
Delete THD and decrement thread counters, including thread_running
*/

void delete_running_thd(THD *thd)
{
mysql_mutex_lock(&LOCK_thread_count);
thd->unlink();
mysql_mutex_unlock(&LOCK_thread_count);

delete thd;
dec_thread_running();
thread_safe_decrement32(&thread_count, &thread_count_lock);
signal_thd_deleted();
}


/*
Send a signal to unblock close_conneciton() if there is no more
threads running with a THD attached
Expand Down
1 change: 0 additions & 1 deletion sql/mysqld.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ void kill_mysql(void);
void close_connection(THD *thd, uint sql_errno= 0);
void handle_connection_in_main_thread(THD *thd);
void create_thread_to_handle_connection(THD *thd);
void delete_running_thd(THD *thd);
void signal_thd_deleted();
void unlink_thd(THD *thd);
bool one_thread_per_connection_end(THD *thd, bool put_in_cache);
Expand Down

0 comments on commit e9ad4bd

Please sign in to comment.