Commit 8ed283d8 authored by Michael Widenius's avatar Michael Widenius

Fixed bug MPDEV-628 / LP:989055 - Querying myisam table metadata may corrupt the table.

The issue was that there was that SHOW commands could open the table in the store engine, even in cases
where it should not be allowed to do that (ie, the storage engines meta data for that table was under big changes).

The cases where this should not be allowed are:
- ALTER TABLE DISABLE KEYS
- ALTER TABLE ENABLE KEYS
- REPAIR TABLE
- OPTIMIZE TABLE
- DROP TABLE

This patch adds a new mode, protected_against_usage(). If this is used then the SHOW command will wait until the table
is accessable. This is implemented by re-using the already exising 'version' flag for TABLE_SHARE.
It also added functions to be used to change TABLE_SHARE->version instead of changing it directly.
	


mysql-test/r/myisam-metadata.result:
  Added test case
mysql-test/t/myisam-metadata.test:
  Added test case
sql/mysqld.cc:
  Start from refresh_version 2 as 0 and 1 are reserved.
sql/sql_admin.cc:
  Added MYSQL_OPEN_FOR_REPAIR
  Updated call to wait_while_table_is_used()
sql/sql_base.cc:
  Updated call to wait_while_table_is_used()
  - Allow one to specify how the table should be removed (for all commands except show or for all commands).
  - Don't allow one to reopen the table if one has called share->protect_against_usage()
sql/sql_base.h:
  Added TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE, which is used to mark that no one can reopen this table, except with MYSQL_OPEN_FOR_REPAIR .
  - Added MYSQL_OPEN_FOR_REPAIR
  - Updated prototype for wait_while_table_is_used()
sql/sql_table.cc:
  Updated call to wait_while_table_is_used()
  Use MYSQL_OPEN_FOR_REPAIR for open tables that where repaired.
sql/sql_truncate.cc:
  Updated call to wait_while_table_is_used()
sql/table.cc:
  Use set_refresh_version()
sql/table.h:
  Added functions to be used to change TABLE_SHARE->version instead of changing it directly
parent b917fdb9
DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (
id INT PRIMARY KEY,
a VARCHAR(100),
INDEX(a)
) ENGINE=MyISAM;
ALTER TABLE t1 DISABLE KEYS;
ALTER TABLE t1 ENABLE KEYS;
SHOW TABLE STATUS LIKE 't1';
Name Engine Version Row_format Rows Avg_row_length Data_length Max_data_length Index_length Data_free Auto_increment Create_time Update_time Check_time Collation Checksum Create_options Comment
t1 MyISAM 10 Dynamic 100000 27 # # # 0 NULL # # # latin1_swedish_ci NULL
DROP TABLE t1;
#
# Test bugs in MyISAM that may cause problems for metadata
#
--source include/big_test.inc
--disable_warnings
DROP TABLE IF EXISTS t1;
--enable_warnings
#
# LP:989055 - Querying myisam table metadata may corrupt the table
#
CREATE TABLE t1 (
id INT PRIMARY KEY,
a VARCHAR(100),
INDEX(a)
) ENGINE=MyISAM;
ALTER TABLE t1 DISABLE KEYS;
let $1=100000;
--disable_query_log
while ($1)
{
eval insert into t1 values($1, "line number $1");
dec $1;
}
--enable_query_log
--connect(con1,localhost,root,,)
--send
ALTER TABLE t1 ENABLE KEYS;
--connection default
--let $wait_timeout=10
--let $show_statement= SHOW PROCESSLIST
--let $field= State
--let $condition= = 'Repair by sorting'
--source include/wait_show_condition.inc
--replace_column 7 # 8 # 9 # 12 # 13 # 14 #
SHOW TABLE STATUS LIKE 't1';
--connection con1
--reap
--connection default
--disconnect con1
DROP TABLE t1;
...@@ -7300,7 +7300,7 @@ static int mysql_init_variables(void) ...@@ -7300,7 +7300,7 @@ static int mysql_init_variables(void)
log_error_file_ptr= log_error_file; log_error_file_ptr= log_error_file;
protocol_version= PROTOCOL_VERSION; protocol_version= PROTOCOL_VERSION;
what_to_log= ~ (1L << (uint) COM_TIME); what_to_log= ~ (1L << (uint) COM_TIME);
refresh_version= 1L; /* Increments on each reload */ refresh_version= 2L; /* Increments on each reload. 0 and 1 are reserved */
executed_events= 0; executed_events= 0;
global_query_id= thread_id= 1L; global_query_id= thread_id= 1L;
my_atomic_rwlock_init(&global_query_id_lock); my_atomic_rwlock_init(&global_query_id_lock);
......
...@@ -87,6 +87,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, ...@@ -87,6 +87,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
const char **ext; const char **ext;
MY_STAT stat_info; MY_STAT stat_info;
Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH | Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
MYSQL_OPEN_FOR_REPAIR |
MYSQL_OPEN_HAS_MDL_LOCK | MYSQL_OPEN_HAS_MDL_LOCK |
MYSQL_LOCK_IGNORE_TIMEOUT)); MYSQL_LOCK_IGNORE_TIMEOUT));
DBUG_ENTER("prepare_for_repair"); DBUG_ENTER("prepare_for_repair");
...@@ -197,7 +198,8 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, ...@@ -197,7 +198,8 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
*/ */
pos_in_locked_tables= table->pos_in_locked_tables; pos_in_locked_tables= table->pos_in_locked_tables;
if (wait_while_table_is_used(thd, table, if (wait_while_table_is_used(thd, table,
HA_EXTRA_PREPARE_FOR_FORCED_CLOSE)) HA_EXTRA_PREPARE_FOR_FORCED_CLOSE,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto end; goto end;
/* Close table but don't remove from locked list */ /* Close table but don't remove from locked list */
close_all_tables_for_name(thd, table_list->table->s, close_all_tables_for_name(thd, table_list->table->s,
...@@ -589,8 +591,10 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, ...@@ -589,8 +591,10 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
*/ */
if (lock_type == TL_WRITE && !table->table->s->tmp_table) if (lock_type == TL_WRITE && !table->table->s->tmp_table)
{ {
table->table->s->protect_against_usage();
if (wait_while_table_is_used(thd, table->table, if (wait_while_table_is_used(thd, table->table,
HA_EXTRA_PREPARE_FOR_RENAME)) HA_EXTRA_PREPARE_FOR_RENAME,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto err; goto err;
DEBUG_SYNC(thd, "after_admin_flush"); DEBUG_SYNC(thd, "after_admin_flush");
/* Flush entries in the query cache involving this table. */ /* Flush entries in the query cache involving this table. */
......
...@@ -1074,7 +1074,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, ...@@ -1074,7 +1074,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables,
if (share) if (share)
{ {
kill_delayed_threads_for_table(share); kill_delayed_threads_for_table(share);
/* tdc_remove_table() also sets TABLE_SHARE::version to 0. */ /* tdc_remove_table() calls share->remove_from_cache_at_close() */
tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, table->db, tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, table->db,
table->table_name, TRUE); table->table_name, TRUE);
found=1; found=1;
...@@ -2333,7 +2333,8 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db, ...@@ -2333,7 +2333,8 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db,
*/ */
bool wait_while_table_is_used(THD *thd, TABLE *table, bool wait_while_table_is_used(THD *thd, TABLE *table,
enum ha_extra_function function) enum ha_extra_function function,
enum_tdc_remove_table_type remove_type)
{ {
DBUG_ENTER("wait_while_table_is_used"); DBUG_ENTER("wait_while_table_is_used");
DBUG_PRINT("enter", ("table: '%s' share: 0x%lx db_stat: %u version: %lu", DBUG_PRINT("enter", ("table: '%s' share: 0x%lx db_stat: %u version: %lu",
...@@ -2344,7 +2345,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table, ...@@ -2344,7 +2345,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table,
table->mdl_ticket, thd->variables.lock_wait_timeout)) table->mdl_ticket, thd->variables.lock_wait_timeout))
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN, tdc_remove_table(thd, remove_type,
table->s->db.str, table->s->table_name.str, table->s->db.str, table->s->table_name.str,
FALSE); FALSE);
/* extra() call must come only after all instances above are closed */ /* extra() call must come only after all instances above are closed */
...@@ -3090,7 +3091,9 @@ retry_share: ...@@ -3090,7 +3091,9 @@ retry_share:
goto err_unlock; goto err_unlock;
} }
if (!(flags & MYSQL_OPEN_IGNORE_FLUSH)) if (!(flags & MYSQL_OPEN_IGNORE_FLUSH) ||
(share->protected_against_usage() &&
!(flags & MYSQL_OPEN_FOR_REPAIR)))
{ {
if (share->has_old_version()) if (share->has_old_version())
{ {
...@@ -9371,6 +9374,7 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, ...@@ -9371,6 +9374,7 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
TABLE *table; TABLE *table;
TABLE_SHARE *share; TABLE_SHARE *share;
DBUG_ENTER("tdc_remove_table"); DBUG_ENTER("tdc_remove_table");
DBUG_PRINT("enter",("name: %s remove_type: %d", table_name, remove_type));
if (! has_lock) if (! has_lock)
mysql_mutex_lock(&LOCK_open); mysql_mutex_lock(&LOCK_open);
...@@ -9396,7 +9400,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, ...@@ -9396,7 +9400,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
{ {
DBUG_ASSERT(share->used_tables.is_empty()); DBUG_ASSERT(share->used_tables.is_empty());
} }
else if (remove_type == TDC_RT_REMOVE_NOT_OWN) else if (remove_type == TDC_RT_REMOVE_NOT_OWN ||
remove_type == TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE)
{ {
I_P_List_iterator<TABLE, TABLE_share> it2(share->used_tables); I_P_List_iterator<TABLE, TABLE_share> it2(share->used_tables);
while ((table= it2++)) while ((table= it2++))
...@@ -9407,8 +9412,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, ...@@ -9407,8 +9412,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
} }
#endif #endif
/* /*
Set share's version to zero in order to ensure that it gets Mark share to ensure that it gets automatically deleted once
automatically deleted once it is no longer referenced. it is no longer referenced.
Note that code in TABLE_SHARE::wait_for_old_version() assumes Note that code in TABLE_SHARE::wait_for_old_version() assumes
that marking share as old and removal of its unused tables that marking share as old and removal of its unused tables
...@@ -9417,7 +9422,13 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, ...@@ -9417,7 +9422,13 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
TDC does not contain old shares which don't have any tables TDC does not contain old shares which don't have any tables
used. used.
*/ */
share->version= 0; if (remove_type == TDC_RT_REMOVE_NOT_OWN)
share->remove_from_cache_at_close();
else
{
/* Ensure that no can open the table while it's used */
share->protect_against_usage();
}
while ((table= it++)) while ((table= it++))
free_cache_entry(table); free_cache_entry(table);
......
...@@ -60,7 +60,8 @@ enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND, ...@@ -60,7 +60,8 @@ enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND,
IGNORE_EXCEPT_NON_UNIQUE}; IGNORE_EXCEPT_NON_UNIQUE};
enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN, enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN,
TDC_RT_REMOVE_UNUSED}; TDC_RT_REMOVE_UNUSED,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE};
/* bits for last argument to remove_table_from_cache() */ /* bits for last argument to remove_table_from_cache() */
#define RTFC_NO_FLAG 0x0000 #define RTFC_NO_FLAG 0x0000
...@@ -128,6 +129,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update, ...@@ -128,6 +129,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update,
*/ */
#define MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK 0x1000 #define MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK 0x1000
#define MYSQL_LOCK_NOT_TEMPORARY 0x2000 #define MYSQL_LOCK_NOT_TEMPORARY 0x2000
#define MYSQL_OPEN_FOR_REPAIR 0x4000
/** Please refer to the internals manual. */ /** Please refer to the internals manual. */
#define MYSQL_OPEN_REOPEN (MYSQL_OPEN_IGNORE_FLUSH |\ #define MYSQL_OPEN_REOPEN (MYSQL_OPEN_IGNORE_FLUSH |\
...@@ -229,7 +231,9 @@ bool setup_tables_and_check_access(THD *thd, ...@@ -229,7 +231,9 @@ bool setup_tables_and_check_access(THD *thd,
ulong want_access, ulong want_access,
bool full_table_list); bool full_table_list);
bool wait_while_table_is_used(THD *thd, TABLE *table, bool wait_while_table_is_used(THD *thd, TABLE *table,
enum ha_extra_function function); enum ha_extra_function function,
enum_tdc_remove_table_type remove_type=
TDC_RT_REMOVE_NOT_OWN);
void drop_open_table(THD *thd, TABLE *table, const char *db_name, void drop_open_table(THD *thd, TABLE *table, const char *db_name,
const char *table_name); const char *table_name);
......
...@@ -2190,7 +2190,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, ...@@ -2190,7 +2190,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
if (thd->locked_tables_mode) if (thd->locked_tables_mode)
{ {
if (wait_while_table_is_used(thd, table->table, HA_EXTRA_NOT_USED)) if (wait_while_table_is_used(thd, table->table, HA_EXTRA_NOT_USED,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
{ {
error= -1; error= -1;
goto err; goto err;
...@@ -6237,13 +6238,15 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6237,13 +6238,15 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
case LEAVE_AS_IS: case LEAVE_AS_IS:
break; break;
case ENABLE: case ENABLE:
if (wait_while_table_is_used(thd, table, extra_func)) if (wait_while_table_is_used(thd, table, extra_func,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto err; goto err;
DEBUG_SYNC(thd,"alter_table_enable_indexes"); DEBUG_SYNC(thd,"alter_table_enable_indexes");
error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE); error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
break; break;
case DISABLE: case DISABLE:
if (wait_while_table_is_used(thd, table, extra_func)) if (wait_while_table_is_used(thd, table, extra_func,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto err; goto err;
error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE); error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
break; break;
...@@ -6271,7 +6274,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6271,7 +6274,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
simple rename did nothing and therefore we can safely return simple rename did nothing and therefore we can safely return
without additional clean-up. without additional clean-up.
*/ */
if (wait_while_table_is_used(thd, table, extra_func)) if (wait_while_table_is_used(thd, table, extra_func,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto err; goto err;
close_all_tables_for_name(thd, table->s, HA_EXTRA_PREPARE_FOR_RENAME); close_all_tables_for_name(thd, table->s, HA_EXTRA_PREPARE_FOR_RENAME);
/* /*
...@@ -6704,6 +6708,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6704,6 +6708,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
if (table->s->tmp_table) if (table->s->tmp_table)
{ {
Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH | Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
MYSQL_OPEN_FOR_REPAIR |
MYSQL_LOCK_IGNORE_TIMEOUT)); MYSQL_LOCK_IGNORE_TIMEOUT));
TABLE_LIST tbl; TABLE_LIST tbl;
bzero((void*) &tbl, sizeof(tbl)); bzero((void*) &tbl, sizeof(tbl));
...@@ -6776,7 +6781,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6776,7 +6781,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
table->file->indexes_are_disabled()) table->file->indexes_are_disabled())
need_lock_for_indexes= true; need_lock_for_indexes= true;
if (!table->s->tmp_table && need_lock_for_indexes && if (!table->s->tmp_table && need_lock_for_indexes &&
wait_while_table_is_used(thd, table, extra_func)) wait_while_table_is_used(thd, table, extra_func,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
goto err_new_table_cleanup; goto err_new_table_cleanup;
thd_proc_info(thd, "manage keys"); thd_proc_info(thd, "manage keys");
DEBUG_SYNC(thd, "alter_table_manage_keys"); DEBUG_SYNC(thd, "alter_table_manage_keys");
...@@ -6996,7 +7002,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6996,7 +7002,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
if (lower_case_table_names) if (lower_case_table_names)
my_casedn_str(files_charset_info, old_name); my_casedn_str(files_charset_info, old_name);
if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME)) if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
{ {
if (pending_inplace_add_index) if (pending_inplace_add_index)
{ {
......
...@@ -364,7 +364,8 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref, ...@@ -364,7 +364,8 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref,
{ {
DEBUG_SYNC(thd, "upgrade_lock_for_truncate"); DEBUG_SYNC(thd, "upgrade_lock_for_truncate");
/* To remove the table from the cache we need an exclusive lock. */ /* To remove the table from the cache we need an exclusive lock. */
if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_DROP)) if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_DROP,
TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
m_ticket_downgrade= table->mdl_ticket; m_ticket_downgrade= table->mdl_ticket;
/* Close if table is going to be recreated. */ /* Close if table is going to be recreated. */
......
...@@ -323,7 +323,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key, ...@@ -323,7 +323,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key,
share->normalized_path.str= share->path.str; share->normalized_path.str= share->path.str;
share->normalized_path.length= path_length; share->normalized_path.length= path_length;
share->version= refresh_version; share->set_refresh_version();
/* /*
Since alloc_table_share() can be called without any locking (for Since alloc_table_share() can be called without any locking (for
......
...@@ -793,12 +793,33 @@ struct TABLE_SHARE ...@@ -793,12 +793,33 @@ struct TABLE_SHARE
return table_map_id; return table_map_id;
} }
/** Is this table share being expelled from the table definition cache? */ /** Is this table share being expelled from the table definition cache? */
inline bool has_old_version() const inline bool has_old_version() const
{ {
return version != refresh_version; return version != refresh_version;
} }
inline bool protected_against_usage() const
{
return version == 0;
}
inline void protect_against_usage()
{
version= 0;
}
/*
Remove from table definition cache at close.
Table can still be opened by SHOW
*/
inline void remove_from_cache_at_close()
{
if (version != 0) /* Don't remove protection */
version= 1;
}
inline void set_refresh_version()
{
version= refresh_version;
}
/** /**
Convert unrelated members of TABLE_SHARE to one enum Convert unrelated members of TABLE_SHARE to one enum
representing its type. representing its type.
......
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