Merge istruewing@bk-internal.mysql.com:/home/bk/mysql-5.1

into  mysql.com:/home/mydev/mysql-5.1-amerge

mysql-test/r/lock_multi.result

@@ -66,6 +66,21 @@ Select_priv
 N
 use test;
 use test;
+CREATE TABLE t1 (c1 int);
+LOCK TABLE t1 WRITE;
+ FLUSH TABLES WITH READ LOCK;
+CREATE TABLE t2 (c1 int);
+UNLOCK TABLES;
+UNLOCK TABLES;
+DROP TABLE t1, t2;
+CREATE TABLE t1 (c1 int);
+LOCK TABLE t1 WRITE;
+ FLUSH TABLES WITH READ LOCK;
+CREATE TABLE t2 AS SELECT * FROM t1;
+ERROR HY000: Table 't2' was not locked with LOCK TABLES
+UNLOCK TABLES;
+UNLOCK TABLES;
+DROP TABLE t1;
 CREATE DATABASE mysqltest_1;
 FLUSH TABLES WITH READ LOCK;
  DROP DATABASE mysqltest_1;
@@ -80,19 +95,3 @@ lock tables t1 write;
  alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; alter table t1 auto_increment=0; //
 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;

mysql-test/t/lock_multi.test

@@ -156,6 +156,55 @@ connection locker;
 use test;
 #
 connection default;
+#
+# Test if CREATE TABLE with LOCK TABLE deadlocks.
+#
+connection writer;
+CREATE TABLE t1 (c1 int);
+LOCK TABLE t1 WRITE;
+#
+# This waits until t1 is unlocked.
+connection locker;
+send FLUSH TABLES WITH READ LOCK;
+--sleep 1
+#
+# This must not block.
+connection writer;
+CREATE TABLE t2 (c1 int);
+UNLOCK TABLES;
+#
+# This awakes now.
+connection locker;
+reap;
+UNLOCK TABLES;
+#
+connection default;
+DROP TABLE t1, t2;
+#
+# Test if CREATE TABLE SELECT with LOCK TABLE deadlocks.
+#
+connection writer;
+CREATE TABLE t1 (c1 int);
+LOCK TABLE t1 WRITE;
+#
+# This waits until t1 is unlocked.
+connection locker;
+send FLUSH TABLES WITH READ LOCK;
+--sleep 1
+#
+# This must not block.
+connection writer;
+--error 1100
+CREATE TABLE t2 AS SELECT * FROM t1;
+UNLOCK TABLES;
+#
+# This awakes now.
+connection locker;
+reap;
+UNLOCK TABLES;
+#
+connection default;
+DROP TABLE t1;
 
 #
 # Bug#19815 - CREATE/RENAME/DROP DATABASE can deadlock on a global read lock
@@ -218,32 +267,4 @@ connection locker;
 drop table t1;
 
 # End of 5.0 tests
-# 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;
 

mysys/thr_lock.c

@@ -204,6 +204,8 @@ static void check_locks(THR_LOCK *lock, const char *where,
       {
 	if ((int) data->type == (int) TL_READ_NO_INSERT)
 	  count++;
+        /* Protect against infinite loop. */
+        DBUG_ASSERT(count <= lock->read_no_write_count);
       }
       if (count != lock->read_no_write_count)
       {

sql/lock.cc

@@ -935,7 +935,7 @@ void unlock_table_name(THD *thd, TABLE_LIST *table_list)
   if (table_list->table)
   {
     hash_delete(&open_cache, (byte*) table_list->table);
-    (void) pthread_cond_broadcast(&COND_refresh);
+    broadcast_refresh();
   }
 }
 
@@ -1037,9 +1037,9 @@ end:
 			(default 0, which will unlock all tables)
 
   NOTES
-    One must have a lock on LOCK_open when calling this
-    This function will send a COND_refresh signal to inform other threads
-    that the name locks are removed
+    One must have a lock on LOCK_open when calling this.
+    This function will broadcast refresh signals to inform other threads
+    that the name locks are removed.
 
   RETURN
     0	ok
@@ -1054,7 +1054,7 @@ void unlock_table_names(THD *thd, TABLE_LIST *table_list,
        table != last_table;
        table= table->next_local)
     unlock_table_name(thd,table);
-  pthread_cond_broadcast(&COND_refresh);
+  broadcast_refresh();
   DBUG_VOID_RETURN;
 }
 
@@ -1344,3 +1344,39 @@ bool make_global_read_lock_block_commit(THD *thd)
   thd->exit_cond(old_message); // this unlocks LOCK_global_read_lock
   DBUG_RETURN(error);
 }
+
+
+/*
+  Broadcast COND_refresh and COND_global_read_lock.
+
+  SYNOPSIS
+    broadcast_refresh()
+      void                      No parameters.
+
+  DESCRIPTION
+    Due to a bug in a threading library it could happen that a signal
+    did not reach its target. A condition for this was that the same
+    condition variable was used with different mutexes in
+    pthread_cond_wait(). Some time ago we changed LOCK_open to
+    LOCK_global_read_lock in global read lock handling. So COND_refresh
+    was used with LOCK_open and LOCK_global_read_lock.
+
+    We did now also change from COND_refresh to COND_global_read_lock
+    in global read lock handling. But now it is necessary to signal
+    both conditions at the same time.
+
+  NOTE
+    When signalling COND_global_read_lock within the global read lock
+    handling, it is not necessary to also signal COND_refresh.
+
+  RETURN
+    void
+*/
+
+void broadcast_refresh(void)
+{
+  VOID(pthread_cond_broadcast(&COND_refresh));
+  VOID(pthread_cond_broadcast(&COND_global_read_lock));
+}
+
+

sql/mysql_priv.h

@@ -1696,6 +1696,7 @@ void start_waiting_global_read_lock(THD *thd);
 bool make_global_read_lock_block_commit(THD *thd);
 bool set_protect_against_global_read_lock(void);
 void unset_protect_against_global_read_lock(void);
+void broadcast_refresh(void);
 
 /* Lock based on name */
 int lock_and_wait_for_table_name(THD *thd, TABLE_LIST *table_list);

sql/sql_base.cc

@@ -1104,7 +1104,7 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived)
   if (found_old_table)
   {
     /* Tell threads waiting for refresh that something has happened */
-    VOID(pthread_cond_broadcast(&COND_refresh));
+    broadcast_refresh();
   }
   if (!lock_in_use)
     VOID(pthread_mutex_unlock(&LOCK_open));
@@ -1674,7 +1674,7 @@ TABLE *unlink_open_table(THD *thd, TABLE *list, TABLE *find)
   }
   *prev=0;
   // Notify any 'refresh' threads
-  pthread_cond_broadcast(&COND_refresh);
+  broadcast_refresh();
   return start;
 }
 
@@ -2224,7 +2224,7 @@ static bool reopen_table(TABLE *table)
   if (table->triggers)
     table->triggers->set_table(table);
 
-  VOID(pthread_cond_broadcast(&COND_refresh));
+  broadcast_refresh();
   error=0;
 
  end:
@@ -2325,7 +2325,7 @@ bool reopen_tables(THD *thd,bool get_locks,bool in_refresh)
   {
     my_afree((gptr) tables);
   }
-  VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
+  broadcast_refresh();
   *prev=0;
   DBUG_RETURN(error);
 }
@@ -2361,7 +2361,7 @@ void close_old_data_files(THD *thd, TABLE *table, bool abort_locks,
     }
   }
   if (found)
-    VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
+    broadcast_refresh();
   DBUG_VOID_RETURN;
 }
 
@@ -2514,6 +2514,8 @@ TABLE *drop_locked_tables(THD *thd,const char *db, const char *table_name)
     }
   }
   *prev=0;
+  if (found)
+    broadcast_refresh();
   if (thd->locked_tables && thd->locked_tables->table_count == 0)
   {
     my_free((gptr) thd->locked_tables,MYF(0));
@@ -6194,7 +6196,7 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name,
         Signal any thread waiting for tables to be freed to
         reopen their tables
       */
-      (void) pthread_cond_broadcast(&COND_refresh);
+      broadcast_refresh();
       DBUG_PRINT("info", ("Waiting for refresh signal"));
       if (!(flags & RTFC_CHECK_KILLED_FLAG) || !thd->killed)
       {

sql/sql_handler.cc

@@ -254,7 +254,8 @@ err:
 
   DESCRIPTION
     Though this function takes a list of tables, only the first list entry
-    will be closed. Broadcasts a COND_refresh condition.
+    will be closed.
+    Broadcasts refresh if it closed the table.
 
   RETURN
     FALSE ok
@@ -291,7 +292,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables)
       if (close_thread_table(thd, table_ptr))
       {
         /* Tell threads waiting for refresh that something has happened */
-        VOID(pthread_cond_broadcast(&COND_refresh));
+        broadcast_refresh();
       }
       VOID(pthread_mutex_unlock(&LOCK_open));
     }
@@ -615,7 +616,7 @@ err0:
     tables are closed (if MYSQL_HA_FLUSH_ALL) is set.
     If 'tables' is NULL and MYSQL_HA_FLUSH_ALL is not set,
     all HANDLER tables marked for flush are closed.
-    Broadcasts a COND_refresh condition, for every table closed.
+    Broadcasts refresh for every table closed.
 
   NOTE
     Since mysql_ha_flush() is called when the base table has to be closed,
@@ -712,7 +713,7 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags,
                                 MYSQL_HA_REOPEN_ON_USAGE mark for reopen.
 
   DESCRIPTION
-    Broadcasts a COND_refresh condition, for every table closed.
+    Broadcasts refresh if it closed the table.
     The caller must lock LOCK_open.
 
   RETURN
@@ -750,7 +751,7 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags)
   if (close_thread_table(thd, table_ptr))
   {
     /* Tell threads waiting for refresh that something has happened */
-    VOID(pthread_cond_broadcast(&COND_refresh));
+    broadcast_refresh();
   }
 
   DBUG_RETURN(0);

sql/sql_insert.cc

@@ -1421,18 +1421,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
     */
     if (! (tmp= find_handler(thd, table_list)))
     {
-      /*
-        Avoid that a global read lock steps in while we are creating the
-        new thread. It would block trying to open the table. Hence, the
-        DI thread and this thread would wait until after the global
-        readlock is gone. Since the insert thread needs to wait for a
-        global read lock anyway, we do it right now. Note that
-        wait_if_global_read_lock() sets a protection against a new
-        global read lock when it succeeds. This needs to be released by
-        start_waiting_global_read_lock().
-      */
-      if (wait_if_global_read_lock(thd, 0, 1))
-        goto err;
       if (!(tmp=new delayed_insert()))
       {
 	my_error(ER_OUTOFMEMORY,MYF(0),sizeof(delayed_insert));
@@ -1473,11 +1461,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 	pthread_cond_wait(&tmp->cond_client,&tmp->mutex);
       }
       pthread_mutex_unlock(&tmp->mutex);
-      /*
-        Release the protection against the global read lock and wake
-        everyone, who might want to set a global read lock.
-      */
-      start_waiting_global_read_lock(thd);
       thd->proc_info="got old table";
       if (tmp->thd.killed)
       {
@@ -1513,11 +1496,6 @@ static TABLE *delayed_get_table(THD *thd,TABLE_LIST *table_list)
 
  err1:
   thd->fatal_error();
-  /*
-    Release the protection against the global read lock and wake
-    everyone, who might want to set a global read lock.
-  */
-  start_waiting_global_read_lock(thd);
  err:
   pthread_mutex_unlock(&LOCK_delayed_create);
   DBUG_RETURN(0); // Continue with normal insert
@@ -2876,7 +2854,7 @@ bool select_create::send_eof()
     if (!table->s->tmp_table)
     {
       if (close_thread_table(thd, &table))
-        VOID(pthread_cond_broadcast(&COND_refresh));
+        broadcast_refresh();
     }
     thd->extra_lock=0;
     table=0;
@@ -2906,7 +2884,7 @@ void select_create::abort()
         quick_rm_table(table_type, create_table->db, create_table->table_name);
       /* Tell threads waiting for refresh that something has happened */
       if (version != refresh_version)
-        VOID(pthread_cond_broadcast(&COND_refresh));
+        broadcast_refresh();
     }
     else if (!create_info->table_existed)
       close_temporary_table(thd, table, 1, 1);

sql/sql_parse.cc

@@ -2390,17 +2390,37 @@ static void reset_one_shot_variables(THD *thd)
 }
 
 
-/****************************************************************************
-** mysql_execute_command
-** Execute command saved in thd and current_lex->sql_command
-****************************************************************************/
+/*
+  Execute command saved in thd and current_lex->sql_command
+
+  SYNOPSIS
+    mysql_execute_command()
+      thd                       Thread handle
+
+  IMPLEMENTATION
+
+    Before every operation that can request a write lock for a table
+    wait if a global read lock exists. However do not wait if this
+    thread has locked tables already. No new locks can be requested
+    until the other locks are released. The thread that requests the
+    global read lock waits for write locked tables to become unlocked.
+
+    Note that wait_if_global_read_lock() sets a protection against a new
+    global read lock when it succeeds. This needs to be released by
+    start_waiting_global_read_lock() after the operation.
+
+  RETURN
+    FALSE       OK
+    TRUE        Error
+*/
 
 bool
 mysql_execute_command(THD *thd)
 {
-  bool	res= FALSE;
-  int result= 0;
-  LEX	*lex= thd->lex;
+  bool res= FALSE;
+  bool need_start_waiting= FALSE; // have protection against global read lock
+  int  result= 0;
+  LEX  *lex= thd->lex;
   /* first SELECT_LEX (have special meaning for many of non-SELECTcommands) */
   SELECT_LEX *select_lex= &lex->select_lex;
   /* first table of first SELECT_LEX */
@@ -2865,7 +2885,8 @@ mysql_execute_command(THD *thd)
       TABLE in the same way. That way we avoid that a new table is
       created during a gobal read lock.
     */
-    if (wait_if_global_read_lock(thd, 0, 1))
+    if (!thd->locked_tables &&
+        !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
     {
       res= 1;
       goto end_with_restore_list;
@@ -2901,7 +2922,7 @@ mysql_execute_command(THD *thd)
           {
             update_non_unique_table_error(create_table, "CREATE", duplicate);
             res= 1;
-            goto end_with_restart_wait;
+            goto end_with_restore_list;
           }
         }
         /* If we create merge table, we have to test tables in merge, too */
@@ -2917,7 +2938,7 @@ mysql_execute_command(THD *thd)
             {
               update_non_unique_table_error(tab, "CREATE", duplicate);
               res= 1;
-              goto end_with_restart_wait;
+              goto end_with_restore_list;
             }
           }
         }
@@ -2962,13 +2983,6 @@ mysql_execute_command(THD *thd)
 	send_ok(thd);
     }
 
-end_with_restart_wait:
-    /*
-      Release the protection against the global read lock and wake
-      everyone, who might want to set a global read lock.
-    */
-    start_waiting_global_read_lock(thd);
-
     /* put tables back for PS rexecuting */
 end_with_restore_list:
     lex->link_first_table_back(create_table, link_to_local);
@@ -3089,6 +3103,13 @@ end_with_restore_list:
       if (end_active_trans(thd))
 	goto error;
 
+      if (!thd->locked_tables &&
+          !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+      {
+        res= 1;
+        break;
+      }
+
       thd->enable_slow_log= opt_log_slow_admin_statements;
       res= mysql_alter_table(thd, select_lex->db, lex->name,
                              &lex->create_info,
@@ -3345,6 +3366,14 @@ end_with_restore_list:
       break;
     /* Skip first table, which is the table we are inserting in */
     select_lex->context.table_list= first_table->next_local;
+
+    if (!thd->locked_tables &&
+        !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+    {
+      res= 1;
+      break;
+    }
+
     res= mysql_insert(thd, all_tables, lex->field_list, lex->many_values,
 		      lex->update_list, lex->value_list,
                       lex->duplicates, lex->ignore);
@@ -3368,6 +3397,14 @@ end_with_restore_list:
     select_lex->options|= SELECT_NO_UNLOCK;
 
     unit->set_limit(select_lex);
+
+    if (! thd->locked_tables &&
+        ! (need_start_waiting= ! wait_if_global_read_lock(thd, 0, 1)))
+    {
+      res= 1;
+      break;
+    }
+
     if (!(res= open_and_lock_tables(thd, all_tables)))
     {
       /* Skip first table, which is the table we are inserting in */
@@ -3435,6 +3472,14 @@ end_with_restore_list:
       break;
     DBUG_ASSERT(select_lex->offset_limit == 0);
     unit->set_limit(select_lex);
+
+    if (!thd->locked_tables &&
+        !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+    {
+      res= 1;
+      break;
+    }
+
     res = mysql_delete(thd, all_tables, select_lex->where,
                        &select_lex->order_list,
                        unit->select_limit_cnt, select_lex->options,
@@ -3448,6 +3493,13 @@ end_with_restore_list:
       (TABLE_LIST *)thd->lex->auxilliary_table_list.first;
     multi_delete *result;
 
+    if (!thd->locked_tables &&
+        !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1)))
+    {
+      res= 1;
+      break;
+    }
+
     if ((res= multi_delete_precheck(thd, all_tables)))
       break;
 
@@ -5178,11 +5230,22 @@ end:
   */
   if (!(sql_command_flags[lex->sql_command] & CF_HAS_ROW_COUNT))
     thd->row_count_func= -1;
-  DBUG_RETURN(res || thd->net.report_error);
+
+  goto finish;
 
 error:
-  res= 1;           // would be better to set res=1 before "goto error"
-  goto end;
+  res= TRUE;
+
+finish:
+  if (need_start_waiting)
+  {
+    /*
+      Release the protection against the global read lock and wake
+      everyone, who might want to set a global read lock.
+    */
+    start_waiting_global_read_lock(thd);
+  }
+  DBUG_RETURN(res || thd->net.report_error);
 }
 
 

sql/sql_table.cc

@@ -3312,8 +3312,7 @@ bool mysql_create_table_internal(THD *thd,
     my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alias);
     goto err;
   }
-  if (wait_if_global_read_lock(thd, 0, 1))
-    goto err;
+
   VOID(pthread_mutex_lock(&LOCK_open));
   if (!internal_tmp_table && !(create_info->options & HA_LEX_CREATE_TMP_TABLE))
   {
@@ -3389,7 +3388,6 @@ bool mysql_create_table_internal(THD *thd,
   error= FALSE;
 unlock_and_end:
   VOID(pthread_mutex_unlock(&LOCK_open));
-  start_waiting_global_read_lock(thd);
 
 err:
   thd->proc_info="After create";
@@ -3621,7 +3619,7 @@ void close_cached_table(THD *thd, TABLE *table)
   thd->open_tables=unlink_open_table(thd,thd->open_tables,table);
 
   /* When lock on LOCK_open is freed other threads can continue */
-  pthread_cond_broadcast(&COND_refresh);
+  broadcast_refresh();
   DBUG_VOID_RETURN;
 }
 
@@ -6133,7 +6131,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     }
   }
   VOID(pthread_mutex_unlock(&LOCK_open));
-  VOID(pthread_cond_broadcast(&COND_refresh));
+  broadcast_refresh();
   /*
     The ALTER TABLE is always in its own transaction.
     Commit must not be called while LOCK_open is locked. It could call