#At bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-maria/
2660 Michael Widenius 2008-08-19
Fixes for Bug #38016 Maria: trying to access freed memory when committing a transaction
Don't write out states if they haven't changed
modified:
sql/sql_base.cc
sql/sql_table.cc
storage/maria/ha_maria.cc
storage/maria/ma_close.c
storage/maria/ma_extra.c
storage/maria/ma_open.c
storage/maria/ma_state.c
per-file messages:
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
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2008-04-28 16:24:05 +0000
+++ b/sql/sql_base.cc 2008-08-18 22:21:22 +0000
@@ -2184,6 +2184,7 @@ void unlink_open_table(THD *thd, TABLE *
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 *ta
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
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);
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2008-06-28 12:45:15 +0000
+++ b/sql/sql_table.cc 2008-08-18 22:21:22 +0000
@@ -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);
}
=== modified file 'storage/maria/ha_maria.cc'
--- a/storage/maria/ha_maria.cc 2008-07-11 14:33:47 +0000
+++ b/storage/maria/ha_maria.cc 2008-08-18 22:21:22 +0000
@@ -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_functi
{
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 *n
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;
=== modified file 'storage/maria/ma_close.c'
--- a/storage/maria/ma_close.c 2008-07-05 11:03:21 +0000
+++ b/storage/maria/ma_close.c 2008-08-18 22:21:22 +0000
@@ -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))
=== modified file 'storage/maria/ma_extra.c'
--- a/storage/maria/ma_extra.c 2008-07-12 14:14:28 +0000
+++ b/storage/maria/ma_extra.c 2008-08-18 22:21:22 +0000
@@ -326,7 +326,7 @@ int maria_extra(MARIA_HA *info, enum ha_
{
_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_
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
=== modified file 'storage/maria/ma_open.c'
--- a/storage/maria/ma_open.c 2008-07-09 12:07:38 +0000
+++ b/storage/maria/ma_open.c 2008-08-18 22:21:22 +0000
@@ -1186,6 +1186,7 @@ uint _ma_state_info_write(MARIA_SHARE *s
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;
}
=== modified file 'storage/maria/ma_state.c'
--- a/storage/maria/ma_state.c 2008-07-12 14:14:28 +0000
+++ b/storage/maria/ma_state.c 2008-08-18 22:21:22 +0000
@@ -79,6 +79,9 @@ my_bool _ma_setup_live_state(MARIA_HA *i
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 *t
{
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 *t
/* 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 *t
@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_
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;
}
| Thread |
|---|
| • bzr commit into MySQL/Maria:mysql-maria branch (monty:2660) Bug#38016 | Michael Widenius | 19 Aug |
