Skip to content

Commit

Permalink
MDEV-33443: Unsafe use of LOCK_thd_kill in my_malloc_size_cb_func()
Browse files Browse the repository at this point in the history
my_malloc_size_cb_func() can be called from contexts where it is not safe to
wait for LOCK_thd_kill, for example while holding LOCK_plugin. This could
lead to (probably very unlikely) deadlock of the server.

Fix by skipping the enforcement of --max-session-mem-used in the rare cases
when LOCK_thd_kill cannot be obtained. The limit will instead be enforced on
the following memory allocation. This does not significantly degrade the
behaviour of --max-session-mem-used; that limit is in any case only enforced
"softly", not taking effect until the next point at which the thread does a
check_killed().

Reviewed-by: Monty <monty@mariadb.org>
Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org>
  • Loading branch information
knielsen committed Feb 16, 2024
1 parent c73c6ae commit fdaa7a9
Showing 1 changed file with 29 additions and 14 deletions.
43 changes: 29 additions & 14 deletions sql/mysqld.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3789,20 +3789,35 @@ static void my_malloc_size_cb_func(long long size, my_bool is_thread_specific)
thd->status_var.local_memory_used > (int64)thd->variables.max_mem_used &&
likely(!thd->killed) && !thd->get_stmt_da()->is_set())
{
/* Ensure we don't get called here again */
char buf[50], *buf2;
thd->set_killed(KILL_QUERY);
my_snprintf(buf, sizeof(buf), "--max-session-mem-used=%llu",
thd->variables.max_mem_used);
if ((buf2= (char*) thd->alloc(256)))
{
my_snprintf(buf2, 256, ER_THD(thd, ER_OPTION_PREVENTS_STATEMENT), buf);
thd->set_killed(KILL_QUERY, ER_OPTION_PREVENTS_STATEMENT, buf2);
}
else
{
thd->set_killed(KILL_QUERY, ER_OPTION_PREVENTS_STATEMENT,
"--max-session-mem-used");
/*
Ensure we don't get called here again.
It is not safe to wait for LOCK_thd_kill here, as we could be called
from almost any context. For example while LOCK_plugin is being held;
but THD::awake() locks LOCK_thd_kill and LOCK_plugin in the opposite
order (MDEV-33443).
So ignore the max_mem_used limit in the unlikely case we cannot obtain
LOCK_thd_kill here (the limit will be enforced on the next allocation).
*/
if (!mysql_mutex_trylock(&thd->LOCK_thd_kill)) {
char buf[50], *buf2;
thd->set_killed_no_mutex(KILL_QUERY);
my_snprintf(buf, sizeof(buf), "--max-session-mem-used=%llu",
thd->variables.max_mem_used);
if ((buf2= (char*) thd->alloc(256)))
{
my_snprintf(buf2, 256,
ER_THD(thd, ER_OPTION_PREVENTS_STATEMENT), buf);
thd->set_killed_no_mutex(KILL_QUERY,
ER_OPTION_PREVENTS_STATEMENT, buf2);
}
else
{
thd->set_killed_no_mutex(KILL_QUERY, ER_OPTION_PREVENTS_STATEMENT,
"--max-session-mem-used");
}
mysql_mutex_unlock(&thd->LOCK_thd_kill);
}
}
DBUG_ASSERT((longlong) thd->status_var.local_memory_used >= 0 ||
Expand Down

0 comments on commit fdaa7a9

Please sign in to comment.