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-10-09 12:02:59-03:00, davi@stripped +2 -0
Bug#31409 RENAME TABLE causes server crash or deadlock when used with HANDLER statements
This deadlock occurs when a client issues a HANDLER ... OPEN statement
that tries to open a table that has a pending name-lock on it by another
client that also needs a name-lock on some other table which is already
open and associated to a HANDLER instance owned by the first client.
The deadlock happens because the open_table() function will back-off
and wait until the name-lock goes away, causing a circular wait if some
other name-lock is also pending for one of the open HANDLER tables.
Such situation, for example, can be easily repeated by issuing a RENAME
TABLE command in such a way that the existing table is already open
as a HANDLER table by another client and this client tries to open
a HANDLER to the new table name.
The solution is to allow handler tables with older versions (marked for
flush) to be closed before waiting for the name-lock completion. This is
safe because no other name-lock can be issued between the flush and the
check for pending name-locks.
The test case for this bug is going to be committed into 5.1 because it
requires a test feature only avaiable in 5.1 (wait_condition).
sql/sql_base.cc@stripped, 2007-10-09 12:02:57-03:00, davi@stripped +11 -1
Improve comments in the open_table() function, stating the importance
of the handler tables flushing for the back-off process.
sql/sql_handler.cc@stripped, 2007-10-09 12:02:57-03:00, davi@stripped +28 -19
Allows handler tables flushes when opening new tables in order to avoid
potential deadlocks. Add comments explaining the importance of the flush.
diff -Nrup a/sql/sql_base.cc b/sql/sql_base.cc
--- a/sql/sql_base.cc 2007-08-24 08:28:10 -03:00
+++ b/sql/sql_base.cc 2007-10-09 12:02:57 -03:00
@@ -1745,7 +1745,13 @@ TABLE *open_table(THD *thd, TABLE_LIST *
DBUG_RETURN(0);
}
- /* close handler tables which are marked for flush */
+ /*
+ In order for the back off and re-start process to work properly,
+ handler tables having old versions (due to FLUSH TABLES or pending
+ name-lock) MUST be closed. This is specially important if a name-lock
+ is pending for any table of the handler_tables list, otherwise a
+ deadlock may occur.
+ */
if (thd->handler_tables)
mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE, TRUE);
@@ -1810,6 +1816,10 @@ TABLE *open_table(THD *thd, TABLE_LIST *
table->db_stat == 0 signals wait_for_locked_table_names
that the tables in question are not used any more. See
table_is_used call for details.
+
+ Notice that HANDLER tables were already taken care of by
+ the earlier call to mysql_ha_flush() in this same critical
+ section.
*/
close_old_data_files(thd,thd->open_tables,0,0);
/*
diff -Nrup a/sql/sql_handler.cc b/sql/sql_handler.cc
--- a/sql/sql_handler.cc 2007-10-04 17:34:38 -03:00
+++ b/sql/sql_handler.cc 2007-10-09 12:02:57 -03:00
@@ -182,7 +182,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST
char *db, *name, *alias;
uint dblen, namelen, aliaslen, counter;
int error;
- TABLE *backup_open_tables, *backup_handler_tables;
+ TABLE *backup_open_tables;
DBUG_ENTER("mysql_ha_open");
DBUG_PRINT("enter",("'%s'.'%s' as '%s' reopen: %d",
tables->db, tables->table_name, tables->alias,
@@ -211,14 +211,20 @@ bool mysql_ha_open(THD *thd, TABLE_LIST
}
}
- /* save open_ and handler_ tables state */
- backup_open_tables= thd->open_tables;
- backup_handler_tables= thd->handler_tables;
+ /*
+ Save and reset the open_tables list so that open_tables() won't
+ be able to access (or know about) the previous list. And on return
+ from open_tables(), thd->open_tables will contain only the opened
+ table.
+
+ The thd->handler_tables list is kept as-is to avoid deadlocks if
+ open_table(), called by open_tables(), needs to back-off because
+ of a pending name-lock on the table being opened.
- /* no pre-opened tables */
+ See open_table() back-off comments for more details.
+ */
+ backup_open_tables= thd->open_tables;
thd->open_tables= NULL;
- /* to avoid flushes */
- thd->handler_tables= NULL;
/*
open_tables() will set 'tables->table' if successful.
@@ -231,9 +237,12 @@ bool mysql_ha_open(THD *thd, TABLE_LIST
error= open_tables(thd, &tables, &counter, 0);
/* restore the state and merge the opened table into handler_tables list */
- thd->handler_tables= thd->open_tables ?
- thd->open_tables->next= backup_handler_tables,
- thd->open_tables : backup_handler_tables;
+ if (thd->open_tables)
+ {
+ thd->open_tables->next= thd->handler_tables;
+ thd->handler_tables= thd->open_tables;
+ }
+
thd->open_tables= backup_open_tables;
if (error)
@@ -360,7 +369,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST
ha_rows select_limit_cnt, ha_rows offset_limit_cnt)
{
TABLE_LIST *hash_tables;
- TABLE *table, *backup_open_tables, *backup_handler_tables;
+ TABLE *table, *backup_open_tables;
MYSQL_LOCK *lock;
List<Item> list;
Protocol *protocol= thd->protocol;
@@ -437,20 +446,20 @@ retry:
}
tables->table=table;
- /* save open_ and handler_ tables state */
+ /* save open_tables state */
backup_open_tables= thd->open_tables;
- backup_handler_tables= thd->handler_tables;
-
- /* no pre-opened tables */
- thd->open_tables= NULL;
- /* to avoid flushes */
- thd->handler_tables= NULL;
+ /*
+ mysql_lock_tables() needs thd->open_tables to be set correctly to
+ be able to handle aborts properly. When the abort happens, it's
+ safe to not protect thd->handler_tables because it won't close any
+ tables.
+ */
+ thd->open_tables= thd->handler_tables;
lock= mysql_lock_tables(thd, &tables->table, 1,
MYSQL_LOCK_NOTIFY_IF_NEED_REOPEN, &need_reopen);
/* restore previous context */
- thd->handler_tables= backup_handler_tables;
thd->open_tables= backup_open_tables;
if (need_reopen)
