1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash

Browse Source 
Version for 5.0.
It fixes three 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".
3. In 5.0 (not in 4.1 or 4.0) DROP TABLE had a possible deadlock flaw in
 concur with FLUSH TABLES WITH READ LOCK. I call the fix for this
 problem "the 5.0 addendum fix".


include/my_pthread.h:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  Added a new macro for the 5.0 addendum fix.
mysql-test/r/handler.result:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  The test result.
mysql-test/t/handler.test:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  The test case.
sql/lock.cc:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  Changed a comment which did confuse me and which is not fully
  correct anymore after the 5.0 addendum fix.
  Added an assertion which would fire without the 5.0 addendum fix.
sql/mysql_priv.h:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  Changed a definition for the secondary fix.
sql/sql_base.cc:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  Changed function calls for the secondary fix.
sql/sql_class.cc:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  Changed a function call for the secondary fix.
sql/sql_handler.cc:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  The first two diffs make the primary bug fix.
  The rest is for the secondary fix.
sql/sql_table.cc:
  Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
  The first diff (four changed places) make the 5.0 addendum fix.
  The other three are changed function calls for the secondary fix.
This commit is contained in:
unknown 2005-11-15 21:57:02 +01:00
parent d9c7aaf23f
commit 013b3d8ab3
9 changed files with 200 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
include/my_pthread.h
View File

@ -536,9 +536,15 @@ void safe_mutex_end(FILE *file);
#define pthread_cond_timedwait(A,B,C) safe_cond_timedwait((A),(B),(C),__FILE__,__LINE__)
#define pthread_mutex_trylock(A) pthread_mutex_lock(A)
#define pthread_mutex_t safe_mutex_t
#define safe_mutex_assert_owner(mp) DBUG_ASSERT((mp)->count > 0 && pthread_equal(pthread_self(),(mp)->thread))
#define safe_mutex_assert_owner(mp) \
DBUG_ASSERT((mp)->count > 0 && \
pthread_equal(pthread_self(), (mp)->thread))
#define safe_mutex_assert_not_owner(mp) \
DBUG_ASSERT(! (mp)->count || \
! pthread_equal(pthread_self(), (mp)->thread))
#else
#define safe_mutex_assert_owner(mp)
#define safe_mutex_assert_not_owner(mp)
#endif /* SAFE_MUTEX */
/* READ-WRITE thread locking */

37
mysql-test/r/handler.result
View File

@ -445,3 +445,40 @@ 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;
create table t1 (c1 int);
insert into t1 values (14397);
flush tables with read lock;
drop table t1;
ERROR HY000: Can't execute the query because you have a conflicting read lock
send the below to another connection, do not wait for the result
drop table t1;
proceed with the normal connection
select * from t1;
c1
14397
unlock tables;
read the result from the other connection
proceed with the normal connection
select * from t1;
ERROR 42S02: Table 'test.t1' doesn't exist
drop table if exists t1;
Warnings:
Note 1051 Unknown table 't1'

75
mysql-test/t/handler.test
View File

@ -347,4 +347,79 @@ 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;
# End of 4.1 tests
#
# Addendum to Bug#14397 - OPTIMIZE TABLE with an open HANDLER causes a crash
# Show that DROP TABLE can no longer deadlock against
# FLUSH TABLES WITH READ LOCK. This is a 5.0 issue.
#
create table t1 (c1 int);
insert into t1 values (14397);
flush tables with read lock;
# The thread with the global read lock cannot drop the table itself:
--error 1223
drop table t1;
#
# client 2
# We need a second connection to try the drop.
# The drop waits for the global read lock to go away.
# Without the addendum fix it locked LOCK_open before entering the wait loop.
connection con2;
--exec echo send the below to another connection, do not wait for the result
send drop table t1;
--sleep 1
#
# client 1
# Now we need something that wants LOCK_open. A simple table access which
# opens the table does the trick.
--exec echo proceed with the normal connection
connection default;
# This would hang on LOCK_open without the 5.0 addendum fix.
select * from t1;
# Release the read lock. This should make the DROP go through.
unlock tables;
#
# client 2
# Read the result of the drop command.
connection con2;
--exec echo read the result from the other connection
reap;
#
# client 1
# Now back to normal operation. The table should not exist any more.
--exec echo proceed with the normal connection
connection default;
--error 1146
select * from t1;
# Just to be sure and not confuse the next test case writer.
drop table if exists t1;

18
sql/lock.cc
View File

@ -815,10 +815,13 @@ static void print_lock_error(int error, const char *table)
access to them is protected with a mutex LOCK_global_read_lock
(XXX: one should never take LOCK_open if LOCK_global_read_lock is taken,
otherwise a deadlock may occur - see mysql_rm_table. Other mutexes could
be a problem too - grep the code for global_read_lock if you want to use
any other mutex here)
(XXX: one should never take LOCK_open if LOCK_global_read_lock is
taken, otherwise a deadlock may occur. Other mutexes could be a
problem too - grep the code for global_read_lock if you want to use
any other mutex here) Also one must not hold LOCK_open when calling
wait_if_global_read_lock(). When the thread with the global read lock
tries to close its tables, it needs to take LOCK_open in
close_thread_table().
How blocking of threads by global read lock is achieved: that's
advisory. Any piece of code which should be blocked by global read lock must
@ -937,6 +940,13 @@ bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh,
DBUG_ENTER("wait_if_global_read_lock");
LINT_INIT(old_message);
/*
Assert that we do not own LOCK_open. If we would own it, other
threads could not close their tables. This would make a pretty
deadlock.
*/
safe_mutex_assert_not_owner(&LOCK_open);
(void) pthread_mutex_lock(&LOCK_global_read_lock);
if ((need_exit_cond= must_wait))
{

3
sql/mysql_priv.h
View File

@ -890,7 +890,8 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen);
bool mysql_ha_close(THD *thd, TABLE_LIST *tables);
bool 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

7
sql/sql_base.cc
View File

@ -311,7 +311,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",
@ -1238,7 +1239,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
/* close handler tables which are marked for flush */
if (thd->handler_tables)
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 ;
@ -1642,7 +1643,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);

2
sql/sql_class.cc
View File

@ -376,7 +376,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);
delete_dynamic(&user_var_events);
hash_free(&user_vars);

50
sql/sql_handler.cc
View File

@ -336,6 +336,7 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
ha_rows select_limit_cnt, ha_rows offset_limit_cnt)
{
TABLE_LIST *hash_tables;
TABLE **table_ptr;
TABLE *table;
MYSQL_LOCK *lock;
List<Item> list;
@ -368,6 +369,28 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables,
DBUG_PRINT("info-in-hash",("'%s'.'%s' as '%s' tab %p",
hash_tables->db, hash_tables->table_name,
hash_tables->alias, table));
/* Table might have been flushed. */
if (table && (table->s->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)
{}
(*table_ptr)->file->ha_index_or_rnd_end();
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)
{
/*
@ -594,6 +617,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
@ -601,7 +625,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,
@ -611,10 +634,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));
@ -640,6 +665,12 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
(*table_ptr)->s->db,
(*table_ptr)->s->table_name,
(*table_ptr)->alias));
/* 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;
}
@ -658,6 +689,12 @@ int mysql_ha_flush(THD *thd, TABLE_LIST *tables, uint mode_flags)
if ((mode_flags & MYSQL_HA_FLUSH_ALL) ||
((*table_ptr)->s->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;
}
@ -665,6 +702,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);
}
@ -696,8 +737,8 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags)
table->alias, mode_flags));
if ((hash_tables= (TABLE_LIST*) hash_search(&thd->handler_tables_hash,
(byte*) (*table_ptr)->alias,
strlen((*table_ptr)->alias) + 1)))
(byte*) table->alias,
strlen(table->alias) + 1)))
{
if (! (mode_flags & MYSQL_HA_REOPEN_ON_USAGE))
{
@ -712,6 +753,7 @@ static int mysql_ha_flush_table(THD *thd, TABLE **table_ptr, uint mode_flags)
}
(*table_ptr)->file->ha_index_or_rnd_end();
safe_mutex_assert_owner(&LOCK_open);
if (close_thread_table(thd, table_ptr))
{
/* Tell threads waiting for refresh that something has happened */

23
sql/sql_table.cc
View File

@ -103,23 +103,28 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists,
/* mark for close and remove all cached entries */
thd->mysys_var->current_mutex= &LOCK_open;
thd->mysys_var->current_cond= &COND_refresh;
VOID(pthread_mutex_lock(&LOCK_open));
if (!drop_temporary)
{
if ((error= wait_if_global_read_lock(thd, 0, 1)))
{
my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), tables->table_name);
goto err;
DBUG_RETURN(TRUE);
}
else
need_start_waiters= TRUE;
}
/*
Acquire LOCK_open after wait_if_global_read_lock(). If we would hold
LOCK_open during wait_if_global_read_lock(), other threads could not
close their tables. This would make a pretty deadlock.
*/
thd->mysys_var->current_mutex= &LOCK_open;
thd->mysys_var->current_cond= &COND_refresh;
VOID(pthread_mutex_lock(&LOCK_open));
error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
err:
pthread_mutex_unlock(&LOCK_open);
pthread_mutex_lock(&thd->mysys_var->mutex);
@ -232,7 +237,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
char *db=table->db;
db_type table_type= DB_TYPE_UNKNOWN;
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->table_name))
{
tmp_table_deleted=1;
@ -2171,7 +2176,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF))
DBUG_RETURN(TRUE);
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_local)
{
char table_name[NAME_LEN*2+2];
@ -3128,7 +3133,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
new_db= db;
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);
/* DISCARD/IMPORT TABLESPACE is always alone in an ALTER TABLE */
if (alter_info->tablespace_op != NO_TABLESPACE_OP)
DBUG_RETURN(mysql_discard_or_import_tablespace(thd,table_list,