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

into  mysql.com:/home/mydev/mysql-5.0-bug16986-main


sql/sql_table.cc:
  Auto merged

mysql-test/r/lock_multi.result

@@ -67,6 +67,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 table t1 (f1 int(12) unsigned not null auto_increment, primary key(f1)) engine=innodb;
 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; //

mysql-test/t/lock_multi.test

@@ -142,6 +142,7 @@ disconnect con2;
 --error ER_DB_DROP_EXISTS
 DROP DATABASE mysqltest_1;
 
+#
 # Bug#16986 - Deadlock condition with MyISAM tables
 #
 connection locker;
@@ -170,6 +171,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 #17264: MySQL Server freeze

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

@@ -905,7 +905,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();
   }
 }
 
@@ -997,9 +997,9 @@ bool lock_table_names(THD *thd, TABLE_LIST *table_list)
 			(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
@@ -1013,7 +1013,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();
 }
 
 
@@ -1304,3 +1304,37 @@ bool make_global_read_lock_block_commit(THD *thd)
 }
 
 
+/*
+  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

@@ -1344,6 +1344,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

@@ -530,7 +530,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));
@@ -1035,7 +1035,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;
 }
 
@@ -1577,7 +1577,7 @@ bool reopen_table(TABLE *table,bool locked)
   if (table->triggers)
     table->triggers->set_table(table);
 
-  VOID(pthread_cond_broadcast(&COND_refresh));
+  broadcast_refresh();
   error=0;
 
  end:
@@ -1678,7 +1678,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);
 }
@@ -1715,7 +1715,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;
 }
 
@@ -1807,7 +1807,7 @@ bool drop_locked_tables(THD *thd,const char *db, const char *table_name)
   }
   *prev=0;
   if (found)
-    VOID(pthread_cond_broadcast(&COND_refresh)); // Signal to refresh
+    broadcast_refresh();
   if (thd->locked_tables && thd->locked_tables->table_count == 0)
   {
     my_free((gptr) thd->locked_tables,MYF(0));
@@ -5249,7 +5249,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 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
 
   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));
     }
@@ -608,7 +609,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
     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,
@@ -704,7 +705,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
@@ -742,7 +743,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

@@ -1349,18 +1349,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));
@@ -1401,11 +1389,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)
       {
@@ -1441,11 +1424,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
@@ -2676,7 +2654,7 @@ bool select_create::send_eof()
       hash_delete(&open_cache,(byte*) table);
       /* Tell threads waiting for refresh that something has happened */
       if (version != refresh_version)
-        VOID(pthread_cond_broadcast(&COND_refresh));
+        broadcast_refresh();
     }
     lock=0;
     table=0;
@@ -2705,7 +2683,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, create_table->db, create_table->table_name);

sql/sql_parse.cc

@@ -2337,15 +2337,35 @@ 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;
+  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) */
@@ -2832,7 +2852,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;
@@ -2857,7 +2878,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 */
@@ -2873,7 +2894,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;
             }
           }
         }
@@ -2915,13 +2936,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);
@@ -3039,6 +3053,13 @@ mysql_execute_command(THD *thd)
 	goto error;
       else
       {
+        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,
@@ -3296,6 +3317,14 @@ mysql_execute_command(THD *thd)
       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);
@@ -3319,6 +3348,14 @@ mysql_execute_command(THD *thd)
     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 */
@@ -3386,6 +3423,14 @@ mysql_execute_command(THD *thd)
       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,
@@ -3399,6 +3444,13 @@ mysql_execute_command(THD *thd)
       (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;
 
@@ -4965,10 +5017,22 @@ mysql_execute_command(THD *thd)
   if (lex->sql_command != SQLCOM_CALL && lex->sql_command != SQLCOM_EXECUTE &&
       uc_update_queries[lex->sql_command]<2)
     thd->row_count_func= -1;
-  DBUG_RETURN(res || thd->net.report_error);
+
+  goto end;
 
 error:
-  DBUG_RETURN(1);
+  res= TRUE;
+
+end:
+  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

@@ -1689,8 +1689,6 @@ bool mysql_create_table(THD *thd,const char *db, const char *table_name,
     my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alias);
     DBUG_RETURN(TRUE);
   }
-  if (wait_if_global_read_lock(thd, 0, 1))
-    DBUG_RETURN(TRUE);
   VOID(pthread_mutex_lock(&LOCK_open));
   if (!internal_tmp_table && !(create_info->options & HA_LEX_CREATE_TMP_TABLE))
   {
@@ -1758,7 +1756,6 @@ bool mysql_create_table(THD *thd,const char *db, const char *table_name,
 
 end:
   VOID(pthread_mutex_unlock(&LOCK_open));
-  start_waiting_global_read_lock(thd);
   thd->proc_info="After create";
   DBUG_RETURN(error);
 
@@ -1938,7 +1935,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;
 }
 
@@ -3909,7 +3906,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
   if (error)
   {
     VOID(pthread_mutex_unlock(&LOCK_open));
-    VOID(pthread_cond_broadcast(&COND_refresh));
+    broadcast_refresh();
     goto err;
   }
   thd->proc_info="end";
@@ -3919,7 +3916,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     Query_log_event qinfo(thd, thd->query, thd->query_length, FALSE, FALSE);
     mysql_bin_log.write(&qinfo);
   }
-  VOID(pthread_cond_broadcast(&COND_refresh));
+  broadcast_refresh();
   VOID(pthread_mutex_unlock(&LOCK_open));
 #ifdef HAVE_BERKELEY_DB
   if (old_db_type == DB_TYPE_BERKELEY_DB)