ha_innodb.cc, trx0trx.h, lock0lock.c, trx0trx.c:

Reserve the MySQL LOCK_thread_count mutex when printing thd->query of
an arbitrary transaction; if we are printing thd->query of a transaction that
we know is currently executing inside InnoDB, then we know that MySQL cannot
meanwhile change thd->query, and no need to reserve the MySQL mutex; note
that this patch still leaves open the possibility of races in MySQL's
thd->query_len


innobase/trx/trx0trx.c:
  Reserve the MySQL LOCK_thread_count mutex when printing thd->query of an arbitrary transaction; if we are printing thd->query of the a transaction that we know is currently executing inside InnoDB, then we know that MySQL cannot meanwhile change thd->query, and no need to reserve the MySQL mutex; note that thsi patch still leaves aopen the possibility of races in MySQL's thd->query_len
innobase/lock/lock0lock.c:
  Reserve the MySQL LOCK_thread_count mutex when printing thd->query of an arbitrary transaction; if we are printing thd->query of the a transaction that we know is currently executing inside InnoDB, then we know that MySQL cannot meanwhile change thd->query, and no need to reserve the MySQL mutex; note that thsi patch still leaves aopen the possibility of races in MySQL's thd->query_len
innobase/include/trx0trx.h:
  Reserve the MySQL LOCK_thread_count mutex when printing thd->query of an arbitrary transaction; if we are printing thd->query of the a transaction that we know is currently executing inside InnoDB, then we know that MySQL cannot meanwhile change thd->query, and no need to reserve the MySQL mutex; note that thsi patch still leaves aopen the possibility of races in MySQL's thd->query_len
sql/ha_innodb.cc:
  Reserve the MySQL LOCK_thread_count mutex when printing thd->query of an arbitrary transaction; if we are printing thd->query of the a transaction that we know is currently executing inside InnoDB, then we know that MySQL cannot meanwhile change thd->query, and no need to reserve the MySQL mutex; note that thsi patch still leaves aopen the possibility of races in MySQL's thd->query_len

4 files changed, 75 insertions, 17 deletions

diff --git a/innobase/include/trx0trx.h b/innobase/include/trx0trx.h
index 71269cb1e4e..07d5e5a8215 100644
--- a/innobase/include/trx0trx.h
+++ b/innobase/include/trx0trx.h
@@ -275,13 +275,15 @@ trx_commit_step(
 	que_thr_t*	thr);	/* in: query thread */
 /**************************************************************************
 Prints info about a transaction to the standard output. The caller must
-own the kernel mutex. */
+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. */
 
 void
 trx_print(
 /*======*/
 	FILE*	f,	/* in: output stream */
-	trx_t* trx); 	/* in: transaction */
+	trx_t*	trx);	/* in: transaction */
 
 
 /* Signal to a transaction */
diff --git a/innobase/lock/lock0lock.c b/innobase/lock/lock0lock.c
index 4343496f6e1..791b81366b2 100644
--- a/innobase/lock/lock0lock.c
+++ b/innobase/lock/lock0lock.c
@@ -17,6 +17,32 @@ Created 5/7/1996 Heikki Tuuri
 #include "dict0mem.h"
 #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
@@ -3974,6 +4000,11 @@ lock_print_info(
 	ulint	i;
 	mtr_t	mtr;
 
+	/* 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) {
@@ -4037,6 +4068,7 @@ loop:
 
 	if (trx == NULL) {
 		lock_mutex_exit_kernel();
+		innobase_mysql_end_print_arbitrary_thd();
 
 		ut_ad(lock_validate());
 
@@ -4101,6 +4133,7 @@ loop:
 
  		if (load_page_first) {
 			lock_mutex_exit_kernel();
+			innobase_mysql_end_print_arbitrary_thd();
 
 			mtr_start(&mtr);
 			
@@ -4110,6 +4143,7 @@ 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 69cd6c7b22d..335e1f69228 100644
--- a/innobase/trx/trx0trx.c
+++ b/innobase/trx/trx0trx.c
@@ -1562,7 +1562,9 @@ trx_mark_sql_stat_end(
 
 /**************************************************************************
 Prints info about a transaction to the standard output. The caller must
-own the kernel mutex. */
+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. */
 
 void
 trx_print(
diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index e330a0b532f..194c8b558f0 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -313,6 +313,35 @@ 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));
+}
+
+/*****************************************************************
+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! */
+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
 standard output. NOTE that /mysql/innobase/trx/trx0trx.c must contain
 the prototype for this function! */
@@ -329,15 +358,6 @@ innobase_mysql_print_thd(
 
         thd = (const THD*) input_thd;
 
-/* We cannot use LOCK_thread_count to protect this operation because we own
-the InnoDB kernel_mutex when we enter this function, but in freeing of a
-THD object, MySQL first reserves LOCK_thread_count and AFTER THAT InnoDB
-reserves kernel_mutex when freeing the trx object => a deadlock can occur.
-The solution is for MySQL to use a separate mutex to protect thd->query and
-thd->query_len. Someone should do that! This bug has been here for 3 years!
-
-	VOID(pthread_mutex_lock(&LOCK_thread_count)); */
-
   	fprintf(f, "MySQL thread id %lu, query id %lu",
 		thd->thread_id, thd->query_id);
 	if (thd->host) {
@@ -367,14 +387,16 @@ thd->query_len. Someone should do that! This bug has been here for 3 years!
 		len = thd->query_length;
 
 		if (len > 300) {
-			len = 300;	/* A TEMPORARY SOLUTION: print at most
+			len = 300;	/* ADDITIONAL SAFETY: print at most
 					300 chars to reduce the probability of
-					a seg fault in a race */
+					a seg fault if there is a race in
+					thd->query_len in MySQL; on May 13,
+					2004 we do not know */
 		}
 		
 		for (i = 0; i < len && s[i]; i++);
 
-		memcpy(buf, s, i);	/* use memcpy to reduce the timeframe
+		memcpy(buf, s, i);	/* Use memcpy to reduce the timeframe
 					for a race, compared to fwrite() */
 		buf[300] = '\0';
 
@@ -383,8 +405,6 @@ thd->query_len. Someone should do that! This bug has been here for 3 years!
 	}
 
 	putc('\n', f);
-
-/*	VOID(pthread_mutex_unlock(&LOCK_thread_count)); */
 }
 
 /*************************************************************************