[MariaDB commits] [PATCH] MDEV-33443: Unsafe use of LOCK_thd_kill in my_malloc_size_cb_func()

[Kristian Nielsen via commits]

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 <kniel...@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

_______________________________________________
commits mailing list -- commits@lists.mariadb.org
To unsubscribe send an email to commits-le...@lists.mariadb.org