mirror of
https://github.com/MariaDB/server.git
synced 2025-07-29 05:21:33 +03:00
Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
Version for 4.0. It fixes two problems: 1. The cause of the bug was that we did not check the table version for the HANDLER ... READ commands. We did not notice when a table was replaced by a new one. This can happen during ALTER TABLE, REPAIR TABLE, and OPTIMIZE TABLE (there might be more cases). I call the fix for this problem "the primary bug fix". 2. mysql_ha_flush() was not always called with a locked LOCK_open. Though the function comment clearly said it must. I changed the code so that the locking is done when required. I call the fix for this problem "the secondary fix".
This commit is contained in:
@ -447,3 +447,21 @@ drop table t2;
|
||||
drop table t3;
|
||||
drop table t4;
|
||||
drop table t5;
|
||||
create table t1 (c1 int);
|
||||
insert into t1 values (1);
|
||||
handler t1 open;
|
||||
handler t1 read first;
|
||||
c1
|
||||
1
|
||||
send the below to another connection, do not wait for the result
|
||||
optimize table t1;
|
||||
proceed with the normal connection
|
||||
handler t1 read next;
|
||||
c1
|
||||
1
|
||||
handler t1 close;
|
||||
read the result from the other connection
|
||||
Table Op Msg_type Msg_text
|
||||
test.t1 optimize status OK
|
||||
proceed with the normal connection
|
||||
drop table t1;
|
||||
|
@ -339,3 +339,32 @@ drop table t2;
|
||||
drop table t3;
|
||||
drop table t4;
|
||||
drop table t5;
|
||||
|
||||
#
|
||||
# Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
|
||||
#
|
||||
create table t1 (c1 int);
|
||||
insert into t1 values (1);
|
||||
# client 1
|
||||
handler t1 open;
|
||||
handler t1 read first;
|
||||
# client 2
|
||||
connect (con2,localhost,root,,);
|
||||
connection con2;
|
||||
--exec echo send the below to another connection, do not wait for the result
|
||||
send optimize table t1;
|
||||
--sleep 1
|
||||
# client 1
|
||||
--exec echo proceed with the normal connection
|
||||
connection default;
|
||||
handler t1 read next;
|
||||
handler t1 close;
|
||||
# client 2
|
||||
--exec echo read the result from the other connection
|
||||
connection con2;
|
||||
reap;
|
||||
# client 1
|
||||
--exec echo proceed with the normal connection
|
||||
connection default;
|
||||
drop table t1;
|
||||
|
||||
|
@ -547,7 +547,8 @@ int mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen= 0);
|
||||
int mysql_ha_close(THD *thd, TABLE_LIST *tables);
|
||||
int mysql_ha_read(THD *, TABLE_LIST *,enum enum_ha_read_modes,char *,
|
||||
List<Item> *,enum ha_rkey_function,Item *,ha_rows,ha_rows);
|
||||
int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags);
|
||||
int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags,
|
||||
bool is_locked);
|
||||
/* mysql_ha_flush mode_flags bits */
|
||||
#define MYSQL_HA_CLOSE_FINAL 0x00
|
||||
#define MYSQL_HA_REOPEN_ON_USAGE 0x01
|
||||
|
@ -390,7 +390,8 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh,
|
||||
thd->proc_info="Flushing tables";
|
||||
|
||||
close_old_data_files(thd,thd->open_tables,1,1);
|
||||
mysql_ha_flush(thd, tables, MYSQL_HA_REOPEN_ON_USAGE | MYSQL_HA_FLUSH_ALL);
|
||||
mysql_ha_flush(thd, tables, MYSQL_HA_REOPEN_ON_USAGE | MYSQL_HA_FLUSH_ALL,
|
||||
TRUE);
|
||||
bool found=1;
|
||||
/* Wait until all threads has closed all the tables we had locked */
|
||||
DBUG_PRINT("info", ("Waiting for others threads to close their open tables"));
|
||||
@ -863,7 +864,7 @@ TABLE *open_table(THD *thd,const char *db,const char *table_name,
|
||||
}
|
||||
|
||||
/* close handler tables which are marked for flush */
|
||||
mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE);
|
||||
mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE, TRUE);
|
||||
|
||||
for (table=(TABLE*) hash_search(&open_cache,(byte*) key,key_length) ;
|
||||
table && table->in_use ;
|
||||
@ -1262,7 +1263,7 @@ bool wait_for_tables(THD *thd)
|
||||
{
|
||||
thd->some_tables_deleted=0;
|
||||
close_old_data_files(thd,thd->open_tables,0,dropping_tables != 0);
|
||||
mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE);
|
||||
mysql_ha_flush(thd, (TABLE_LIST*) NULL, MYSQL_HA_REOPEN_ON_USAGE, TRUE);
|
||||
if (!table_is_used(thd->open_tables,1))
|
||||
break;
|
||||
(void) pthread_cond_wait(&COND_refresh,&LOCK_open);
|
||||
|
@ -217,7 +217,7 @@ void THD::cleanup(void)
|
||||
close_thread_tables(this);
|
||||
}
|
||||
mysql_ha_flush(this, (TABLE_LIST*) 0,
|
||||
MYSQL_HA_CLOSE_FINAL | MYSQL_HA_FLUSH_ALL);
|
||||
MYSQL_HA_CLOSE_FINAL | MYSQL_HA_FLUSH_ALL, FALSE);
|
||||
hash_free(&handler_tables_hash);
|
||||
close_temporary_tables(this);
|
||||
hash_free(&user_vars);
|
||||
|
@ -357,6 +357,7 @@ int mysql_ha_read(THD *thd, TABLE_LIST *tables,
|
||||
ha_rows select_limit,ha_rows offset_limit)
|
||||
{
|
||||
TABLE_LIST *hash_tables;
|
||||
TABLE **table_ptr;
|
||||
TABLE *table;
|
||||
int err;
|
||||
int keyno=-1;
|
||||
@ -379,6 +380,27 @@ int mysql_ha_read(THD *thd, TABLE_LIST *tables,
|
||||
DBUG_PRINT("info-in-hash",("'%s'.'%s' as '%s' tab %p",
|
||||
hash_tables->db, hash_tables->real_name,
|
||||
hash_tables->alias, table));
|
||||
/* Table might have been flushed. */
|
||||
if (table && (table->version != refresh_version))
|
||||
{
|
||||
/*
|
||||
We must follow the thd->handler_tables chain, as we need the
|
||||
address of the 'next' pointer referencing this table
|
||||
for close_thread_table().
|
||||
*/
|
||||
for (table_ptr= &(thd->handler_tables);
|
||||
*table_ptr && (*table_ptr != table);
|
||||
table_ptr= &(*table_ptr)->next)
|
||||
{}
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
if (close_thread_table(thd, table_ptr))
|
||||
{
|
||||
/* Tell threads waiting for refresh that something has happened */
|
||||
VOID(pthread_cond_broadcast(&COND_refresh));
|
||||
}
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
table= hash_tables->table= NULL;
|
||||
}
|
||||
if (!table)
|
||||
{
|
||||
/*
|
||||
@ -593,6 +615,7 @@ err0:
|
||||
MYSQL_HA_REOPEN_ON_USAGE mark for reopen.
|
||||
MYSQL_HA_FLUSH_ALL flush all tables, not only
|
||||
those marked for flush.
|
||||
is_locked If LOCK_open is locked.
|
||||
|
||||
DESCRIPTION
|
||||
The list of HANDLER tables may be NULL, in which case all HANDLER
|
||||
@ -600,7 +623,6 @@ err0:
|
||||
If 'tables' is NULL and MYSQL_HA_FLUSH_ALL is not set,
|
||||
all HANDLER tables marked for flush are closed.
|
||||
Broadcasts a COND_refresh condition, for every table closed.
|
||||
The caller must lock LOCK_open.
|
||||
|
||||
NOTE
|
||||
Since mysql_ha_flush() is called when the base table has to be closed,
|
||||
@ -610,10 +632,12 @@ err0:
|
||||
0 ok
|
||||
*/
|
||||
|
||||
int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
|
||||
int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags,
|
||||
bool is_locked)
|
||||
{
|
||||
TABLE_LIST *tmp_tables;
|
||||
TABLE **table_ptr;
|
||||
bool did_lock= FALSE;
|
||||
DBUG_ENTER("mysql_ha_flush");
|
||||
DBUG_PRINT("enter", ("tables: %p mode_flags: 0x%02x", tables, mode_flags));
|
||||
|
||||
@ -637,6 +661,12 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
|
||||
(*table_ptr)->table_cache_key,
|
||||
(*table_ptr)->real_name,
|
||||
(*table_ptr)->table_name));
|
||||
/* The first time it is required, lock for close_thread_table(). */
|
||||
if (! did_lock && ! is_locked)
|
||||
{
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
did_lock= TRUE;
|
||||
}
|
||||
mysql_ha_flush_table(thd, table_ptr, mode_flags);
|
||||
continue;
|
||||
}
|
||||
@ -655,6 +685,12 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
|
||||
if ((mode_flags & MYSQL_HA_FLUSH_ALL) ||
|
||||
((*table_ptr)->version != refresh_version))
|
||||
{
|
||||
/* The first time it is required, lock for close_thread_table(). */
|
||||
if (! did_lock && ! is_locked)
|
||||
{
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
did_lock= TRUE;
|
||||
}
|
||||
mysql_ha_flush_table(thd, table_ptr, mode_flags);
|
||||
continue;
|
||||
}
|
||||
@ -662,6 +698,10 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
|
||||
}
|
||||
}
|
||||
|
||||
/* Release the lock if it was taken by this function. */
|
||||
if (did_lock)
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
|
||||
@ -693,8 +733,8 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags)
|
||||
table->table_name, mode_flags));
|
||||
|
||||
if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash,
|
||||
(byte*) (*table_ptr)->table_name,
|
||||
strlen((*table_ptr)->table_name) + 1)))
|
||||
(byte*) table->table_name,
|
||||
strlen(table->table_name) + 1)))
|
||||
{
|
||||
if (! (mode_flags & MYSQL_HA_REOPEN_ON_USAGE))
|
||||
{
|
||||
@ -708,6 +748,7 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags)
|
||||
}
|
||||
}
|
||||
|
||||
safe_mutex_assert_owner(&LOCK_open);
|
||||
if (close_thread_table(thd, table_ptr))
|
||||
{
|
||||
/* Tell threads waiting for refresh that something has happened */
|
||||
|
@ -179,7 +179,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
|
||||
{
|
||||
char *db=table->db;
|
||||
uint flags;
|
||||
mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL);
|
||||
mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, TRUE);
|
||||
if (!close_temporary_table(thd, db, table->real_name))
|
||||
{
|
||||
tmp_table_deleted=1;
|
||||
@ -1239,7 +1239,7 @@ static int mysql_admin_table(THD* thd, TABLE_LIST* tables,
|
||||
if (send_fields(thd, field_list, 1))
|
||||
DBUG_RETURN(-1);
|
||||
|
||||
mysql_ha_flush(thd, tables, MYSQL_HA_CLOSE_FINAL);
|
||||
mysql_ha_flush(thd, tables, MYSQL_HA_CLOSE_FINAL, FALSE);
|
||||
for (table = tables; table; table = table->next)
|
||||
{
|
||||
char table_name[NAME_LEN*2+2];
|
||||
@ -1500,7 +1500,7 @@ int mysql_alter_table(THD *thd,char *new_db, char *new_name,
|
||||
}
|
||||
used_fields=create_info->used_fields;
|
||||
|
||||
mysql_ha_flush(thd, table_list, MYSQL_HA_CLOSE_FINAL);
|
||||
mysql_ha_flush(thd, table_list, MYSQL_HA_CLOSE_FINAL, FALSE);
|
||||
if (!(table=open_ltable(thd,table_list,TL_WRITE_ALLOW_READ)))
|
||||
DBUG_RETURN(-1);
|
||||
|
||||
|
Reference in New Issue
Block a user