MDEV-26383 fixup: Consistently protect freed_indexes with autoinc_mutex

To avoid potential race conditions between concurrent access to dict_table_t::freed_indexes, let us consistently use dict_table_t::autoinc_mutex. dict_table_remove_from_cache_low(): To avoid extensive hold time of table->autoinc_mutex, unconditionally free the FTS data structures.
MariaDB · Aug 23, 2021 · ca89489 · ca89489
1 parent 08e5a3d
commit ca89489
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 23 deletions.
diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc
@@ -253,15 +253,7 @@ ATTRIBUTE_COLD static void btr_search_lazy_free(dict_index_t *index)
 {
   ut_ad(index->freed());
   dict_table_t *table= index->table;
-  bool non_exist_table= (table->id == 0);
-
-  if (non_exist_table)
-  {
-    /* autoinc_mutex should be acquired to avoid the race condition
-    in case of multiple threads accessing the evicted table
-    or dropped table. */
-    mysql_mutex_lock(&table->autoinc_mutex);
-  }
+  mysql_mutex_lock(&table->autoinc_mutex);
 
   /* Perform the skipped steps of dict_index_remove_from_cache_low(). */
   UT_LIST_REMOVE(table->freed_indexes, index);
@@ -271,15 +263,14 @@ ATTRIBUTE_COLD static void btr_search_lazy_free(dict_index_t *index)
   if (!UT_LIST_GET_LEN(table->freed_indexes) &&
       !UT_LIST_GET_LEN(table->indexes))
   {
-    ut_ad(non_exist_table);
+    ut_ad(!table->id);
     mysql_mutex_unlock(&table->autoinc_mutex);
     mysql_mutex_destroy(&table->autoinc_mutex);
     dict_mem_table_free(table);
     return;
   }
 
-  if (non_exist_table)
-    mysql_mutex_unlock(&table->autoinc_mutex);
+  mysql_mutex_unlock(&table->autoinc_mutex);
 }
 
 /** Clear the adaptive hash index on all pages in the buffer pool. */

diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc
@@ -2,7 +2,7 @@
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2012, Facebook Inc.
-Copyright (c) 2013, 2020, MariaDB Corporation.
+Copyright (c) 2013, 2021, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -1258,6 +1258,7 @@ dict_index_t *dict_index_t::clone_if_needed()
     return this;
   dict_index_t *prev= UT_LIST_GET_PREV(indexes, this);
 
+  mysql_mutex_lock(&table->autoinc_mutex);
   UT_LIST_REMOVE(table->indexes, this);
   UT_LIST_ADD_LAST(table->freed_indexes, this);
   dict_index_t *index= clone();
@@ -1266,6 +1267,7 @@ dict_index_t *dict_index_t::clone_if_needed()
     UT_LIST_INSERT_AFTER(table->indexes, prev, index);
   else
     UT_LIST_ADD_FIRST(table->indexes, index);
+  mysql_mutex_unlock(&table->autoinc_mutex);
   return index;
 }
 #endif /* BTR_CUR_HASH_ADAPT */
@@ -1961,15 +1963,21 @@ dict_table_remove_from_cache_low(
 	}
 
 #ifdef BTR_CUR_HASH_ADAPT
-	if (UNIV_UNLIKELY(UT_LIST_GET_LEN(table->freed_indexes) != 0)) {
-		if (table->fts) {
-			fts_optimize_remove_table(table);
-			fts_free(table);
-			table->fts = NULL;
-		}
+	if (table->fts) {
+		fts_optimize_remove_table(table);
+		fts_free(table);
+		table->fts = NULL;
+	}
+
+	mysql_mutex_lock(&table->autoinc_mutex);
+
+	ulint freed = UT_LIST_GET_LEN(table->freed_indexes);
+
+	table->vc_templ = NULL;
+	table->id = 0;
+	mysql_mutex_unlock(&table->autoinc_mutex);
 
-		table->vc_templ = NULL;
-		table->id = 0;
+	if (UNIV_UNLIKELY(freed != 0)) {
 		return;
 	}
 #endif /* BTR_CUR_HASH_ADAPT */
@@ -2244,8 +2252,10 @@ dict_index_remove_from_cache_low(
 	zero. See also: dict_table_can_be_evicted() */
 
 	if (index->n_ahi_pages()) {
+		mysql_mutex_lock(&table->autoinc_mutex);
 		index->set_freed();
 		UT_LIST_ADD_LAST(table->freed_indexes, index);
+		mysql_mutex_unlock(&table->autoinc_mutex);
 		return;
 	}
 #endif /* BTR_CUR_HASH_ADAPT */

diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h
@@ -1713,7 +1713,8 @@ struct dict_table_t {
 	UT_LIST_BASE_NODE_T(dict_index_t)	indexes;
 #ifdef BTR_CUR_HASH_ADAPT
 	/** List of detached indexes that are waiting to be freed along with
-	the last adaptive hash index entry */
+	the last adaptive hash index entry.
+	Protected by autoinc_mutex (sic!) */
 	UT_LIST_BASE_NODE_T(dict_index_t)	freed_indexes;
 #endif /* BTR_CUR_HASH_ADAPT */
 
@@ -1880,7 +1881,7 @@ struct dict_table_t {
 	from a select. */
 	lock_t*					autoinc_lock;
 
-	/** Mutex protecting the autoincrement counter. */
+	/** Mutex protecting the autoinc counter and freed_indexes. */
 	mysql_mutex_t				autoinc_mutex;
 
 	/** Autoinc counter value to give to the next inserted row. */