List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:August 15 2007 7:13am
Subject:bk commit into 5.0 tree (davi:1.2512) BUG#25856
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of davi. When davi does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-08-15 02:13:39-03:00, davi@stripped +3 -0
  Bug#25856 (HANDLER table OPEN in one connection lock DROP TABLE in another one)
  
  mysql_ha_open calls mysql_ha_close on the error path (unsupported) to close the (opened)
table before inserting it into the tables hash list handler_tables_hash) but
mysql_ha_close only closes tables which are on the hash list, causing the table to be
left open and locked.
  
  This change moves the table close logic into a separate function that is always called
on the error path of mysql_ha_open or on a normal handler close (mysql_ha_close).

  mysql-test/r/handler.result@stripped, 2007-08-15 02:09:44-03:00, davi@stripped +4 -0
    Bug#25856 test result

  mysql-test/t/handler.test@stripped, 2007-08-15 02:09:44-03:00, davi@stripped +12 -0
    Bug#25856 test case

  sql/sql_handler.cc@stripped, 2007-08-15 02:09:44-03:00, davi@stripped +53 -33
    Move the table close logic into a separate function that is always called on the error
path of mysql_ha_open or on a normal handler close 

diff -Nrup a/mysql-test/r/handler.result b/mysql-test/r/handler.result
--- a/mysql-test/r/handler.result	2006-10-04 08:09:34 -03:00
+++ b/mysql-test/r/handler.result	2007-08-15 02:09:44 -03:00
@@ -482,3 +482,7 @@ ERROR 42S02: Table 'test.t1' doesn't exi
 drop table if exists t1;
 Warnings:
 Note	1051	Unknown table 't1'
+create table t1 (a int) ENGINE=MEMORY;
+handler t1 open;
+ERROR HY000: Table storage engine for 't1' doesn't have this option
+drop table t1;
diff -Nrup a/mysql-test/t/handler.test b/mysql-test/t/handler.test
--- a/mysql-test/t/handler.test	2006-08-20 12:47:28 -03:00
+++ b/mysql-test/t/handler.test	2007-08-15 02:09:44 -03:00
@@ -427,3 +427,15 @@ select * from t1;
 # Just to be sure and not confuse the next test case writer.
 drop table if exists t1;
 
+#
+# Bug#25856 - HANDLER table OPEN in one connection lock DROP TABLE in another one
+#
+create table t1 (a int) ENGINE=MEMORY;
+# client 2
+connection con2;
+--error 1031
+handler t1 open;
+# client 1
+connection default;
+drop table t1;
+
diff -Nrup a/sql/sql_handler.cc b/sql/sql_handler.cc
--- a/sql/sql_handler.cc	2007-05-11 13:33:11 -03:00
+++ b/sql/sql_handler.cc	2007-08-15 02:09:44 -03:00
@@ -119,6 +119,50 @@ static void mysql_ha_hash_free(TABLE_LIS
   my_free((char*) tables, MYF(0));
 }
 
+/*
+  Close a HANDLER table.
+
+  SYNOPSIS
+    mysql_ha_close_table()
+    thd                         Thread identifier.
+    tables                      A list of tables with the first entry to close.
+
+  DESCRIPTION
+    Though this function takes a list of tables, only the first list entry
+    will be closed.
+    Broadcasts refresh if it closed the table.
+
+  RETURN
+    Nothing
+*/
+
+static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables)
+{
+  TABLE **table_ptr;
+
+  /*
+    Though we could take the table pointer from hash_tables->table,
+    we must follow the thd->handler_tables chain anyway, as we need the
+    address of the 'next' pointer referencing this table
+    for close_thread_table().
+  */
+  for (table_ptr= &(thd->handler_tables);
+       *table_ptr && (*table_ptr != tables->table);
+         table_ptr= &(*table_ptr)->next)
+    ;
+
+  if (*table_ptr)
+  {
+    (*table_ptr)->file->ha_index_or_rnd_end();
+    VOID(pthread_mutex_lock(&LOCK_open));
+    if (close_thread_table(thd, table_ptr))
+    {
+      /* Tell threads waiting for refresh that something has happened */
+      broadcast_refresh();
+    }
+    VOID(pthread_mutex_unlock(&LOCK_open));
+  }
+}
 
 /*
   Open a HANDLER table.
@@ -145,7 +189,7 @@ static void mysql_ha_hash_free(TABLE_LIS
 
 bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
 {
-  TABLE_LIST    *hash_tables;
+  TABLE_LIST    *hash_tables = NULL;
   char          *db, *name, *alias;
   uint          dblen, namelen, aliaslen, counter;
   int           error;
@@ -197,7 +241,6 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
   {
     if (! reopen)
       my_error(ER_ILLEGAL_HA, MYF(0), tables->alias);
-    mysql_ha_close(thd, tables);
     goto err;
   }
 
@@ -225,11 +268,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
 
     /* add to hash */
     if (my_hash_insert(&thd->handler_tables_hash, (byte*) hash_tables))
-    {
-      my_free((char*) hash_tables, MYF(0));
-      mysql_ha_close(thd, tables);
       goto err;
-    }
   }
 
   if (! reopen)
@@ -238,13 +277,17 @@ bool mysql_ha_open(THD *thd, TABLE_LIST 
   DBUG_RETURN(FALSE);
 
 err:
+  if (hash_tables)
+    my_free((char*) hash_tables, MYF(0));
+  if (tables->table)
+    mysql_ha_close_table(thd, tables);
   DBUG_PRINT("exit",("ERROR"));
   DBUG_RETURN(TRUE);
 }
 
 
 /*
-  Close a HANDLER table.
+  Close a HANDLER table by alias or table name
 
   SYNOPSIS
     mysql_ha_close()
@@ -252,9 +295,8 @@ err:
     tables                      A list of tables with the first entry to close.
 
   DESCRIPTION
-    Though this function takes a list of tables, only the first list entry
-    will be closed.
-    Broadcasts refresh if it closed the table.
+    Closes the table that is associated (on the handler tables hash) with the
+    name (table->alias) of the specified table.
 
   RETURN
     FALSE ok
@@ -264,7 +306,6 @@ err:
 bool mysql_ha_close(THD *thd, TABLE_LIST *tables)
 {
   TABLE_LIST    *hash_tables;
-  TABLE         **table_ptr;
   DBUG_ENTER("mysql_ha_close");
   DBUG_PRINT("enter",("'%s'.'%s' as '%s'",
                       tables->db, tables->table_name, tables->alias));
@@ -273,28 +314,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST
                                               (byte*) tables->alias,
                                               strlen(tables->alias) + 1)))
   {
-    /*
-      Though we could take the table pointer from hash_tables->table,
-      we must follow the thd->handler_tables chain anyway, as we need the
-      address of the 'next' pointer referencing this table
-      for close_thread_table().
-    */
-    for (table_ptr= &(thd->handler_tables);
-         *table_ptr && (*table_ptr != hash_tables->table);
-           table_ptr= &(*table_ptr)->next)
-      ;
-
-    if (*table_ptr)
-    {
-      (*table_ptr)->file->ha_index_or_rnd_end();
-      VOID(pthread_mutex_lock(&LOCK_open));
-      if (close_thread_table(thd, table_ptr))
-      {
-        /* Tell threads waiting for refresh that something has happened */
-        broadcast_refresh();
-      }
-      VOID(pthread_mutex_unlock(&LOCK_open));
-    }
+    mysql_ha_close_table(thd, hash_tables);
     hash_delete(&thd->handler_tables_hash, (byte*) hash_tables);
   }
   else
Thread
bk commit into 5.0 tree (davi:1.2512) BUG#25856Davi Arnaut15 Aug
  • Re: bk commit into 5.0 tree (davi:1.2512) BUG#25856Konstantin Osipov15 Aug