MDEV-31049 fil_delete_tablespace() returns wrong file handle if tablespace was...

MDEV-31049 fil_delete_tablespace() returns wrong file handle if tablespace was closed by parallel thread

fil_delete_tablespace() stores file handle in local variable and calls
mtr_t::commit_file()=>fil_system_t::detach(..., detach_handle=true), which
sets space->chain.start->handle = OS_FILE_CLOSED. fil_system_t::detach()
is invoked under fil_system.mutex.

But before the mutex is acquired some parallel thread can change
space->chain.start->handle. fil_delete_tablespace() returns value, stored
in local variable, i.e. wrong value.

File handle can be closed, for example, from buf_flush_space() when the
limit of innodb_open_files exceded and fil_space_t::get() causes
fil_space_t::try_to_close() call.

fil_space_t::try_to_close() is executed under fil_system.mutex. And
mtr_t::commit_file() locks it for fil_system_t::detach() call.
fil_system_t::detach() returns detached file handle if its argument
detach_handle is true. The fix is to let mtr_t::commit_file() to pass
that detached file handle to fil_delete_tablespace().

storage/innobase/fil/fil0fil.cc

@@ -1687,9 +1687,7 @@ pfs_os_file_t fil_delete_tablespace(ulint id)
     mtr_t mtr;
     mtr.start();
     mtr.log_file_op(FILE_DELETE, id, space->chain.start->name);
-    handle= space->chain.start->handle;
-    mtr.commit_file(*space, nullptr);
-
+    mtr.commit_file(*space, nullptr, &handle);
     fil_space_free_low(space);
   }
 

storage/innobase/include/mtr0mtr.h

@@ -93,10 +93,16 @@ struct mtr_t {
   ATTRIBUTE_COLD void commit_shrink(fil_space_t &space);
 
   /** Commit a mini-transaction that is deleting or renaming a file.
-  @param space   tablespace that is being renamed or deleted
-  @param name    new file name (nullptr=the file will be deleted)
+  @param space           tablespace that is being renamed or deleted
+  @param name            new file name (nullptr=the file will be deleted)
+  @param detached_handle if detached_handle != nullptr and if space is detached
+                         during the function execution the file handle if its
+                         node will be set to OS_FILE_CLOSED, and the previous
+                         value of the file handle will be assigned to the
+                         address, pointed by detached_handle.
   @return whether the operation succeeded */
-  ATTRIBUTE_COLD bool commit_file(fil_space_t &space, const char *name);
+  ATTRIBUTE_COLD bool commit_file(fil_space_t &space, const char *name,
+      pfs_os_file_t *detached_handle= nullptr);
 
   /** Commit a mini-transaction that did not modify any pages,
   but generated some redo log on a higher level, such as

storage/innobase/mtr/mtr0mtr.cc

@@ -333,8 +333,14 @@ void mtr_t::commit_shrink(fil_space_t &space)
 /** Commit a mini-transaction that is deleting or renaming a file.
 @param space   tablespace that is being renamed or deleted
 @param name    new file name (nullptr=the file will be deleted)
+@param detached_handle if detached_handle != nullptr and if space is detached
+                       during the function execution the file handle if its
+                       node will be set to OS_FILE_CLOSED, and the previous
+                       value of the file handle will be assigned to the
+                       address, pointed by detached_handle.
 @return whether the operation succeeded */
-bool mtr_t::commit_file(fil_space_t &space, const char *name)
+bool mtr_t::commit_file(fil_space_t &space, const char *name,
+    pfs_os_file_t *detached_handle)
 {
   ut_ad(is_active());
   ut_ad(!is_inside_ibuf());
@@ -402,7 +408,9 @@ bool mtr_t::commit_file(fil_space_t &space, const char *name)
     ut_ad(!space.referenced());
     ut_ad(space.is_stopping());
 
-    fil_system.detach(&space, true);
+    pfs_os_file_t handle = fil_system.detach(&space, true);
+    if (detached_handle)
+      *detached_handle = handle;
     mysql_mutex_unlock(&fil_system.mutex);
 
     success= true;