Fixed lp:925377 "Querying myisam table metadata while 'alter table..enable keys' is running may corrupt the table"

Fixed wrong mutex order bug in Aria when flush_log_for_bitmap() was called when table is not yet marked for change.

include/my_base.h:
  Added flag that table is opened only for status
mysql-test/r/myisam-big.result:
  Test case for lp:925377
mysql-test/t/myisam-big.test:
  Test case for lp:925377
sql/sql_base.cc:
  If thd->version == 0 (happens only when we are opening a table that is flushed under  MYSQL_LOCK_IGNORE_FLUSH), open the table in HA_OPEN_FOR_STATUS mode
storage/maria/ma_bitmap.c:
  Fixed wrong mutex order bug in Aria when flush_log_for_bitmap() was called when table is not yet marked for change.
storage/maria/ma_dbug.c:
  Ignore last_version <= 1 as these are either flushed or only opened for status
storage/maria/ma_open.c:
  Use last_version=1 as a marker that table was opened with HA_OPEN_FOR_STATUS.
  In this case we just open a new version of the table in read only mode.
storage/myisam/mi_create.c:
  Update prototype
storage/myisam/mi_dbug.c:
  Ignore last_version <= 1 as these are either flushed or only opened for status
storage/myisam/mi_open.c:
  Use last_version=1 as a marker that table was opened with HA_OPEN_FOR_STATUS.
  If HA_OPEN_FOR_STATUS is used, we will not assert if there is an old not-to-be-used version of the table existing.
  In this case we just open a new version of the table in read only mode.
storage/myisam/myisamdef.h:
  Updated prototype

11 files changed, 143 insertions, 16 deletions

diff --git a/include/my_base.h b/include/my_base.h
index 361c0aa0b00..16f8803d2d5 100644
--- a/include/my_base.h
+++ b/include/my_base.h
@@ -54,6 +54,7 @@
 /* Internal temp table, used for temporary results */
 #define HA_OPEN_INTERNAL_TABLE          512
 #define HA_OPEN_MERGE_TABLE		1024
+#define HA_OPEN_FOR_STATUS		2048
 
 /* The following is parameter to ha_rkey() how to use key */
 
diff --git a/mysql-test/r/myisam-big.result b/mysql-test/r/myisam-big.result
new file mode 100644
index 00000000000..95a6e91d766
--- /dev/null
+++ b/mysql-test/r/myisam-big.result
@@ -0,0 +1,40 @@
+drop table if exists t1,t2;
+create table t1 (id int, sometext varchar(100)) engine=myisam;
+insert into t1 values (1, "hello"),(2, "hello2"),(4, "hello3"),(4, "hello4");
+create table t2 like t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+select count(*) from t1;
+count(*)
+131072
+alter table t1 add index (id), add index(sometext), add index(sometext,id);
+alter table t1 disable keys;
+alter table t1 enable keys;
+drop table t1,t2;
diff --git a/mysql-test/t/myisam-big.test b/mysql-test/t/myisam-big.test
new file mode 100644
index 00000000000..2fec2450ecd
--- /dev/null
+++ b/mysql-test/t/myisam-big.test
@@ -0,0 +1,64 @@
+#
+# Test bugs in the MyISAM code that require more space/time
+--source include/big_test.inc
+
+# Initialise
+--disable_warnings
+drop table if exists t1,t2;
+--enable_warnings
+
+#
+# BUG#925377:
+# Querying myisam table metadata while 'alter table..enable keys' is
+# running may corrupt the table
+#
+create table t1 (id int, sometext varchar(100)) engine=myisam;
+insert into t1 values (1, "hello"),(2, "hello2"),(4, "hello3"),(4, "hello4");
+create table t2 like t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+insert into t2 select * from t1;
+insert into t1 select * from t1;
+select count(*) from t1;
+connect (con2,localhost,root,,);
+connection con2;
+alter table t1 add index (id), add index(sometext), add index(sometext,id);
+alter table t1 disable keys;
+send alter table t1 enable keys;
+connection default;
+--sleep 1
+--disable_query_log
+--disable_result_log
+show table status like 't1';
+--enable_query_log
+--enable_result_log
+connection con2;
+reap;
+disconnect con2;
+connection default;
+drop table t1,t2;
diff --git a/sql/sql_base.cc b/sql/sql_base.cc
index 95a149741de..877c3dfa82e 100644
--- a/sql/sql_base.cc
+++ b/sql/sql_base.cc
@@ -2985,7 +2985,9 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
     }
 
     error= open_unireg_entry(thd, table, table_list, alias, key, key_length,
-                             mem_root, (flags & OPEN_VIEW_NO_PARSE));
+                             mem_root,
+                             (flags & (OPEN_VIEW_NO_PARSE |
+                                       MYSQL_LOCK_IGNORE_FLUSH)));
     if (error > 0)
     {
       my_free((uchar*)table, MYF(0));
@@ -4074,8 +4076,11 @@ retry:
                                       HA_GET_INDEX | HA_TRY_READ_ONLY),
                               READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD |
                               (flags & OPEN_VIEW_NO_PARSE),
-                              thd->open_options, entry, table_list,
-                              mem_root);
+                              thd->open_options |
+                              (thd->version == 0 &&
+                               (flags & MYSQL_LOCK_IGNORE_FLUSH) ?
+                               HA_OPEN_FOR_STATUS : 0),
+                              entry, table_list, mem_root);
     if (error)
       goto err;
     /* TODO: Don't free this */
@@ -4113,7 +4118,11 @@ retry:
                                                HA_TRY_READ_ONLY),
                                        (READ_KEYINFO | COMPUTE_TYPES |
                                         EXTRA_RECORD),
-                                       thd->open_options, entry, FALSE)))
+                                       thd->open_options |
+                                       (thd->version == 0 &&
+                                        (flags & MYSQL_LOCK_IGNORE_FLUSH) ?
+                                        HA_OPEN_FOR_STATUS : 0),
+                                       entry, FALSE)))
   {
     if (error == 7)                             // Table def changed
     {
diff --git a/storage/maria/ma_bitmap.c b/storage/maria/ma_bitmap.c
index 25ab5c6531e..f5cfa1ce23f 100644
--- a/storage/maria/ma_bitmap.c
+++ b/storage/maria/ma_bitmap.c
@@ -368,7 +368,7 @@ static inline void _ma_bitmap_mark_file_changed(MARIA_SHARE *share,
     if (flush_translog && share->now_transactional)
       (void) translog_flush(share->state.logrec_file_id);
 
-    _ma_mark_file_changed(share);
+    _ma_mark_file_changed_now(share);
     pthread_mutex_lock(&share->bitmap.bitmap_lock);
     /* purecov: end */
   }
diff --git a/storage/maria/ma_dbug.c b/storage/maria/ma_dbug.c
index af90a108e2a..8391f1a9d9d 100644
--- a/storage/maria/ma_dbug.c
+++ b/storage/maria/ma_dbug.c
@@ -186,7 +186,7 @@ my_bool _ma_check_table_is_closed(const char *name, const char *where)
     MARIA_SHARE *share= info->s;
     if (!strcmp(share->unique_file_name.str, filename))
     {
-      if (share->last_version)
+      if (share->last_version > 1)
       {
 	fprintf(stderr,"Warning:  Table: %s is open on %s\n", name,where);
 	DBUG_PRINT("warning",("Table: %s is open on %s", name,where));
diff --git a/storage/maria/ma_open.c b/storage/maria/ma_open.c
index 0b5ee2feb85..5ff4c02a2b5 100644
--- a/storage/maria/ma_open.c
+++ b/storage/maria/ma_open.c
@@ -62,7 +62,8 @@ MARIA_HA *_ma_test_if_reopen(const char *filename)
   {
     MARIA_HA *info=(MARIA_HA*) pos->data;
     MARIA_SHARE *share= info->s;
-    if (!strcmp(share->unique_file_name.str,filename) && share->last_version)
+    if (!strcmp(share->unique_file_name.str,filename) &&
+        share->last_version > 1)
       return info;
   }
   return 0;
@@ -840,7 +841,12 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags)
     share->base.key_parts=key_parts;
     share->base.all_key_parts=key_parts+unique_key_parts;
     if (!(share->last_version=share->state.version))
-      share->last_version=1;			/* Safety */
+      share->last_version= 2;			/* Safety */
+    if (open_flags & HA_OPEN_FOR_STATUS)
+    {
+      share->last_version= 1;                  /* Not reusable version */
+      share->options|= HA_OPTION_READ_ONLY_DATA;
+    }
     share->rec_reflength=share->base.rec_reflength; /* May be changed */
     share->base.margin_key_file_length=(share->base.max_key_file_length -
 					(keys ? MARIA_INDEX_BLOCK_MARGIN *
diff --git a/storage/myisam/mi_create.c b/storage/myisam/mi_create.c
index 21b683d0d4d..919f0f3e040 100644
--- a/storage/myisam/mi_create.c
+++ b/storage/myisam/mi_create.c
@@ -643,7 +643,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
     NOTE: The filename is compared against unique_file_name of every
     open table. Hence we need a real path here.
   */
-  if (test_if_reopen(filename))
+  if (test_if_reopen(filename, 1))
   {
     my_printf_error(0, "MyISAM table '%s' is in use "
                     "(most likely by a MERGE table). Try FLUSH TABLES.",
diff --git a/storage/myisam/mi_dbug.c b/storage/myisam/mi_dbug.c
index 61c52b454a0..559fa8710a0 100644
--- a/storage/myisam/mi_dbug.c
+++ b/storage/myisam/mi_dbug.c
@@ -183,7 +183,7 @@ my_bool check_table_is_closed(const char *name, const char *where)
     MYISAM_SHARE *share=info->s;
     if (!strcmp(share->unique_file_name,filename))
     {
-      if (share->last_version)
+      if (share->last_version > 1)
       {
 	fprintf(stderr,"Warning:  Table: %s is open on %s\n", name,where);
 	DBUG_PRINT("warning",("Table: %s is open on %s", name,where));
diff --git a/storage/myisam/mi_open.c b/storage/myisam/mi_open.c
index 744059d941c..46ec10c064e 100644
--- a/storage/myisam/mi_open.c
+++ b/storage/myisam/mi_open.c
@@ -52,7 +52,8 @@ if (pos > end_pos)             \
 ** In MySQL the server will handle version issues.
 ******************************************************************************/
 
-MI_INFO *test_if_reopen(char *filename)
+MI_INFO *test_if_reopen(char *filename,
+                        my_bool ignore_last_version __attribute__((unused)))
 {
   LIST *pos;
 
@@ -61,8 +62,8 @@ MI_INFO *test_if_reopen(char *filename)
     MI_INFO *info=(MI_INFO*) pos->data;
     MYISAM_SHARE *share=info->s;
     DBUG_ASSERT(strcmp(share->unique_file_name,filename) ||
-                share->last_version);
-    if (!strcmp(share->unique_file_name,filename) && share->last_version)
+                share->last_version > 1 || ignore_last_version);
+    if (!strcmp(share->unique_file_name,filename) && share->last_version > 1)
       return info;
   }
   return 0;
@@ -109,7 +110,8 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
   }
 
   pthread_mutex_lock(&THR_LOCK_myisam);
-  if (!(old_info=test_if_reopen(name_buff)))
+  if (!(old_info=test_if_reopen(name_buff,
+                                test(open_flags & HA_OPEN_FOR_STATUS))))
   {
     share= &share_buff;
     bzero((uchar*) &share_buff,sizeof(share_buff));
@@ -511,7 +513,12 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
     share->base.key_parts=key_parts;
     share->base.all_key_parts=key_parts+unique_key_parts;
     if (!(share->last_version=share->state.version))
-      share->last_version=1;			/* Safety */
+      share->last_version= 2;			/* Safety */
+    if (open_flags & HA_OPEN_FOR_STATUS)
+    {
+      share->last_version= 1;                  /* Not reusable version */
+      share->options|= HA_OPTION_READ_ONLY_DATA;
+    }
     share->rec_reflength=share->base.rec_reflength; /* May be changed */
     share->base.margin_key_file_length=(share->base.max_key_file_length -
 					(keys ? MI_INDEX_BLOCK_MARGIN *
diff --git a/storage/myisam/myisamdef.h b/storage/myisam/myisamdef.h
index 617ad5e8382..58d69afbea8 100644
--- a/storage/myisam/myisamdef.h
+++ b/storage/myisam/myisamdef.h
@@ -726,7 +726,7 @@ my_bool mi_check_status(void *param);
 void mi_fix_status(MI_INFO *org_table, MI_INFO *new_table);
 void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows);
 
-extern MI_INFO *test_if_reopen(char *filename);
+extern MI_INFO *test_if_reopen(char *filename, my_bool ignore_last_version);
 my_bool check_table_is_closed(const char *name, const char *where);
 int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name,
                      File file_to_dup);