Fixes for Bug #38016 Maria: trying to access freed memory when committing a transaction

Don't write out states if they haven't changed

sql/sql_base.cc:
  Call extra(HA_EXTRA_PREPARE_FOR_DROP) before doing a drop of a table
  More DBUG
sql/sql_table.cc:
  Call extra(HA_EXTRA_PREPARE_FOR_RENAME) before renaming a table
storage/maria/ha_maria.cc:
  Ensure that file->trn is set when we call extra(HA_EXTRA_PREPARE_FOR_DROP/RENAME)
storage/maria/ma_close.c:
  When doing close, assert if we have pointers in trn->table_list that points to the MARIA_SHARE
storage/maria/ma_extra.c:
  Reset info->state_start in case of drop/rename. This fixes the problem of accessing freed memory in repair
  Don't write state changed if they haven't changed
storage/maria/ma_open.c:
  Reset share->changed after we have written out a state (speed optimization to not write states when they haven't changed)
storage/maria/ma_state.c:
  Decrement share->in_trans properly in DBUG_BINARY to ensure that the DBUG_ASSERT() in maria_close() works
  More DBUG

sql/sql_base.cc

@@ -2184,6 +2184,7 @@ void unlink_open_table(THD *thd, TABLE *find, bool unlock)
 void drop_open_table(THD *thd, TABLE *table, const char *db_name,
                      const char *table_name)
 {
+  DBUG_ENTER("drop_open_table");
   if (table->s->tmp_table)
     close_temporary_table(thd, table, 1, 1);
   else
@@ -2194,10 +2195,12 @@ void drop_open_table(THD *thd, TABLE *table, const char *db_name,
       unlink_open_table() also tells threads waiting for refresh or close
       that something has happened.
     */
+    table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
     unlink_open_table(thd, table, FALSE);
     quick_rm_table(table_type, db_name, table_name, 0);
     VOID(pthread_mutex_unlock(&LOCK_open));
   }
+  DBUG_VOID_RETURN;
 }
 
 
@@ -3680,6 +3683,9 @@ TABLE *drop_locked_tables(THD *thd,const char *db, const char *table_name)
     if (!strcmp(table->s->table_name.str, table_name) &&
 	!strcmp(table->s->db.str, db))
     {
+      /* Inform handler that table will be dropped after close */
+      table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
+
       /* If MERGE child, forward lock handling to parent. */
       mysql_lock_remove(thd, thd->locked_tables,
                         table->parent ? table->parent : table, TRUE);

sql/sql_table.cc

@@ -7185,7 +7185,7 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   if (errpos >= 3 && to->file->ha_end_bulk_insert(error > 1) && error <= 0)
   {
     to->file->print_error(my_errno,MYF(0));
-    error=1;
+    error= 1;
   }
   to->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
@@ -7208,6 +7208,8 @@ copy_data_between_tables(TABLE *from,TABLE *to,
   to->file->ha_release_auto_increment();
   if (errpos >= 2 && to->file->ha_external_lock(thd,F_UNLCK))
     error=1;
+  if (error < 0 && to->file->extra(HA_EXTRA_PREPARE_FOR_RENAME))
+    error= 1;
   DBUG_RETURN(error > 0 ? -1 : 0);
 }
 

storage/maria/ha_maria.cc

@@ -44,6 +44,7 @@ C_MODE_END
 #ifdef MARIA_CANNOT_ROLLBACK
 #define trans_register_ha(A, B, C)  do { /* nothing */ } while(0)
 #endif
+#define THD_TRN (*(TRN **)thd_ha_data(thd, maria_hton))
 
 ulong pagecache_division_limit, pagecache_age_threshold;
 ulonglong pagecache_buffer_size;
@@ -2201,6 +2202,21 @@ int ha_maria::extra(enum ha_extra_function operation)
 {
   if ((specialflag & SPECIAL_SAFE_MODE) && operation == HA_EXTRA_KEYREAD)
     return 0;
+
+  /*
+    We have to set file->trn here because in some cases we call
+    extern_lock(F_UNLOCK) (which resets file->trn) followed by maria_close()
+    without calling commit/rollback in between.  If file->trn is not set
+    we can't remove file->share from the transaction list in the extra() call.
+  */
+
+  if (!file->trn &&
+      (operation == HA_EXTRA_PREPARE_FOR_DROP ||
+       operation == HA_EXTRA_PREPARE_FOR_RENAME))
+  {
+    THD *thd= table->in_use;
+    file->trn= THD_TRN;
+  }
   return maria_extra(file, operation, 0);
 }
 
@@ -2240,8 +2256,6 @@ int ha_maria::delete_table(const char *name)
   return maria_delete_table(name);
 }
 
-#define THD_TRN (*(TRN **)thd_ha_data(thd, maria_hton))
-
 int ha_maria::external_lock(THD *thd, int lock_type)
 {
   TRN *trn= THD_TRN;

storage/maria/ma_close.c

@@ -69,6 +69,10 @@ int maria_close(register MARIA_HA *info)
   if (flag)
   {
     /* Last close of file; Flush everything */
+
+    /* Check that we don't have any dangling pointers from the transaction */
+    DBUG_ASSERT(share->in_trans == 0);
+
     if (share->kfile.file >= 0)
     {
       if ((*share->once_end)(share))

storage/maria/ma_extra.c

@@ -326,7 +326,7 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function,
     {
       _ma_remove_table_from_trnman(share, info->trn);
       /* Ensure we don't point to the deleted data in trn */
-      info->state= &share->state.state;
+      info->state= info->state_start= &share->state.state;
     }
 
     type= do_flush ? FLUSH_RELEASE : FLUSH_IGNORE_CHANGED;
@@ -347,7 +347,7 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function,
       if (do_flush)
       {
         /* Save the state so that others can find it from disk. */
-        if (_ma_state_info_write(share, 1 | 2) ||
+        if ((share->changed && _ma_state_info_write(share, 1 | 2)) ||
             my_sync(share->kfile.file, MYF(0)))
           error= my_errno;
         else

storage/maria/ma_open.c

@@ -1186,6 +1186,7 @@ uint _ma_state_info_write(MARIA_SHARE *share, uint pWrite)
   res= _ma_state_info_write_sub(share->kfile.file, &share->state, pWrite);
   if (pWrite & 4)
     pthread_mutex_unlock(&share->intern_lock);
+  share->changed= 0;
   return res;
 }
 

storage/maria/ma_state.c

@@ -79,6 +79,9 @@ my_bool _ma_setup_live_state(MARIA_HA *info)
 
   pthread_mutex_lock(&share->intern_lock);
   share->in_trans++;
+  DBUG_PRINT("info", ("share: 0x%lx  in_trans: %d",
+                      (ulong) share, share->in_trans));
+
   history= share->state_history;
 
   /*
@@ -359,15 +362,16 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit,
 {
   my_bool error= 0;
   MARIA_USED_TABLES *tables, *next;
+  DBUG_ENTER("_ma_trnman_end_trans_hook");
   
   for (tables= (MARIA_USED_TABLES*) trn->used_tables;
        tables;
        tables= next)
   {
+    MARIA_SHARE *share= tables->share;
     next= tables->next;
     if (commit)
     {
-      MARIA_SHARE *share= tables->share;
       MARIA_STATE_HISTORY *history;
 
       pthread_mutex_lock(&share->intern_lock);
@@ -405,13 +409,27 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit,
         /* Remove not visible states */
         share->state_history= _ma_remove_not_visible_states(history, 0, 1);
       }
+      DBUG_PRINT("info", ("share: 0x%lx  in_trans: %d",
+                          (ulong) share, share->in_trans));
+      share->in_trans--;
+      pthread_mutex_unlock(&share->intern_lock);
+    }
+    else
+    {
+#ifndef DBUG_OFF
+      /*
+        We need to keep share->in_trans correct in the debug library
+        because of the assert in maria_close()
+      */
+      pthread_mutex_lock(&share->intern_lock);
       share->in_trans--;
       pthread_mutex_unlock(&share->intern_lock);
+#endif
     }
     my_free(tables, MYF(0));
   }
   trn->used_tables= 0;
-  return error;
+  DBUG_RETURN(error);
 }
 
 
@@ -421,11 +439,18 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit,
    @notes
      This is used when we unlock a table from a group of locked tables
      just before doing a rename or drop table.
+
+     share->internal_lock must be locked when function is called
 */
 
 void _ma_remove_table_from_trnman(MARIA_SHARE *share, TRN *trn)
 {
   MARIA_USED_TABLES *tables, **prev;
+  DBUG_ENTER("_ma_remove_table_from_trnman");
+  DBUG_PRINT("enter", ("share: 0x%lx  in_trans: %d",
+                       (ulong) share, share->in_trans));
+
+  safe_mutex_assert_owner(&share->intern_lock);
   
   for (prev= (MARIA_USED_TABLES**) &trn->used_tables, tables= *prev;
        tables;
@@ -434,11 +459,14 @@ void _ma_remove_table_from_trnman(MARIA_SHARE *share, TRN *trn)
     if (tables->share == share)
     {
       *prev= tables->next;
+      share->in_trans--;
+      DBUG_PRINT("info", ("in_trans: %d", share->in_trans));
       my_free(tables, MYF(0));
+      break;
     }
-    else
-      prev= &tables->next;
+    prev= &tables->next;
   }
+  DBUG_VOID_RETURN;
 }