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 the fixes for bugs 31397 and 31409.

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.

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_handler.cc

@@ -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 *tables, bool reopen)
   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,18 +211,31 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen)
     }
   }
 
+  /* 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 into handler_tables list */
+  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 *tables,
                    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 *tables,
   uint          num_rows;
   byte		*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 *tables,
   List_iterator<Item> it(list);
   it++;
 
+retry:
   if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash,
                                               (byte*) tables->alias,
                                               strlen(tables->alias) + 1)))
@@ -427,9 +437,28 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
   }
   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;
+    goto retry;
+  }
 
   if (!lock)
     goto err0; // mysql_lock_tables() printed error message already