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

10 files changed, 131 insertions, 22 deletions

diff --git a/mysql-test/r/myisam-metadata.result b/mysql-test/r/myisam-metadata.result
new file mode 100644
index 00000000000..5192253d5d1
--- /dev/null
+++ b/mysql-test/r/myisam-metadata.result
@@ -0,0 +1,12 @@
+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;
diff --git a/mysql-test/t/myisam-metadata.test b/mysql-test/t/myisam-metadata.test
new file mode 100644
index 00000000000..c5327aa3a71
--- /dev/null
+++ b/mysql-test/t/myisam-metadata.test
@@ -0,0 +1,49 @@
+#
+# 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;
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index 14737cd69bb..c4bc8c62391 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -7300,7 +7300,7 @@ static int mysql_init_variables(void)
   log_error_file_ptr= log_error_file;
   protocol_version= PROTOCOL_VERSION;
   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;
   global_query_id= thread_id= 1L;
   my_atomic_rwlock_init(&global_query_id_lock);
diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc
index c65e56edbe0..9f6fe9ee62b 100644
--- a/sql/sql_admin.cc
+++ b/sql/sql_admin.cc
@@ -87,6 +87,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
   const char **ext;
   MY_STAT stat_info;
   Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
+                                  MYSQL_OPEN_FOR_REPAIR |
                                   MYSQL_OPEN_HAS_MDL_LOCK |
                                   MYSQL_LOCK_IGNORE_TIMEOUT));
   DBUG_ENTER("prepare_for_repair");
@@ -197,7 +198,8 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
     */
     pos_in_locked_tables= table->pos_in_locked_tables;
     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;
     /* Close table but don't remove from locked list */
     close_all_tables_for_name(thd, table_list->table->s,
@@ -589,8 +591,10 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
     */
     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,
-                                   HA_EXTRA_PREPARE_FOR_RENAME))
+                                   HA_EXTRA_PREPARE_FOR_RENAME,
+                                   TDC_RT_REMOVE_NOT_OWN_AND_MARK_NOT_USABLE))
         goto err;
       DEBUG_SYNC(thd, "after_admin_flush");
       /* Flush entries in the query cache involving this table. */
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index c76f2d43279..327e08ff92f 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -1074,7 +1074,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables,
       if (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,
                          table->table_name, TRUE);
 	found=1;
@@ -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,
-                              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_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,
              table->mdl_ticket, thd->variables.lock_wait_timeout))
     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,
                    FALSE);
   /* extra() call must come only after all instances above are closed */
@@ -3090,7 +3091,9 @@ retry_share:
     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())
     {
@@ -9371,6 +9374,7 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
   TABLE *table;
   TABLE_SHARE *share;
   DBUG_ENTER("tdc_remove_table");
+  DBUG_PRINT("enter",("name: %s  remove_type: %d", table_name, remove_type));
 
   if (! has_lock)
     mysql_mutex_lock(&LOCK_open);
@@ -9396,7 +9400,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
       {
         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);
         while ((table= it2++))
@@ -9407,8 +9412,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type,
       }
 #endif
       /*
-        Set share's version to zero in order to ensure that it gets
-        automatically deleted once it is no longer referenced.
+        Mark share to ensure that it gets automatically deleted once
+        it is no longer referenced.
 
         Note that code in TABLE_SHARE::wait_for_old_version() assumes
         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,
         TDC does not contain old shares which don't have any tables
         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++))
         free_cache_entry(table);
diff --git a/sql/sql_base.h b/sql/sql_base.h
index 45d41777fea..2a39c67b350 100644
--- a/sql/sql_base.h
+++ b/sql/sql_base.h
@@ -60,7 +60,8 @@ enum find_item_error_report_type {REPORT_ALL_ERRORS, REPORT_EXCEPT_NOT_FOUND,
                                   IGNORE_EXCEPT_NON_UNIQUE};
 
 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() */
 #define RTFC_NO_FLAG                0x0000
@@ -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_LOCK_NOT_TEMPORARY		0x2000
+#define MYSQL_OPEN_FOR_REPAIR                   0x4000
 
 /** Please refer to the internals manual. */
 #define MYSQL_OPEN_REOPEN  (MYSQL_OPEN_IGNORE_FLUSH |\
@@ -229,7 +231,9 @@ bool setup_tables_and_check_access(THD *thd,
                                    ulong want_access,
                                    bool full_table_list);
 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,
                      const char *table_name);
diff --git a/sql/sql_table.cc b/sql/sql_table.cc
index 330c28ebbd8..fed05c5902e 100644
--- a/sql/sql_table.cc
+++ b/sql/sql_table.cc
@@ -2190,7 +2190,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
 
       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;
           goto err;
@@ -6237,13 +6238,15 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
     case LEAVE_AS_IS:
       break;
     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;
       DEBUG_SYNC(thd,"alter_table_enable_indexes");
       error= table->file->ha_enable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
       break;
     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;
       error=table->file->ha_disable_indexes(HA_KEY_SWITCH_NONUNIQ_SAVE);
       break;
@@ -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
         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;
       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,
     if (table->s->tmp_table)
     {
       Open_table_context ot_ctx(thd, (MYSQL_OPEN_IGNORE_FLUSH |
+                                      MYSQL_OPEN_FOR_REPAIR |
                                       MYSQL_LOCK_IGNORE_TIMEOUT));
       TABLE_LIST tbl;
       bzero((void*) &tbl, sizeof(tbl));
@@ -6776,7 +6781,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
         table->file->indexes_are_disabled())
       need_lock_for_indexes= true;
     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;
     thd_proc_info(thd, "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,
   if (lower_case_table_names)
     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)
     {
diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc
index 4b77344c042..d47fb24eaba 100644
--- a/sql/sql_truncate.cc
+++ b/sql/sql_truncate.cc
@@ -364,7 +364,8 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref,
   {
     DEBUG_SYNC(thd, "upgrade_lock_for_truncate");
     /* 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);
     m_ticket_downgrade= table->mdl_ticket;
     /* Close if table is going to be recreated. */
diff --git a/sql/table.cc b/sql/table.cc
index fba45f421ed..4215f667618 100644
--- a/sql/table.cc
+++ b/sql/table.cc
@@ -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.length= path_length;
 
-    share->version=       refresh_version;
+    share->set_refresh_version();
 
     /*
       Since alloc_table_share() can be called without any locking (for
diff --git a/sql/table.h b/sql/table.h
index c21b01f53f7..eaea0fe65e5 100644
--- a/sql/table.h
+++ b/sql/table.h
@@ -793,12 +793,33 @@ struct TABLE_SHARE
     return table_map_id;
   }
 
-
   /** Is this table share being expelled from the table definition cache?  */
   inline bool has_old_version() const
   {
     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
     representing its type.