Bug#19881: slave cores at close_temporary_tables under shutdown

   The bug was found in rpl_stm_000001 testing. In essence the following happens

   SLAVE thread receives          what happens
   start
                           init THD and its temp_table (tt0)
   stop
                           storing tt0 pointer to rli->save...
   start
                           restoring temp_tables - new pointer tt1
                           executing regular binlog event DROP temp_table
                           at the end of which tt1-refered list
                           must be empty (slave_open_temp_tables == 0)
                           but the pointer refers to tt0 location!
   shutdown
                           end_slave calls cleaning of temp_tables and crashes.

   The reason of the crash is that tt1 values is not zero upon DROPing the single temp table.
   This is due to alg of removing links from temp_tables list which "adapted" 5.0 code
   but w/o accounting that thd->temporary_tables in slave thread in prone to freeing.
   Upon freeing there is no more original '0' value available to denote empty list.

   temporary_tables must not refer to any "external" location, one of which thd->temporary_tables represents (since belong to THD instance).
   The fix done in sql_base.cc for two functions, look at there for details.


sql/sql_base.cc:
     refining prepend and remove link operation to thd->temporary_tables.
     The list turns to be "flat" double-linked, i.e "prev" accessor refers to an item instead of pointer to one as it was previously with "open_prev".
     On removal an invariant involving slave_open_temp_tables counter is checked.
     When it is zero thd->temporary_tables is set to zero explicitly. This can not be done, for what previous code hoped, because thd object changes when slave stop/start while
     slave's temporary_tables are maintained all the time, until reset/shutdow

1 files changed, 29 insertions, 7 deletions

diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 8ea53747b40..61ae91396ef 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1533,17 +1533,37 @@ bool close_temporary_table(THD *thd, TABLE_LIST *table_list)
 }
 
 /*
-  Close temporary table and unlink from thd->temporary tables
+  unlink from thd->temporary tables and close temporary table
 */
 
 void close_temporary_table(THD *thd, TABLE *table,
                            bool free_share, bool delete_table)
 {
-  TABLE **prev= table->open_prev;
-  if ((*table->open_prev= table->next))
-    table->next->open_prev= prev;
+  if (table->prev)
+  {
+    table->prev->next= table->next;
+    if (table->prev->next)
+      table->next->prev= table->prev;
+  }
+  else
+  {
+    /* removing the item from the list */
+    DBUG_ASSERT(table == thd->temporary_tables);
+    /*
+      slave must reset its temporary list pointer to zero to exclude
+      passing non-zero value to end_slave via rli->save_temporary_tables
+      when no temp tables opened, see an invariant below.
+    */
+    thd->temporary_tables= table->next;
+    if (thd->temporary_tables)
+      table->next->prev= 0;
+  }
   if (thd->slave_thread)
+  {
+    /* natural invariant of temporary_tables */
+    DBUG_ASSERT(slave_open_temp_tables || !thd->temporary_tables);
     slave_open_temp_tables--;
+  }
   close_temporary(table, free_share, delete_table);
 }
 
@@ -3500,10 +3520,12 @@ TABLE *open_temporary_table(THD *thd, const char *path, const char *db,
 
   if (link_in_list)
   {
-    tmp_table->open_prev= &thd->temporary_tables;
-    if ((tmp_table->next= thd->temporary_tables))
-      thd->temporary_tables->open_prev= &tmp_table->next;
+    /* growing temp list at the head */
+    tmp_table->next= thd->temporary_tables;
+    if (tmp_table->next)
+      tmp_table->next->prev= tmp_table;
     thd->temporary_tables= tmp_table;
+    thd->temporary_tables->prev= 0;
     if (thd->slave_thread)
       slave_open_temp_tables++;
   }