Skip to content

Commit 2a4c573

Browse files
knielsenvuvova
authored andcommitted
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>
1 parent fefd6d5 commit 2a4c573

File tree

5 files changed

+11
-8
lines changed

5 files changed

+11
-8
lines changed

sql/my_apc.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,13 +197,16 @@ bool Apc_target::make_apc_call(THD *caller_thd, Apc_call *call,
197197
This should be called periodically by the APC target thread.
198198
*/
199199

200-
void Apc_target::process_apc_requests()
200+
void Apc_target::process_apc_requests(bool force)
201201
{
202202
while (1)
203203
{
204204
Call_request *request;
205-
206-
mysql_mutex_lock(LOCK_thd_kill_ptr);
205+
206+
if (force)
207+
mysql_mutex_lock(LOCK_thd_kill_ptr);
208+
else if (mysql_mutex_trylock(LOCK_thd_kill_ptr))
209+
break; // Mutex is blocked, try again later
207210
if (!(request= get_first_in_queue()))
208211
{
209212
/* No requests in the queue */

sql/my_apc.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ class Apc_target
7070
bool process= !--enabled && have_apc_requests();
7171
mysql_mutex_unlock(LOCK_thd_kill_ptr);
7272
if (unlikely(process))
73-
process_apc_requests();
73+
process_apc_requests(true);
7474
}
7575

76-
void process_apc_requests();
76+
void process_apc_requests(bool force);
7777
/*
7878
A lightweight function, intended to be used in frequent checks like this:
7979

sql/sql_class.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4724,7 +4724,7 @@ extern "C" enum thd_kill_levels thd_kill_level(const MYSQL_THD thd)
47244724
if (unlikely(apc_target->have_apc_requests()))
47254725
{
47264726
if (thd == current_thd)
4727-
apc_target->process_apc_requests();
4727+
apc_target->process_apc_requests(false);
47284728
}
47294729
return THD_IS_NOT_KILLED;
47304730
}

sql/sql_class.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3606,7 +3606,7 @@ class THD: public THD_count, /* this must be first */
36063606
return TRUE;
36073607
}
36083608
if (apc_target.have_apc_requests())
3609-
apc_target.process_apc_requests();
3609+
apc_target.process_apc_requests(false);
36103610
return FALSE;
36113611
}
36123612

unittest/sql/my_apc-t.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void *test_apc_service_thread(void *ptr)
100100
//apc_target.enable();
101101
for (int i = 0; i < 10 && !service_should_exit; i++)
102102
{
103-
apc_target.process_apc_requests();
103+
apc_target.process_apc_requests(true);
104104
my_sleep(int_rand(30));
105105
}
106106
}

0 commit comments

Comments
 (0)