MDEV-31080 fil_validate() failures during deferred tablespace recovery

fil_space_t::create(), fil_space_t::add(): Expect the caller to
acquire and release fil_system.mutex. In this way, creating a tablespace
and adding the first (usually only) data file will be atomic.

recv_sys_t::recover_deferred(): Correctly protect some changes by
holding fil_system.mutex.

Tested by: Matthias Leich

5 files changed, 52 insertions, 35 deletions

diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 19ebdc8d67e..48d205f428a 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -311,6 +311,8 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,
 			     uint32_t size, bool is_raw, bool atomic_write,
 			     uint32_t max_pages)
 {
+	mysql_mutex_assert_owner(&fil_system.mutex);
+
 	fil_node_t*	node;
 
 	ut_ad(name != NULL);
@@ -335,7 +337,6 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,
 
 	node->atomic_write = atomic_write;
 
-	mysql_mutex_lock(&fil_system.mutex);
 	this->size += size;
 	UT_LIST_ADD_LAST(chain, node);
 	if (node->is_open()) {
@@ -346,7 +347,6 @@ fil_node_t* fil_space_t::add(const char* name, pfs_os_file_t handle,
 			release();
 		}
 	}
-	mysql_mutex_unlock(&fil_system.mutex);
 
 	return node;
 }
@@ -946,6 +946,7 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
 {
 	fil_space_t*	space;
 
+	mysql_mutex_assert_owner(&fil_system.mutex);
 	ut_ad(fil_system.is_initialised());
 	ut_ad(fil_space_t::is_valid_flags(flags & ~FSP_FLAGS_MEM_MASK, id));
 	ut_ad(srv_page_size == UNIV_PAGE_SIZE_ORIG || flags != 0);
@@ -978,8 +979,6 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
 
 	space->latch.SRW_LOCK_INIT(fil_space_latch_key);
 
-	mysql_mutex_lock(&fil_system.mutex);
-
 	if (const fil_space_t *old_space = fil_space_get_by_id(id)) {
 		ib::error() << "Trying to add tablespace with id " << id
 			    << " to the cache, but tablespace '"
@@ -987,7 +986,6 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
 				? old_space->chain.start->name
 				: "")
 			    << "' already exists in the cache!";
-		mysql_mutex_unlock(&fil_system.mutex);
 		space->~fil_space_t();
 		ut_free(space);
 		return(NULL);
@@ -1034,12 +1032,12 @@ fil_space_t *fil_space_t::create(ulint id, ulint flags,
 	if (rotate) {
 		fil_system.default_encrypt_tables.push_back(*space);
 		space->is_in_default_encrypt = true;
-	}
 
-	mysql_mutex_unlock(&fil_system.mutex);
-
-	if (rotate && srv_n_fil_crypt_threads_started) {
-		fil_crypt_threads_signal();
+		if (srv_n_fil_crypt_threads_started) {
+			mysql_mutex_unlock(&fil_system.mutex);
+			fil_crypt_threads_signal();
+			mysql_mutex_lock(&fil_system.mutex);
+		}
 	}
 
 	return(space);
@@ -1998,16 +1996,20 @@ err_exit:
 	DBUG_EXECUTE_IF("checkpoint_after_file_create",
 			log_make_checkpoint(););
 
+	mysql_mutex_lock(&fil_system.mutex);
 	if (fil_space_t* space = fil_space_t::create(space_id, flags,
 						     FIL_TYPE_TABLESPACE,
 						     crypt_data, mode, true)) {
 		fil_node_t* node = space->add(path, file, size, false, true);
+		mysql_mutex_unlock(&fil_system.mutex);
 		IF_WIN(node->find_metadata(), node->find_metadata(file, true));
 		mtr.start();
 		mtr.set_named_space(space);
 		ut_a(fsp_header_init(space, size, &mtr) == DB_SUCCESS);
 		mtr.commit();
 		return space;
+	} else {
+		mysql_mutex_unlock(&fil_system.mutex);
 	}
 
 	if (space_name.data()) {
@@ -2267,8 +2269,10 @@ skip_validate:
 					    first_page)
 		: NULL;
 
+	mysql_mutex_lock(&fil_system.mutex);
 	space = fil_space_t::create(id, flags, purpose, crypt_data);
 	if (!space) {
+		mysql_mutex_unlock(&fil_system.mutex);
 		goto error;
 	}
 
@@ -2278,6 +2282,7 @@ skip_validate:
 	space->add(
 		df_remote.is_open() ? df_remote.filepath() :
 		df_default.filepath(), OS_FILE_CLOSED, 0, false, true);
+	mysql_mutex_unlock(&fil_system.mutex);
 
 	if (must_validate && !srv_read_only_mode) {
 		df_remote.close();
@@ -2566,10 +2571,13 @@ tablespace_check:
 		return FIL_LOAD_INVALID;
 	}
 
+	mysql_mutex_lock(&fil_system.mutex);
+
 	space = fil_space_t::create(
 		space_id, flags, FIL_TYPE_TABLESPACE, crypt_data);
 
 	if (space == NULL) {
+		mysql_mutex_unlock(&fil_system.mutex);
 		return(FIL_LOAD_INVALID);
 	}
 
@@ -2581,6 +2589,7 @@ tablespace_check:
 	let fil_node_open() do that task. */
 
 	space->add(file.filepath(), OS_FILE_CLOSED, 0, false, false);
+	mysql_mutex_unlock(&fil_system.mutex);
 
 	return(FIL_LOAD_OK);
 }
diff --git a/storage/innobase/fsp/fsp0space.cc b/storage/innobase/fsp/fsp0space.cc
index b069250ff9f..8c6344946a4 100644
--- a/storage/innobase/fsp/fsp0space.cc
+++ b/storage/innobase/fsp/fsp0space.cc
@@ -88,25 +88,25 @@ Tablespace::open_or_create(bool is_temp)
 	ut_ad(!m_files.empty());
 
 	for (iterator it = begin(); it != end(); ++it) {
-
 		if (it->m_exists) {
 			err = it->open_or_create(
 				m_ignore_read_only
 				? false : srv_read_only_mode);
+			if (err != DB_SUCCESS) {
+				return err;
+			}
 		} else {
 			err = it->open_or_create(
 				m_ignore_read_only
 				? false : srv_read_only_mode);
 
-			/* Set the correct open flags now that we have
-			successfully created the file. */
-			if (err == DB_SUCCESS) {
-				file_found(*it);
+			if (err != DB_SUCCESS) {
+				return err;
 			}
-		}
 
-		if (err != DB_SUCCESS) {
-			break;
+			/* Set the correct open flags now that we have
+			successfully created the file. */
+			file_found(*it);
 		}
 
 		/* We can close the handle now and open the tablespace
@@ -130,20 +130,22 @@ Tablespace::open_or_create(bool is_temp)
 				fsp_flags = FSP_FLAGS_PAGE_SSIZE();
 			}
 
+			mysql_mutex_lock(&fil_system.mutex);
 			space = fil_space_t::create(
 				m_space_id, fsp_flags,
 				is_temp
 				? FIL_TYPE_TEMPORARY : FIL_TYPE_TABLESPACE,
 				NULL);
 			if (!space) {
+				mysql_mutex_unlock(&fil_system.mutex);
 				return DB_ERROR;
 			}
+		} else {
+			mysql_mutex_lock(&fil_system.mutex);
 		}
-
-		ut_a(fil_validate());
-
 		space->add(it->m_filepath, OS_FILE_CLOSED, it->m_size,
 			   false, true);
+		mysql_mutex_unlock(&fil_system.mutex);
 	}
 
 	return(err);
diff --git a/storage/innobase/fsp/fsp0sysspace.cc b/storage/innobase/fsp/fsp0sysspace.cc
index 497e4100557..577db8eb9f2 100644
--- a/storage/innobase/fsp/fsp0sysspace.cc
+++ b/storage/innobase/fsp/fsp0sysspace.cc
@@ -921,6 +921,7 @@ SysTablespace::open_or_create(
 	/* Close the curent handles, add space and file info to the
 	fil_system cache and the Data Dictionary, and re-open them
 	in file_system cache so that they stay open until shutdown. */
+	mysql_mutex_lock(&fil_system.mutex);
 	ulint	node_counter = 0;
 	for (files_t::iterator it = begin; it != end; ++it) {
 		it->close();
@@ -934,7 +935,8 @@ SysTablespace::open_or_create(
 				FIL_TYPE_TEMPORARY, NULL);
 			ut_ad(space == fil_system.temp_space);
 			if (!space) {
-				return DB_ERROR;
+				err = DB_ERROR;
+				break;
 			}
 			ut_ad(!space->is_compressed());
 			ut_ad(space->full_crc32());
@@ -945,12 +947,11 @@ SysTablespace::open_or_create(
 				FIL_TYPE_TABLESPACE, NULL);
 			ut_ad(space == fil_system.sys_space);
 			if (!space) {
-				return DB_ERROR;
+				err = DB_ERROR;
+				break;
 			}
 		}
 
-		ut_a(fil_validate());
-
 		uint32_t max_size = (++node_counter == m_files.size()
 				    ? (m_last_file_size_max == 0
 				       ? UINT32_MAX
@@ -961,6 +962,7 @@ SysTablespace::open_or_create(
 			   it->m_type != SRV_NOT_RAW, true, max_size);
 	}
 
+	mysql_mutex_unlock(&fil_system.mutex);
 	return(err);
 }
 
diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc
index 3b6e3008a95..f87ff15c393 100644
--- a/storage/innobase/log/log0recv.cc
+++ b/storage/innobase/log/log0recv.cc
@@ -785,9 +785,10 @@ retry:
             if (!os_file_status(name->c_str(), &exists, &ftype) || !exists)
               goto processed;
           }
-          create(it, *name, static_cast<uint32_t>
-                 (1U << FSP_FLAGS_FCRC32_POS_MARKER |
-                  FSP_FLAGS_FCRC32_PAGE_SSIZE()), nullptr, 0);
+          if (create(it, *name, static_cast<uint32_t>
+                     (1U << FSP_FLAGS_FCRC32_POS_MARKER |
+                      FSP_FLAGS_FCRC32_PAGE_SSIZE()), nullptr, 0))
+            mysql_mutex_unlock(&fil_system.mutex);
         }
       }
       else
@@ -816,7 +817,7 @@ processed:
   @param flags      FSP_SPACE_FLAGS
   @param crypt_data encryption metadata
   @param size       tablespace size in pages
-  @return tablespace
+  @return tablespace; the caller must release fil_system.mutex
   @retval nullptr   if crypt_data is invalid */
   static fil_space_t *create(const recv_spaces_t::const_iterator &it,
                              const std::string &name, uint32_t flags,
@@ -828,6 +829,7 @@ processed:
       ut_free(crypt_data);
       return nullptr;
     }
+    mysql_mutex_lock(&fil_system.mutex);
     fil_space_t *space= fil_space_t::create(it->first, flags,
                                             FIL_TYPE_TABLESPACE, crypt_data);
     ut_ad(space);
@@ -900,12 +902,13 @@ processed:
         space->free_limit= fsp_header_get_field(page, FSP_FREE_LIMIT);
         space->free_len= flst_get_len(FSP_HEADER_OFFSET + FSP_FREE + page);
         fil_node_t *node= UT_LIST_GET_FIRST(space->chain);
+        mysql_mutex_unlock(&fil_system.mutex);
         if (!space->acquire())
-	{
+        {
 free_space:
           fil_space_free(it->first, false);
           goto next_item;
-	}
+        }
         if (os_file_write(IORequestWrite, node->name, node->handle,
                           page, 0, fil_space_t::physical_size(flags)) !=
             DB_SUCCESS)
@@ -975,6 +978,7 @@ bool recv_sys_t::recover_deferred(recv_sys_t::map::iterator &p,
       space->free_len= flst_get_len(FSP_HEADER_OFFSET + FSP_FREE + page);
       fil_node_t *node= UT_LIST_GET_FIRST(space->chain);
       node->deferred= true;
+      mysql_mutex_unlock(&fil_system.mutex);
       if (!space->acquire())
         goto release_and_fail;
       fil_names_dirty(space);
@@ -998,8 +1002,10 @@ bool recv_sys_t::recover_deferred(recv_sys_t::map::iterator &p,
           uint32_t(file_size / fil_space_t::physical_size(flags));
         if (n_pages > size)
         {
+          mysql_mutex_lock(&fil_system.mutex);
           space->size= node->size= n_pages;
           space->set_committed_size();
+          mysql_mutex_unlock(&fil_system.mutex);
           goto size_set;
         }
       }
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index a1368c5146c..707804f2206 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -559,14 +559,12 @@ err_exit:
 
   fil_set_max_space_id_if_bigger(space_id);
 
+  mysql_mutex_lock(&fil_system.mutex);
   fil_space_t *space= fil_space_t::create(space_id, fsp_flags,
 					  FIL_TYPE_TABLESPACE, nullptr,
 					  FIL_ENCRYPTION_DEFAULT, true);
-  ut_a(fil_validate());
-  ut_a(space);
-
+  ut_ad(space);
   fil_node_t *file= space->add(name, fh, 0, false, true);
-  mysql_mutex_lock(&fil_system.mutex);
 
   if (create)
   {