More DBUG statements

Replaced COND_refresh with COND_global_read_lock becasue of a bug in NTPL threads when using different mutexes as arguments to pthread_cond_wait()
The original code caused a hang in FLUSH TABLES WITH READ LOCK in some circumstances because pthread_cond_broadcast() was not delivered to other threads.
This fixes:
Bug#16986: Deadlock condition with MyISAM tables
Bug#20048: FLUSH TABLES WITH READ LOCK causes a deadlock


mysql-test/r/flush.result:
  Added test case for deadlock with FLUSH TABLES WITH READ LOCK
mysql-test/r/lock_multi.result:
  Test for bug in LOCK TABLE + optimize table
mysql-test/t/flush.test:
  Added test case for deadlock with FLUSH TABLES WITH READ LOCK
mysql-test/t/lock_multi.test:
  Test for bug in LOCK TABLE + optimize table
sql/lock.cc:
  Replaced COND_refresh with COND_global_read_lock becasue of a bug in NTPL threads when using different mutexes as arguments to pthread_cond_wait()
  The original code caused a hang in FLUSH TABLES WITH READ LOCK in some circumstances because pthread_cond_broadcast() was not delivered to other threads
sql/mysql_priv.h:
  Added COND_global_read_lock
sql/mysqld.cc:
  Added COND_global_read_lock
sql/sql_base.cc:
  More DBUG statements
  Added a broadcast in remove_table_from_cache() to release any threads waiting in open

8 files changed, 142 insertions, 14 deletions

diff --git a/mysql-test/r/flush.result b/mysql-test/r/flush.result
index 16c308e3450..a7f5e5e8fec 100644
--- a/mysql-test/r/flush.result
+++ b/mysql-test/r/flush.result
@@ -48,3 +48,10 @@ lock table t1 read, t2 read, t3 read;
 flush tables with read lock;
 unlock tables;
 drop table t1, t2, t3;
+create table t1 (c1 int);
+create table t2 (c1 int);
+lock table t1 write;
+ flush tables with read lock;
+ insert into t2 values(1);
+unlock tables;
+drop table t1, t2;
diff --git a/mysql-test/r/lock_multi.result b/mysql-test/r/lock_multi.result
index 73e3a9d32e3..89428ed6a9b 100644
--- a/mysql-test/r/lock_multi.result
+++ b/mysql-test/r/lock_multi.result
@@ -43,3 +43,19 @@ Field	Type	Null	Key	Default	Extra
 a	int(11)	YES		NULL	
 unlock tables;
 drop table t1;
+use mysql;
+LOCK TABLES columns_priv WRITE, db WRITE, host WRITE, user WRITE;
+FLUSH TABLES;
+use mysql;
+ SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1;
+OPTIMIZE TABLES columns_priv, db, host, user;
+Table	Op	Msg_type	Msg_text
+mysql.columns_priv	optimize	status	OK
+mysql.db	optimize	status	OK
+mysql.host	optimize	status	OK
+mysql.user	optimize	status	OK
+UNLOCK TABLES;
+Select_priv
+N
+use test;
+use test;
diff --git a/mysql-test/t/flush.test b/mysql-test/t/flush.test
index f5fd9fcadf2..95ba633fefd 100644
--- a/mysql-test/t/flush.test
+++ b/mysql-test/t/flush.test
@@ -102,3 +102,43 @@ unlock tables;
 drop table t1, t2, t3;
 
 # End of 4.1 tests
+
+#
+# Test of deadlock problem when doing FLUSH TABLE with read lock
+# (Bug was in NTPL threads in Linux when using different mutex while
+#  waiting for a condtion variable)
+
+create table t1 (c1 int);
+create table t2 (c1 int);
+
+connect (con1,localhost,root,,);
+connect (con3,localhost,root,,);
+
+connection con1;
+lock table t1 write;
+
+connection con2;
+send flush tables with read lock;
+--sleep 1
+
+connection con3;
+send insert into t2 values(1);
+--sleep 1
+
+connection con1;
+unlock tables;
+disconnect con1;
+
+connection con2;
+reap;
+disconnect con2;
+
+connection con3;
+# It hangs here (insert into t2 does not end).
+reap;
+disconnect con3;
+
+connection default;
+drop table t1, t2;
+
+# End of 5.0 tests
diff --git a/mysql-test/t/lock_multi.test b/mysql-test/t/lock_multi.test
index 0d2266fc2ae..3c829848bf3 100644
--- a/mysql-test/t/lock_multi.test
+++ b/mysql-test/t/lock_multi.test
@@ -107,3 +107,35 @@ show columns from t1;
 connection locker;
 unlock tables;
 drop table t1;
+
+#
+# Bug#16986 - Deadlock condition with MyISAM tables
+#
+connection locker;
+use mysql;
+LOCK TABLES columns_priv WRITE, db WRITE, host WRITE, user WRITE;
+FLUSH TABLES;
+--sleep 1
+#
+connection reader;
+use mysql;
+#NOTE:  This must be a multi-table select, otherwise the deadlock will not occur
+send SELECT user.Select_priv FROM user, db WHERE user.user = db.user LIMIT 1;
+--sleep 1
+#
+connection locker;
+# Make test case independent from earlier grants.
+--replace_result "Table is already up to date" "OK"
+OPTIMIZE TABLES columns_priv, db, host, user;
+UNLOCK TABLES;
+#
+connection reader;
+reap;
+use test;
+#
+connection locker;
+use test;
+#
+connection default;
+
+# End of 5.0 tests
diff --git a/sql/lock.cc b/sql/lock.cc
index 8ea13b2117c..71384fe7fc6 100644
--- a/sql/lock.cc
+++ b/sql/lock.cc
@@ -1138,16 +1138,17 @@ bool lock_global_read_lock(THD *thd)
 
   if (!thd->global_read_lock)
   {
+    const char *old_message;
     (void) pthread_mutex_lock(&LOCK_global_read_lock);
-    const char *old_message=thd->enter_cond(&COND_refresh, &LOCK_global_read_lock,
-					    "Waiting to get readlock");
+    old_message=thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock,
+                                "Waiting to get readlock");
     DBUG_PRINT("info",
 	       ("waiting_for: %d  protect_against: %d",
 		waiting_for_read_lock, protect_against_global_read_lock));
 
     waiting_for_read_lock++;
     while (protect_against_global_read_lock && !thd->killed)
-      pthread_cond_wait(&COND_refresh, &LOCK_global_read_lock);
+      pthread_cond_wait(&COND_global_read_lock, &LOCK_global_read_lock);
     waiting_for_read_lock--;
     if (thd->killed)
     {
@@ -1169,9 +1170,15 @@ bool lock_global_read_lock(THD *thd)
   DBUG_RETURN(0);
 }
 
+
 void unlock_global_read_lock(THD *thd)
 {
   uint tmp;
+  DBUG_ENTER("unlock_global_read_lock");
+  DBUG_PRINT("info",
+             ("global_read_lock: %u  global_read_lock_blocks_commit: %u",
+              global_read_lock, global_read_lock_blocks_commit));
+
   pthread_mutex_lock(&LOCK_global_read_lock);
   tmp= --global_read_lock;
   if (thd->global_read_lock == MADE_GLOBAL_READ_LOCK_BLOCK_COMMIT)
@@ -1179,8 +1186,13 @@ void unlock_global_read_lock(THD *thd)
   pthread_mutex_unlock(&LOCK_global_read_lock);
   /* Send the signal outside the mutex to avoid a context switch */
   if (!tmp)
-    pthread_cond_broadcast(&COND_refresh);
+  {
+    DBUG_PRINT("signal", ("Broadcasting COND_global_read_lock"));
+    pthread_cond_broadcast(&COND_global_read_lock);
+  }
   thd->global_read_lock= 0;
+
+  DBUG_VOID_RETURN;
 }
 
 #define must_wait (global_read_lock &&                             \
@@ -1218,11 +1230,15 @@ bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh,
       */
       DBUG_RETURN(is_not_commit);
     }
-    old_message=thd->enter_cond(&COND_refresh, &LOCK_global_read_lock,
+    old_message=thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock,
 				"Waiting for release of readlock");
     while (must_wait && ! thd->killed &&
 	   (!abort_on_refresh || thd->version == refresh_version))
-      (void) pthread_cond_wait(&COND_refresh,&LOCK_global_read_lock);
+    {
+      DBUG_PRINT("signal", ("Waiting for COND_global_read_lock"));
+      (void) pthread_cond_wait(&COND_global_read_lock, &LOCK_global_read_lock);
+      DBUG_PRINT("signal", ("Got COND_global_read_lock"));
+    }
     if (thd->killed)
       result=1;
   }
@@ -1251,7 +1267,7 @@ void start_waiting_global_read_lock(THD *thd)
         (waiting_for_read_lock || global_read_lock_blocks_commit));
   (void) pthread_mutex_unlock(&LOCK_global_read_lock);
   if (tmp)
-    pthread_cond_broadcast(&COND_refresh);
+    pthread_cond_broadcast(&COND_global_read_lock);
   DBUG_VOID_RETURN;
 }
 
@@ -1273,10 +1289,10 @@ bool make_global_read_lock_block_commit(THD *thd)
   /* For testing we set up some blocking, to see if we can be killed */
   DBUG_EXECUTE_IF("make_global_read_lock_block_commit_loop",
                   protect_against_global_read_lock++;);
-  old_message= thd->enter_cond(&COND_refresh, &LOCK_global_read_lock,
+  old_message= thd->enter_cond(&COND_global_read_lock, &LOCK_global_read_lock,
                                "Waiting for all running commits to finish");
   while (protect_against_global_read_lock && !thd->killed)
-    pthread_cond_wait(&COND_refresh, &LOCK_global_read_lock);
+    pthread_cond_wait(&COND_global_read_lock, &LOCK_global_read_lock);
   DBUG_EXECUTE_IF("make_global_read_lock_block_commit_loop",
                   protect_against_global_read_lock--;);
   if ((error= test(thd->killed)))
diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h
index d1781c0ebbf..f300c72c4ac 100644
--- a/sql/mysql_priv.h
+++ b/sql/mysql_priv.h
@@ -1220,6 +1220,7 @@ extern pthread_mutex_t LOCK_des_key_file;
 #endif
 extern rw_lock_t LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave;
 extern pthread_cond_t COND_refresh, COND_thread_count, COND_manager;
+extern pthread_cond_t COND_global_read_lock;
 extern pthread_attr_t connection_attrib;
 extern I_List<THD> threads;
 extern I_List<NAMED_LIST> key_caches;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 50f19c15fc4..2dcef245a99 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -516,7 +516,7 @@ pthread_mutex_t LOCK_prepared_stmt_count;
 pthread_mutex_t LOCK_des_key_file;
 #endif
 rw_lock_t	LOCK_grant, LOCK_sys_init_connect, LOCK_sys_init_slave;
-pthread_cond_t COND_refresh,COND_thread_count;
+pthread_cond_t COND_refresh,COND_thread_count, COND_global_read_lock;
 pthread_t signal_thread;
 pthread_attr_t connection_attrib;
 
@@ -1235,6 +1235,7 @@ static void clean_up_mutexes()
   (void) pthread_mutex_destroy(&LOCK_prepared_stmt_count);
   (void) pthread_cond_destroy(&COND_thread_count);
   (void) pthread_cond_destroy(&COND_refresh);
+  (void) pthread_cond_destroy(&COND_global_read_lock);
   (void) pthread_cond_destroy(&COND_thread_cache);
   (void) pthread_cond_destroy(&COND_flush_thread_cache);
   (void) pthread_cond_destroy(&COND_manager);
@@ -1657,13 +1658,11 @@ void end_thread(THD *thd, bool put_in_cache)
     }
   }
 
-  DBUG_PRINT("info", ("sending a broadcast"))
-
   /* Tell main we are ready */
   (void) pthread_mutex_unlock(&LOCK_thread_count);
   /* It's safe to broadcast outside a lock (COND... is not deleted here) */
+  DBUG_PRINT("signal", ("Broadcasting COND_thread_count"));
   (void) pthread_cond_broadcast(&COND_thread_count);
-  DBUG_PRINT("info", ("unlocked thread_count mutex"))
 #ifdef ONE_THREAD
   if (!(test_flags & TEST_NO_THREADS))	// For debugging under Linux
 #endif
@@ -2811,6 +2810,7 @@ static int init_thread_environment()
   (void) my_rwlock_init(&LOCK_grant, NULL);
   (void) pthread_cond_init(&COND_thread_count,NULL);
   (void) pthread_cond_init(&COND_refresh,NULL);
+  (void) pthread_cond_init(&COND_global_read_lock,NULL);
   (void) pthread_cond_init(&COND_thread_cache,NULL);
   (void) pthread_cond_init(&COND_flush_thread_cache,NULL);
   (void) pthread_cond_init(&COND_manager,NULL);
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 1f3b9e14631..9bc4ac8dc83 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -316,7 +316,7 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
     bool found=1;
     /* Wait until all threads has closed all the tables we had locked */
     DBUG_PRINT("info",
-	       ("Waiting for others threads to close their open tables"));
+	       ("Waiting for other threads to close their open tables"));
     while (found && ! thd->killed)
     {
       found=0;
@@ -326,6 +326,7 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
 	if ((table->s->version) < refresh_version && table->db_stat)
 	{
 	  found=1;
+          DBUG_PRINT("signal", ("Waiting for COND_refresh"));
 	  pthread_cond_wait(&COND_refresh,&LOCK_open);
 	  break;
 	}
@@ -1046,6 +1047,7 @@ TABLE *unlink_open_table(THD *thd, TABLE *list, TABLE *find)
 
 void wait_for_refresh(THD *thd)
 {
+  DBUG_ENTER("wait_for_refresh");
   safe_mutex_assert_owner(&LOCK_open);
 
   /* Wait until the current table is up to date */
@@ -1063,6 +1065,7 @@ void wait_for_refresh(THD *thd)
   thd->mysys_var->current_cond= 0;
   thd->proc_info= proc_info;
   pthread_mutex_unlock(&thd->mysys_var->mutex);
+  DBUG_VOID_RETURN;
 }
 
 
@@ -1346,6 +1349,9 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
   {
     if (table->s->version != refresh_version)
     {
+      DBUG_PRINT("note",
+                 ("Found table '%s.%s' with different refresh version",
+                  table_list->db, table_list->table_name));
       if (flags & MYSQL_LOCK_IGNORE_FLUSH)
       {
         /* Force close at once after usage */
@@ -5123,6 +5129,7 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
   TABLE *table;
   bool result=0, signalled= 0;
   DBUG_ENTER("remove_table_from_cache");
+  DBUG_PRINT("enter", ("Table: '%s.%s'  flags: %u", db, table_name, flags));
 
   key_length=(uint) (strmov(strmov(key,db)+1,table_name)-key)+1;
   for (;;)
@@ -5147,7 +5154,10 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
       {
         in_use->some_tables_deleted=1;
         if (table->db_stat)
+        {
+          DBUG_PRINT("info", ("Found another active instance of the table"));
   	  result=1;
+        }
         /* Kill delayed insert threads */
         if ((in_use->system_thread & SYSTEM_THREAD_DELAYED_INSERT) &&
             ! in_use->killed)
@@ -5182,6 +5192,12 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
       VOID(hash_delete(&open_cache,(byte*) unused_tables));
     if (result && (flags & RTFC_WAIT_OTHER_THREAD_FLAG))
     {
+      /*
+        Signal any thread waiting for tables to be freed to
+        reopen their tables
+      */
+      (void) pthread_cond_broadcast(&COND_refresh);
+      DBUG_PRINT("info", ("Waiting for refresh signal"));
       if (!(flags & RTFC_CHECK_KILLED_FLAG) || !thd->killed)
       {
         dropping_tables++;