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-06 01:21:55-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 (makred 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-06 01:21:52-03:00, davi@stripped +8 -2
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-06 01:21:53-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-06 01:21:52 -03:00
@@ -1745,7 +1745,12 @@ 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, the
+ handler tables which are marked for flush (with older versions) 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);
@@ -1809,7 +1814,8 @@ TABLE *open_table(THD *thd, TABLE_LIST *
table->s->version (this is an optimization (?)).
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.
+ table_is_used call for details. Stale HANDLER tables were
+ already closed under 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-06 01:21:53 -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;
+ /* not only temporary tables */
+ thd->open_tables= thd->handler_tables;
+ /*
+ Lock table, but if it fails don't close any tables. That's why the
+ thd->handler_table list is not protected here, mysql_lock_tables
+ won't touch it.
+ */
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)
| Thread |
|---|
| • bk commit into 5.0 tree (davi:1.2528) BUG#31409 | Davi Arnaut | 6 Oct |
