Commit ea31b1e6 authored by Michael Widenius's avatar Michael Widenius

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
parent 47a82ad5
......@@ -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);
......
......@@ -7185,7 +7185,7 @@ err:
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 @@ err:
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);
}
......
......@@ -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;
......
......@@ -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))
......
......@@ -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
......
......@@ -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;
}
......
......@@ -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;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment