List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:June 5 2006 4:00pm
Subject:bk commit into 5.1 tree (aelkin:1.2160) BUG#19881
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of elkin. When elkin 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
  1.2160 06/06/05 19:00:08 aelkin@stripped +1 -0
  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
    1.323 06/06/05 19:00:03 aelkin@stripped +24 -8
       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

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	aelkin
# Host:	dsl-hkigw8-feb0de00-199.dhcp.inet.fi
# Root:	/usr_rh9/home/elkin.rh9/MySQL/TEAM/FIXES/5.1/bug19881_double_drop_tmp_slave

--- 1.322/sql/sql_base.cc	2006-05-17 16:13:48 +03:00
+++ 1.323/sql/sql_base.cc	2006-06-05 19:00:03 +03:00
@@ -1168,7 +1168,6 @@
   DBUG_RETURN(found_old_table);
 }
 
-
 /* close_temporary_tables' internal, 4 is due to uint4korr definition */
 static inline uint  tmpkeyval(THD *thd, TABLE *table)
 {
@@ -1533,21 +1532,38 @@
 }
 
 /*
-  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;
+  TABLE *del= table;
+  if (table->prev)
+  {
+    if ((table->prev->next= table->next))
+      table->next->prev= table->prev;
+  }
+  else
+  {
+    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.
+    */
+    if ((thd->temporary_tables= table->next))
+      table->next->prev= 0;
+  }
   if (thd->slave_thread)
+  {
+    DBUG_ASSERT(slave_open_temp_tables || !thd->temporary_tables);
     slave_open_temp_tables--;
+  }
+  /* natural invariant of temporary_tables */
   close_temporary(table, free_share, delete_table);
 }
 
-
 /*
   Close and delete a temporary table
 
@@ -3500,10 +3516,10 @@
 
   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;
+      tmp_table->next->prev= tmp_table;
     thd->temporary_tables= tmp_table;
+    thd->temporary_tables->prev= 0;
     if (thd->slave_thread)
       slave_open_temp_tables++;
   }
Thread
bk commit into 5.1 tree (aelkin:1.2160) BUG#19881Andrei Elkin5 Jun