Bug#60682: deadlock from thd_security_context

The problem is a circular wait condition when providing information
about the internal state of InnoDB. When printing information about
active transactions, InnoDB holds the kernel mutex (so that transactions
do not disappear) and calls back into the server to get a description
(in particular, the query) of every session that is associated with
a transaction. Since these sessions might be executing unrelated
statements, a per-session lock is taken so that the query string can
be safely copied. This lock order poses a problem because there might
be cases, specially when processing a KILL statement, where the per-
session lock is acquired before attempting to acquire locks that might
also be acquired by InnoDB while holding the kernel mutex.

In order to avoid this problematic lock order, the query of a session
associated with an active transaction is no longer displayed in the
output of SHOW ENGINE INNODB STATUS. There query now is only printed
if a session is describing itself (that is, if the session to be
described belongs to the calling thread).

sql/sql_class.cc

@@ -627,11 +627,14 @@ void thd_inc_row_count(THD *thd)
   @param max_query_len how many chars of query to copy (0 for all)
 
   @req LOCK_thread_count
-  
+
   @note LOCK_thread_count mutex is not necessary when the function is invoked on
    the currently running thread (current_thd) or if the caller in some other
-   way guarantees that access to thd->query is serialized.
- 
+   way guarantees that the thread won't be going away.
+
+  @note The query string is only printed if the session (thd) to be
+        described belongs to the calling thread.
+
   @return Pointer to string
 */
 
@@ -644,13 +647,11 @@ char *thd_security_context(THD *thd, char *buffer, unsigned int length,
   char header[256];
   int len;
   /*
-    The pointers thd->query and thd->proc_info might change since they are
-    being modified concurrently. This is acceptable for proc_info since its
-    values doesn't have to very accurate and the memory it points to is static,
-    but we need to attempt a snapshot on the pointer values to avoid using NULL
-    values. The pointer to thd->query however, doesn't point to static memory
-    and has to be protected by LOCK_thread_count or risk pointing to
-    uninitialized memory.
+    The thd->proc_info pointer might change since it is being modified
+    concurrently. This is acceptable for proc_info since its value
+    doesn't have to be accurate and the memory it points to is static,
+    but we need to attempt a snapshot on the pointer values to avoid
+    using NULL values.
   */
   const char *proc_info= thd->proc_info;
 
@@ -684,9 +685,7 @@ char *thd_security_context(THD *thd, char *buffer, unsigned int length,
     str.append(proc_info);
   }
 
-  mysql_mutex_lock(&thd->LOCK_thd_data);
-
-  if (thd->query())
+  if (thd == current_thd && thd->query())
   {
     if (max_query_len < 1)
       len= thd->query_length();
@@ -696,8 +695,6 @@ char *thd_security_context(THD *thd, char *buffer, unsigned int length,
     str.append(thd->query(), len);
   }
 
-  mysql_mutex_unlock(&thd->LOCK_thd_data);
-
   if (str.c_ptr_safe() == buffer)
     return buffer;