bk commit into 5.1 tree (davi:1.2619) BUG#21587

[Davi Arnaut]

Below is the list of changes that have just been committed into a local
5.1 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-02 23:35:50-03:00, davi@stripped +4 -0
  Bug#21587 FLUSH TABLES causes server crash when used with HANDLER statements
  
  This bug is a symptom of the way handler's tables are managed. The
  most different aspect, compared to the conventional behavior, is that
  the handler's tables are long lived, meaning that their lifetimes are
  not bounded by the duration of the command that opened them. For this
  effect the handler code uses its own list (handler_tables instead of
  open_tables) to hold open handler tables so that the tables won't be
  closed at the end of the command/statement. Besides the handler_tables
  list, there is a hash (handler_tables_hash) which is used to associate
  handler aliases to tables and to refresh the tables upon demand (flush
  tables).
  
  The current implementation doesn't work properly with refreshed tables
  -- more precisely when flush commands are issued by other initiators.
  This happens because when a handler open or read statement is being
  processed, the associated table has to be opened or locked and, for this
  matter, the open_tables and handler_tables lists are swapped so that the
  new table being opened is inserted into the handler_tables list. But when
  opening or locking the table, if the refresh version is different from the
  thread refresh version then all used tables in the open_tables list (now
  handler_tables) are refreshed. In the "refreshing" process the handler
  tables are flushed (closed) without being properly unlinked from the
  handler hash.
  
  The current implementation also fails to properly discard handlers of
  dropped tables but this and other problems are going to be addressed
  in future bug reports and patches.
  
  The chosen approach tries to properly save and restore the table state 
  so that no table is flushed during the table open and lock operations.
  The logic is almost the same as before with the list swapping, but with 
  a working glue code.

  mysql-test/include/handler.inc@stripped, 2007-10-02 23:35:44-03:00, davi@stripped +35 -0
    Add test case for Bug#21587

  mysql-test/r/handler_innodb.result@stripped, 2007-10-02 23:35:44-03:00, davi@stripped
+22 -0
    Add test case result for InnoDB run of the handler test for Bug#21587

  mysql-test/r/handler_myisam.result@stripped, 2007-10-02 23:35:45-03:00, davi@stripped
+22 -0
    Add test case result for MyISAM run of the handler test for Bug#21587

  sql/sql_handler.cc@stripped, 2007-10-02 23:35:45-03:00, davi@stripped +42 -12
    Backup and reset the open_tables and handler_table lists when opening
    a new handler table and merge the opened table into the handler_tables
    list afterwards. When locking, do the same but possibly close the table
    if a refresh is pending.

diff -Nrup a/mysql-test/include/handler.inc b/mysql-test/include/handler.inc
--- a/mysql-test/include/handler.inc	2007-08-29 18:32:14 -03:00
+++ b/mysql-test/include/handler.inc	2007-10-02 23:35:44 -03:00
@@ -498,3 +498,38 @@ handler t1_alias read a next;
 handler t1_alias READ a next where inexistent > 0;
 handler t1_alias close;
 drop table t1;
+
+#
+# Bug#21587 FLUSH TABLES causes server crash when used with HANDLER statements
+#
+
+--disable_warnings
+drop table if exists t1,t2;
+--enable_warnings
+create table t1 (c1 int);
+create table t2 (c1 int);
+insert into t1 values (1);
+insert into t2 values (2);
+--echo connection: default
+handler t1 open;
+handler t1 read first;
+connect (flush,localhost,root,,);
+connection flush;
+--echo connection: flush
+--send flush tables;
+connection default;
+--echo connection: default
+let $wait_condition=
+  select count(*) = 1 from information_schema.processlist
+  where state = "Flushing tables";
+--source include/wait_condition.inc
+handler t2 open;
+handler t2 read first;
+handler t1 read next;
+handler t1 close;
+handler t2 close;
+connection flush;
+reap;
+connection default;
+drop table t1,t2;
+disconnect flush;
diff -Nrup a/mysql-test/r/handler_innodb.result b/mysql-test/r/handler_innodb.result
--- a/mysql-test/r/handler_innodb.result	2007-08-29 19:00:46 -03:00
+++ b/mysql-test/r/handler_innodb.result	2007-10-02 23:35:44 -03:00
@@ -535,3 +535,25 @@ handler t1_alias READ a next where inexi
 ERROR 42S22: Unknown column 'inexistent' in 'field list'
 handler t1_alias close;
 drop table t1;
+drop table if exists t1,t2;
+create table t1 (c1 int);
+create table t2 (c1 int);
+insert into t1 values (1);
+insert into t2 values (2);
+connection: default
+handler t1 open;
+handler t1 read first;
+c1
+1
+connection: flush
+flush tables;;
+connection: default
+handler t2 open;
+handler t2 read first;
+c1
+2
+handler t1 read next;
+c1
+handler t1 close;
+handler t2 close;
+drop table t1,t2;
diff -Nrup a/mysql-test/r/handler_myisam.result b/mysql-test/r/handler_myisam.result
--- a/mysql-test/r/handler_myisam.result	2007-08-29 19:00:46 -03:00
+++ b/mysql-test/r/handler_myisam.result	2007-10-02 23:35:45 -03:00
@@ -535,3 +535,25 @@ handler t1_alias READ a next where inexi
 ERROR 42S22: Unknown column 'inexistent' in 'field list'
 handler t1_alias close;
 drop table t1;
+drop table if exists t1,t2;
+create table t1 (c1 int);
+create table t2 (c1 int);
+insert into t1 values (1);
+insert into t2 values (2);
+connection: default
+handler t1 open;
+handler t1 read first;
+c1
+1
+connection: flush
+flush tables;;
+connection: default
+handler t2 open;
+handler t2 read first;
+c1
+2
+handler t1 read next;
+c1
+handler t1 close;
+handler t2 close;
+drop table t1,t2;
diff -Nrup a/sql/sql_handler.cc b/sql/sql_handler.cc
--- a/sql/sql_handler.cc	2007-08-30 16:23:53 -03:00
+++ b/sql/sql_handler.cc	2007-10-02 23:35:45 -03:00
@@ -65,11 +65,6 @@
 static enum enum_ha_read_modes rkey_to_rnext[]=
 { RNEXT_SAME, RNEXT, RPREV, RNEXT, RPREV, RNEXT, RPREV, RPREV };
 
-#define HANDLER_TABLES_HACK(thd) {      \
-  TABLE *tmp=thd->open_tables;          \
-  thd->open_tables=thd->handler_tables; \
-  thd->handler_tables=tmp; }
-
 static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags);
 
 
@@ -187,6 +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;
   DBUG_ENTER("mysql_ha_open");
   DBUG_PRINT("enter",("'%s'.'%s' as '%s'  reopen: %d",
                       tables->db, tables->table_name, tables->alias,
@@ -215,17 +211,30 @@ 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;
+
+  /* no pre-opened tables */
+  thd->open_tables= NULL;
+  /* to avoid flushes */
+  thd->handler_tables= NULL;
+
   /*
     open_tables() will set 'tables->table' if successful.
     It must be NULL for a real open when calling open_tables().
   */
   DBUG_ASSERT(! tables->table);
-  HANDLER_TABLES_HACK(thd);
 
   /* for now HANDLER can be used only for real TABLES */
   tables->required_type= FRMTYPE_TABLE;
   error= open_tables(thd, &tables, &counter, 0);
-  HANDLER_TABLES_HACK(thd);
+
+  /* restore the state and merge the opened table */
+  thd->handler_tables= thd->open_tables ?
+                       thd->open_tables->next= backup_handler_tables,
+                       thd->open_tables : backup_handler_tables;
+  thd->open_tables= backup_open_tables;
 
   if (error)
     goto err;
@@ -351,7 +360,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;
+  TABLE         *table, *backup_open_tables, *backup_handler_tables;
   MYSQL_LOCK    *lock;
   List<Item>	list;
   Protocol	*protocol= thd->protocol;
@@ -361,7 +370,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   uint          num_rows;
   uchar		*key;
   uint		key_len;
-  bool          not_used;
+  bool          need_reopen;
   DBUG_ENTER("mysql_ha_read");
   DBUG_PRINT("enter",("'%s'.'%s' as '%s'",
                       tables->db, tables->table_name, tables->alias));
@@ -375,6 +384,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   List_iterator<Item> it(list);
   it++;
 
+retry:
   if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash,
                                               (uchar*) tables->alias,
                                               strlen(tables->alias) + 1)))
@@ -428,9 +438,29 @@ bool mysql_ha_read(THD *thd, TABLE_LIST 
   }
   tables->table=table;
 
-  HANDLER_TABLES_HACK(thd);
-  lock= mysql_lock_tables(thd, &tables->table, 1, 0, &not_used);
-  HANDLER_TABLES_HACK(thd);
+  /* save open_ and handler_ 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;
+
+  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)
+  {
+    mysql_ha_close_table(thd, tables);
+    hash_tables->table= NULL;
+    close_thread_tables(thd);
+    goto retry;
+  }
 
   if (!lock)
     goto err0; // mysql_lock_tables() printed error message already