[PATCH] MDEV-33443: Unsafe use of LOCK_thd_kill in my_malloc_size_cb_func()
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(). Signed-off-by: Kristian Nielsen <knielsen@knielsen-hq.org> --- sql/mysqld.cc | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 5713045056d..50b2698ccba 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -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 || -- 2.30.2
participants (1)
-
Kristian Nielsen