Bug #38883 (reopened): thd_security_context is not thread safe, crashes?

The bug 38816 changed the lock that protects THD::query from
LOCK_thread_count to LOCK_thd_data, but didn't update the associated
InnoDB functions.

1. The innobase_mysql_prepare_print_arbitrary_thd and the
innobase_mysql_end_print_arbitrary_thd InnoDB functions have been
removed, since now we have a per-thread mutex: now we don't need to wrap
several inter-thread access tries to THD::query with a single global
LOCK_thread_count lock, so we can simplify the code.

2. The innobase_mysql_print_thd function has been modified to lock
LOCK_thd_data in direct way.

4 files changed, 10 insertions, 70 deletions

diff --git a/innobase/include/trx0trx.h b/innobase/include/trx0trx.h
index cf9d1788ad8..5ed9f0b8c97 100644
--- a/innobase/include/trx0trx.h
+++ b/innobase/include/trx0trx.h
@@ -348,9 +348,7 @@ trx_commit_step(
 
 /**************************************************************************
 Prints info about a transaction to the given file. The caller must own the
-kernel mutex and must have called
-innobase_mysql_prepare_print_arbitrary_thd(), unless he knows that MySQL
-or InnoDB cannot meanwhile change the info printed here. */
+kernel mutex. */
 
 void
 trx_print(
diff --git a/innobase/lock/lock0lock.c b/innobase/lock/lock0lock.c
index e0f3f58f737..acf76a08f16 100644
--- a/innobase/lock/lock0lock.c
+++ b/innobase/lock/lock0lock.c
@@ -18,31 +18,6 @@ Created 5/7/1996 Heikki Tuuri
 #include "trx0sys.h"
 
 
-/* 2 function prototypes copied from ha_innodb.cc: */
-
-/*****************************************************************
-If you want to print a thd that is not associated with the current thread,
-you must call this function before reserving the InnoDB kernel_mutex, to
-protect MySQL from setting thd->query NULL. If you print a thd of the current
-thread, we know that MySQL cannot modify thd->query, and it is not necessary
-to call this. Call innobase_mysql_end_print_arbitrary_thd() after you release
-the kernel_mutex.
-NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
-function! */
-
-void
-innobase_mysql_prepare_print_arbitrary_thd(void);
-/*============================================*/
-
-/*****************************************************************
-Relases the mutex reserved by innobase_mysql_prepare_print_arbitrary_thd().
-NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
-function! */
-
-void
-innobase_mysql_end_print_arbitrary_thd(void);
-/*========================================*/
-
 /* Restricts the length of search we will do in the waits-for
 graph of transactions */
 #define LOCK_MAX_N_STEPS_IN_DEADLOCK_CHECK 1000000
@@ -4231,11 +4206,6 @@ lock_print_info_summary(
 /*====================*/
 	FILE*	file)	/* in: file where to print */
 {
-	/* We must protect the MySQL thd->query field with a MySQL mutex, and
-	because the MySQL mutex must be reserved before the kernel_mutex of
-	InnoDB, we call innobase_mysql_prepare_print_arbitrary_thd() here. */
-
-	innobase_mysql_prepare_print_arbitrary_thd();
 	lock_mutex_enter_kernel();
 
 	if (lock_deadlock_found) {
@@ -4322,7 +4292,6 @@ loop:
 
 	if (trx == NULL) {
 		lock_mutex_exit_kernel();
-		innobase_mysql_end_print_arbitrary_thd();
 
 		ut_ad(lock_validate());
 
@@ -4387,7 +4356,6 @@ loop:
 
  		if (load_page_first) {
 			lock_mutex_exit_kernel();
-			innobase_mysql_end_print_arbitrary_thd();
 
 			mtr_start(&mtr);
 			
@@ -4397,7 +4365,6 @@ loop:
 
 			load_page_first = FALSE;
 
-			innobase_mysql_prepare_print_arbitrary_thd();
 			lock_mutex_enter_kernel();
 
 			goto loop;
diff --git a/innobase/trx/trx0trx.c b/innobase/trx/trx0trx.c
index 70fd73f2488..d0e64b84eeb 100644
--- a/innobase/trx/trx0trx.c
+++ b/innobase/trx/trx0trx.c
@@ -1701,9 +1701,7 @@ trx_mark_sql_stat_end(
 
 /**************************************************************************
 Prints info about a transaction to the given file. The caller must own the
-kernel mutex and must have called
-innobase_mysql_prepare_print_arbitrary_thd(), unless he knows that MySQL
-or InnoDB cannot meanwhile change the info printed here. */
+kernel mutex. */
 
 void
 trx_print(
diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index 6586bf818c1..cf6827585f7 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -555,35 +555,6 @@ convert_error_code_to_mysql(
 }
 
 /*****************************************************************
-If you want to print a thd that is not associated with the current thread,
-you must call this function before reserving the InnoDB kernel_mutex, to
-protect MySQL from setting thd->query NULL. If you print a thd of the current
-thread, we know that MySQL cannot modify thd->query, and it is not necessary
-to call this. Call innobase_mysql_end_print_arbitrary_thd() after you release
-the kernel_mutex.
-NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
-function! */
-extern "C"
-void
-innobase_mysql_prepare_print_arbitrary_thd(void)
-/*============================================*/
-{
-	VOID(pthread_mutex_lock(&LOCK_thread_count));
-}
-
-/*****************************************************************
-Releases the mutex reserved by innobase_mysql_prepare_print_arbitrary_thd().
-NOTE that /mysql/innobase/lock/lock0lock.c must contain the prototype for this
-function! */
-extern "C"
-void
-innobase_mysql_end_print_arbitrary_thd(void)
-/*========================================*/
-{
-	VOID(pthread_mutex_unlock(&LOCK_thread_count));
-}
-
-/*****************************************************************
 Prints info of a THD object (== user session thread) to the given file.
 NOTE that /mysql/innobase/trx/trx0trx.c must contain the prototype for
 this function! */
@@ -596,11 +567,11 @@ innobase_mysql_print_thd(
 	uint	max_query_len)	/* in: max query length to print, or 0 to
 				   use the default max length */
 {
-	const THD*	thd;
+	THD*        	thd;
         const Security_context *sctx;
 	const char*	s;
 
-        thd = (const THD*) input_thd;
+        thd = (THD*) input_thd;
         /* We probably want to have original user as part of debug output. */
         sctx = &thd->main_security_ctx;
 
@@ -627,6 +598,10 @@ innobase_mysql_print_thd(
 		fputs(s, f);
 	}
 
+        /* We have to quarantine an access to thd->query and
+           thd->query_length with thd->LOCK_thd_data mutex. */
+	VOID(pthread_mutex_lock(&thd->LOCK_thd_data));
+
 	if ((s = thd->query)) {
 		/* 3100 is chosen because currently 3000 is the maximum
 		   max_query_len we ever give this. */
@@ -671,6 +646,8 @@ innobase_mysql_print_thd(
 		}
 	}
 
+	VOID(pthread_mutex_unlock(&thd->LOCK_thd_data));
+
 	putc('\n', f);
 }