Many files:

  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query


sql/sql_class.h:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/ha_innodb.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/log_event.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/slave.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/sql_db.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/sql_parse.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
sql/sql_show.cc:
  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query

7 files changed, 33 insertions, 5 deletions

diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index 194c8b558f0..4835f794a1d 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -390,15 +390,16 @@ innobase_mysql_print_thd(
 			len = 300;	/* ADDITIONAL SAFETY: print at most
 					300 chars to reduce the probability of
 					a seg fault if there is a race in
-					thd->query_len in MySQL; on May 13,
-					2004 we do not know */
+					thd->query_length in MySQL; after
+					May 14, 2004 probably no race any more,
+					but better be safe */
 		}
 		
 		for (i = 0; i < len && s[i]; i++);
 
 		memcpy(buf, s, i);	/* Use memcpy to reduce the timeframe
 					for a race, compared to fwrite() */
-		buf[300] = '\0';
+		buf[300] = '\0';	/* not needed, just extra safety */
 
 		putc('\n', f);
 		fwrite(buf, 1, i, f);
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 7817ccff3d7..f84c8d1f579 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -1929,6 +1929,7 @@ end:
   VOID(pthread_mutex_lock(&LOCK_thread_count));
   thd->db= 0;	                        // prevent db from being freed
   thd->query= 0;			// just to be sure
+  thd->query_length= 0;
   VOID(pthread_mutex_unlock(&LOCK_thread_count));
   // assume no convert for next query unless set explictly
   thd->variables.convert_set = 0;
diff --git a/sql/slave.cc b/sql/slave.cc
index d6d0a5b5425..c7a7dac141a 100644
--- a/sql/slave.cc
+++ b/sql/slave.cc
@@ -2691,6 +2691,7 @@ err:
 		  IO_RPL_LOG_NAME, llstr(mi->master_log_pos,llbuff));
   VOID(pthread_mutex_lock(&LOCK_thread_count));
   thd->query = thd->db = 0; // extra safety
+  thd->query_length = 0;
   VOID(pthread_mutex_unlock(&LOCK_thread_count));
   if (mysql)
   {
@@ -2839,6 +2840,7 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \
  err:
   VOID(pthread_mutex_lock(&LOCK_thread_count));
   thd->query = thd->db = 0; // extra safety
+  thd->query_length = 0;
   VOID(pthread_mutex_unlock(&LOCK_thread_count));
   thd->proc_info = "Waiting for slave mutex on exit";
   pthread_mutex_lock(&rli->run_lock);
diff --git a/sql/sql_class.h b/sql/sql_class.h
index ad1eed5448f..83dc75e2b84 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -360,7 +360,24 @@ public:
   struct  rand_struct rand;		// used for authentication
   struct  system_variables variables;	// Changeable local variables
   pthread_mutex_t LOCK_delete;		// Locked before thd is deleted
-
+  /* 
+    Note that (A) if we set query = NULL, we must at the same time set
+    query_length = 0, and protect the whole operation with the
+    LOCK_thread_count mutex. And (B) we are ONLY allowed to set query to a
+    non-NULL value if its previous value is NULL. We do not need to protect
+    operation (B) with any mutex. To avoid crashes in races, if we do not
+    know that thd->query cannot change at the moment, one should print
+    thd->query like this:
+      (1) reserve the LOCK_thread_count mutex;
+      (2) check if thd->query is NULL;
+      (3) if not NULL, then print at most thd->query_length characters from
+      it. We will see the query_length field as either 0, or the right value
+      for it.
+    Assuming that the write and read of an n-bit memory field in an n-bit
+    computer is atomic, we can avoid races in the above way. 
+    This printing is needed at least in SHOW PROCESSLIST and SHOW INNODB
+    STATUS.
+  */
   char	  *query;			// Points to the current query,
   /*
     A pointer to the stack frame of handle_one_connection(),
diff --git a/sql/sql_db.cc b/sql/sql_db.cc
index 7dd458a3b5d..865b2e1328f 100644
--- a/sql/sql_db.cc
+++ b/sql/sql_db.cc
@@ -95,6 +95,7 @@ int mysql_create_db(THD *thd, char *db, uint create_options, bool silent)
     {
       VOID(pthread_mutex_lock(&LOCK_thread_count));
       thd->query= 0;
+      thd->query_length= 0;
       VOID(pthread_mutex_unlock(&LOCK_thread_count));
     }
     send_ok(&thd->net, result);
@@ -202,6 +203,7 @@ int mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
       {
 	VOID(pthread_mutex_lock(&LOCK_thread_count));
 	thd->query= 0;
+	thd->query_length= 0;
 	VOID(pthread_mutex_unlock(&LOCK_thread_count));
       }
       send_ok(&thd->net,(ulong) deleted);
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index be2d81fa372..77032bef698 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -1312,6 +1312,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
   thd->proc_info=0;
   thd->command=COM_SLEEP;
   thd->query=0;
+  thd->query_length=0;
   thread_running--;
   VOID(pthread_mutex_unlock(&LOCK_thread_count));
   thd->packet.shrink(thd->variables.net_buffer_length);	// Reclaim some memory
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index 63f3afab128..7f52e52c849 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -1141,7 +1141,11 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose)
         thd_info->query=0;
         if (tmp->query)
         {
-	  /* query_length is always set before tmp->query */
+	  /* 
+            query_length is always set to 0 when we set query = NULL; see
+	    the comment in sql_class.h why this prevents crashes in possible
+            races with query_length
+          */
           uint length= min(max_query_length, tmp->query_length);
           thd_info->query=(char*) thd->memdup(tmp->query,length+1);
           thd_info->query[length]=0;