MDEV-33150 double-locking of LOCK_thd_kill in performance_schema.session_status

perfschema thread walker needs to take thread's LOCK_thd_kill to prevent
the thread from disappearing why it's being looked at.
But there's no need to lock it for the current thread.

In fact, it was harmful as some code down the stack might take
LOCK_thd_kill (e.g. set_killed() does it, and my_malloc_size_cb_func()
calls set_killed()). And it caused a bunch of mutexes being locked under
LOCK_thd_kill, which created problems later when my_malloc_size_cb_func()
called set_killed() at some unspecified point under some
random mutexes.

mysql-test/suite/perfschema/r/misc_session_status.result

+#
+# MDEV-33150 double-locking of LOCK_thd_kill in performance_schema.session_status
+#
+set @old_innodb_io_capacity=@@global.innodb_io_capacity;
+set @old_innodb_io_capacity_max=@@global.innodb_io_capacity_max;
+select * from performance_schema.session_status limit 0;
+VARIABLE_NAME	VARIABLE_VALUE
+set max_session_mem_used=32768;
+select * from performance_schema.session_status;
+ERROR HY000: The MariaDB server is running with the --max-session-mem-used=32768 option so it cannot execute this statement
+set global innodb_io_capacity_max=100;
+Warnings:
+Warning	1210	Setting innodb_io_capacity_max 100 lower than innodb_io_capacity 200.
+Warning	1210	Setting innodb_io_capacity to 100
+set max_session_mem_used=default;
+set global innodb_io_capacity=@old_innodb_io_capacity;
+Warnings:
+Warning	1210	Setting innodb_io_capacity to 200 higher than innodb_io_capacity_max 100
+Warning	1210	Setting innodb_max_io_capacity to 400
+set global innodb_io_capacity_max=@old_innodb_io_capacity_max;

mysql-test/suite/perfschema/t/misc_session_status.test

+--source include/not_embedded.inc
+--source include/have_perfschema.inc
+--echo #
+--echo # MDEV-33150 double-locking of LOCK_thd_kill in performance_schema.session_status
+--echo #
+source include/have_innodb.inc;
+set @old_innodb_io_capacity=@@global.innodb_io_capacity;
+set @old_innodb_io_capacity_max=@@global.innodb_io_capacity_max;
+select * from performance_schema.session_status limit 0; # discover the table
+set max_session_mem_used=32768;
+--error ER_OPTION_PREVENTS_STATEMENT
+# this used to crash, when OOM happened under LOCK_thd_kill
+select * from performance_schema.session_status;
+# this used to cause mutex lock order violation when OOM happened under LOCK_global_system_variables
+set global innodb_io_capacity_max=100;
+set max_session_mem_used=default;
+set global innodb_io_capacity=@old_innodb_io_capacity;
+set global innodb_io_capacity_max=@old_innodb_io_capacity_max;

storage/perfschema/pfs_variable.cc

@@ -254,7 +254,8 @@ int PFS_system_variable_cache::do_materialize_all(THD *unsafe_thd)
     }
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -354,7 +355,8 @@ int PFS_system_variable_cache::do_materialize_session(PFS_thread *pfs_thread)
     }
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -407,7 +409,8 @@ int PFS_system_variable_cache::do_materialize_session(PFS_thread *pfs_thread, ui
     }
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -458,7 +461,8 @@ int PFS_system_variable_cache::do_materialize_session(THD *unsafe_thd)
     }
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -990,7 +994,8 @@ int PFS_status_variable_cache::do_materialize_all(THD* unsafe_thd)
     manifest(m_safe_thd, m_show_var_array.front(), status_vars, "", false, false);
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -1036,7 +1041,8 @@ int PFS_status_variable_cache::do_materialize_session(THD* unsafe_thd)
     manifest(m_safe_thd, m_show_var_array.front(), status_vars, "", false, true);
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;
@@ -1078,7 +1084,8 @@ int PFS_status_variable_cache::do_materialize_session(PFS_thread *pfs_thread)
     manifest(m_safe_thd, m_show_var_array.front(), status_vars, "", false, true);
 
     /* Release lock taken in get_THD(). */
-    mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
+    if (m_safe_thd != current_thd)
+      mysql_mutex_unlock(&m_safe_thd->LOCK_thd_kill);
 
     m_materialized= true;
     ret= 0;

storage/perfschema/pfs_variable.h

@@ -211,8 +211,12 @@ class Find_THD_variable : public Find_THD_Impl
     if (thd != m_unsafe_thd)
       return false;
 
-    /* Hold this lock to keep THD during materialization. */
-    mysql_mutex_lock(&thd->LOCK_thd_kill);
+    /*
+      Hold this lock to keep THD during materialization.
+      But don't lock current_thd (to be able to use set_killed() later
+    */
+    if (thd != current_thd)
+      mysql_mutex_lock(&thd->LOCK_thd_kill);
     return true;
   }
   void set_unsafe_thd(THD *unsafe_thd) { m_unsafe_thd= unsafe_thd; }