MDEV-32728: Wrong mutex usage 'LOCK_thd_data' and 'wait_mutex'
Checking for kill with thd_kill_level() or check_killed() runs apc requests, which takes the LOCK_thd_kill mutex. But this is dangerous, as checking for kill needs to be called while holding many different mutexes, and can lead to cyclic mutex dependency and deadlock. But running apc is only "best effort", so skip running the apc if the LOCK_thd_kill is not available. The apc will then be run on next check of kill signal. Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
This commit is contained in:
parent
fefd6d5559
commit
2a4c573339
@ -197,13 +197,16 @@ bool Apc_target::make_apc_call(THD *caller_thd, Apc_call *call,
|
||||
This should be called periodically by the APC target thread.
|
||||
*/
|
||||
|
||||
void Apc_target::process_apc_requests()
|
||||
void Apc_target::process_apc_requests(bool force)
|
||||
{
|
||||
while (1)
|
||||
{
|
||||
Call_request *request;
|
||||
|
||||
mysql_mutex_lock(LOCK_thd_kill_ptr);
|
||||
|
||||
if (force)
|
||||
mysql_mutex_lock(LOCK_thd_kill_ptr);
|
||||
else if (mysql_mutex_trylock(LOCK_thd_kill_ptr))
|
||||
break; // Mutex is blocked, try again later
|
||||
if (!(request= get_first_in_queue()))
|
||||
{
|
||||
/* No requests in the queue */
|
||||
|
@ -70,10 +70,10 @@ public:
|
||||
bool process= !--enabled && have_apc_requests();
|
||||
mysql_mutex_unlock(LOCK_thd_kill_ptr);
|
||||
if (unlikely(process))
|
||||
process_apc_requests();
|
||||
process_apc_requests(true);
|
||||
}
|
||||
|
||||
void process_apc_requests();
|
||||
void process_apc_requests(bool force);
|
||||
/*
|
||||
A lightweight function, intended to be used in frequent checks like this:
|
||||
|
||||
|
@ -4724,7 +4724,7 @@ extern "C" enum thd_kill_levels thd_kill_level(const MYSQL_THD thd)
|
||||
if (unlikely(apc_target->have_apc_requests()))
|
||||
{
|
||||
if (thd == current_thd)
|
||||
apc_target->process_apc_requests();
|
||||
apc_target->process_apc_requests(false);
|
||||
}
|
||||
return THD_IS_NOT_KILLED;
|
||||
}
|
||||
|
@ -3606,7 +3606,7 @@ public:
|
||||
return TRUE;
|
||||
}
|
||||
if (apc_target.have_apc_requests())
|
||||
apc_target.process_apc_requests();
|
||||
apc_target.process_apc_requests(false);
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
|
@ -100,7 +100,7 @@ void *test_apc_service_thread(void *ptr)
|
||||
//apc_target.enable();
|
||||
for (int i = 0; i < 10 && !service_should_exit; i++)
|
||||
{
|
||||
apc_target.process_apc_requests();
|
||||
apc_target.process_apc_requests(true);
|
||||
my_sleep(int_rand(30));
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user