bugfix: remove my_delete_with_symlink()

it was race condition prone. instead use either a pair of my_delete()
calls with already resolved paths, or a safe high-level function
my_handler_delete_with_symlink(), like MyISAM and Aria already do.

include/my_sys.h

@@ -578,7 +578,6 @@ extern int my_realpath(char *to, const char *filename, myf MyFlags);
 extern File my_create_with_symlink(const char *linkname, const char *filename,
 				   int createflags, int access_flags,
 				   myf MyFlags);
-extern int my_delete_with_symlink(const char *name, myf MyFlags);
 extern int my_rename_with_symlink(const char *from,const char *to,myf MyFlags);
 extern int my_symlink(const char *content, const char *linkname, myf MyFlags);
 extern int my_handler_delete_with_symlink(PSI_file_key key, const char *name,

include/mysql/psi/mysql_file.h

@@ -434,20 +434,6 @@
   inline_mysql_file_create_with_symlink(P1, P2, P3, P4, P5)
 #endif
 
-/**
-  @def mysql_file_delete_with_symlink(K, P1, P2)
-  Instrumented delete with symbolic link.
-  @c mysql_file_delete_with_symlink is a replacement
-  for @c my_delete_with_symlink.
-*/
-#ifdef HAVE_PSI_INTERFACE
-  #define mysql_file_delete_with_symlink(K, P1, P2) \
-  inline_mysql_file_delete_with_symlink(K, __FILE__, __LINE__, P1, P2)
-#else
-  #define mysql_file_delete_with_symlink(K, P1, P2) \
-  inline_mysql_file_delete_with_symlink(P1, P2)
-#endif
-
 /**
   @def mysql_file_rename_with_symlink(K, P1, P2, P3)
   Instrumented rename with symbolic link.
@@ -1348,33 +1334,6 @@ inline_mysql_file_create_with_symlink(
   return file;
 }
 
-static inline int
-inline_mysql_file_delete_with_symlink(
-#ifdef HAVE_PSI_INTERFACE
-  PSI_file_key key, const char *src_file, uint src_line,
-#endif
-  const char *name, myf flags)
-{
-  int result;
-#ifdef HAVE_PSI_INTERFACE
-  struct PSI_file_locker *locker= NULL;
-  PSI_file_locker_state state;
-  if (likely(PSI_server != NULL))
-  {
-    locker= PSI_server->get_thread_file_name_locker(&state, key, PSI_FILE_DELETE,
-                                                    name, &locker);
-    if (likely(locker != NULL))
-      PSI_server->start_file_wait(locker, (size_t) 0, src_file, src_line);
-  }
-#endif
-  result= my_delete_with_symlink(name, flags);
-#ifdef HAVE_PSI_INTERFACE
-  if (likely(locker != NULL))
-    PSI_server->end_file_wait(locker, (size_t) 0);
-#endif
-  return result;
-}
-
 static inline int
 inline_mysql_file_rename_with_symlink(
 #ifdef HAVE_PSI_INTERFACE

mysys/my_symlink2.c

@@ -91,27 +91,6 @@ File my_create_with_symlink(const char *linkname, const char *filename,
   DBUG_RETURN(file);
 }
 
-/*
-  If the file was a symlink, delete both symlink and the file which the
-  symlink pointed to.
-*/
-
-int my_delete_with_symlink(const char *name, myf MyFlags)
-{
-  char link_name[FN_REFLEN];
-  int was_symlink= (!my_disable_symlinks &&
-		    !my_readlink(link_name, name, MYF(0)));
-  int result;
-  DBUG_ENTER("my_delete_with_symlink");
-
-  if (!(result=my_delete(name, MyFlags)))
-  {
-    if (was_symlink)
-      result=my_delete(link_name, MyFlags);
-  }
-  DBUG_RETURN(result);
-}
-
 /*
   If the file is a normal file, just rename it.
   If the file is a symlink:

sql/handler.cc

@@ -3378,12 +3378,10 @@ int handler::delete_table(const char *name)
   int saved_error= 0;
   int error= 0;
   int enoent_or_zero= ENOENT;                   // Error if no file was deleted
-  char buff[FN_REFLEN];
 
   for (const char **ext=bas_ext(); *ext ; ext++)
   {
-    fn_format(buff, name, "", *ext, MY_UNPACK_FILENAME|MY_APPEND_EXT);
-    if (mysql_file_delete_with_symlink(key_file_misc, buff, MYF(0)))
+    if (my_handler_delete_with_symlink(key_file_misc, name, *ext, 0))
     {
       if (my_errno != ENOENT)
       {

sql/sql_db.cc

@@ -1083,9 +1083,9 @@ static bool find_db_tables_and_rm_known_files(THD *thd, MY_DIR *dirp,
       strxmov(filePath, path, "/", file->name, NullS);
       /*
         We ignore ENOENT error in order to skip files that was deleted
-        by concurrently running statement like REAPIR TABLE ...
+        by concurrently running statement like REPAIR TABLE ...
       */
-      if (my_delete_with_symlink(filePath, MYF(0)) &&
+      if (my_handler_delete_with_symlink(key_file_misc, filePath, "", MYF(0)) &&
           my_errno != ENOENT)
       {
         my_error(EE_DELETE, MYF(0), filePath, my_errno);
@@ -1206,7 +1206,7 @@ long mysql_rm_arc_files(THD *thd, MY_DIR *dirp, const char *org_path)
       continue;
     }
     strxmov(filePath, org_path, "/", file->name, NullS);
-    if (mysql_file_delete_with_symlink(key_file_misc, filePath, MYF(MY_WME)))
+    if (my_handler_delete_with_symlink(key_file_misc, filePath, "", MYF(MY_WME)))
     {
       goto err;
     }

storage/maria/ma_create.c

@@ -52,7 +52,8 @@ int maria_create(const char *name, enum data_file_type datafile_type,
        unique_key_parts,fulltext_keys,offset, not_block_record_extra_length;
   uint max_field_lengths, extra_header_size, column_nr;
   ulong reclength, real_reclength,min_pack_length;
-  char filename[FN_REFLEN], linkname[FN_REFLEN], *linkname_ptr;
+  char kfilename[FN_REFLEN], klinkname[FN_REFLEN], *klinkname_ptr;
+  char dfilename[FN_REFLEN], dlinkname[FN_REFLEN], *dlinkname_ptr;
   ulong pack_reclength;
   ulonglong tot_length,max_rows, tmp;
   enum en_fieldtype type;
@@ -805,19 +806,19 @@ int maria_create(const char *name, enum data_file_type datafile_type,
       /* chop off the table name, tempory tables use generated name */
       if ((path= strrchr(ci->index_file_name, FN_LIBCHAR)))
         *path= '\0';
-      fn_format(filename, name, ci->index_file_name, MARIA_NAME_IEXT,
+      fn_format(kfilename, name, ci->index_file_name, MARIA_NAME_IEXT,
                 MY_REPLACE_DIR | MY_UNPACK_FILENAME |
                 MY_RETURN_REAL_PATH | MY_APPEND_EXT);
     }
     else
     {
-      fn_format(filename, ci->index_file_name, "", MARIA_NAME_IEXT,
+      fn_format(kfilename, ci->index_file_name, "", MARIA_NAME_IEXT,
                 MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH |
                 (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT));
     }
-    fn_format(linkname, name, "", MARIA_NAME_IEXT,
+    fn_format(klinkname, name, "", MARIA_NAME_IEXT,
               MY_UNPACK_FILENAME|MY_APPEND_EXT);
-    linkname_ptr= linkname;
+    klinkname_ptr= klinkname;
     /*
       Don't create the table if the link or file exists to ensure that one
       doesn't accidently destroy another table.
@@ -831,10 +832,10 @@ int maria_create(const char *name, enum data_file_type datafile_type,
   {
     char *iext= strrchr(name, '.');
     int have_iext= iext && !strcmp(iext, MARIA_NAME_IEXT);
-    fn_format(filename, name, "", MARIA_NAME_IEXT,
+    fn_format(kfilename, name, "", MARIA_NAME_IEXT,
               MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH |
               (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT));
-    linkname_ptr= NullS;
+    klinkname_ptr= NullS;
     /*
       Replace the current file.
       Don't sync dir now if the data file has the same path.
@@ -854,7 +855,7 @@ int maria_create(const char *name, enum data_file_type datafile_type,
     NOTE: The filename is compared against unique_file_name of every
     open table. Hence we need a real path here.
   */
-  if (_ma_test_if_reopen(filename))
+  if (_ma_test_if_reopen(kfilename))
   {
     my_printf_error(HA_ERR_TABLE_EXIST, "Aria table '%s' is in use "
                     "(most likely by a MERGE table). Try FLUSH TABLES.",
@@ -863,8 +864,8 @@ int maria_create(const char *name, enum data_file_type datafile_type,
     goto err;
   }
 
-  if ((file= mysql_file_create_with_symlink(key_file_kfile, linkname_ptr,
-                                            filename, 0, create_mode,
+  if ((file= mysql_file_create_with_symlink(key_file_kfile, klinkname_ptr,
+                                            kfilename, 0, create_mode,
                                             MYF(MY_WME|create_flag))) < 0)
     goto err;
   errpos=1;
@@ -1118,30 +1119,30 @@ int maria_create(const char *name, enum data_file_type datafile_type,
         /* chop off the table name, tempory tables use generated name */
         if ((path= strrchr(ci->data_file_name, FN_LIBCHAR)))
           *path= '\0';
-        fn_format(filename, name, ci->data_file_name, MARIA_NAME_DEXT,
+        fn_format(dfilename, name, ci->data_file_name, MARIA_NAME_DEXT,
                   MY_REPLACE_DIR | MY_UNPACK_FILENAME | MY_APPEND_EXT);
       }
       else
       {
-        fn_format(filename, ci->data_file_name, "", MARIA_NAME_DEXT,
+        fn_format(dfilename, ci->data_file_name, "", MARIA_NAME_DEXT,
                   MY_UNPACK_FILENAME |
                   (have_dext ? MY_REPLACE_EXT : MY_APPEND_EXT));
       }
-      fn_format(linkname, name, "",MARIA_NAME_DEXT,
+      fn_format(dlinkname, name, "",MARIA_NAME_DEXT,
                 MY_UNPACK_FILENAME | MY_APPEND_EXT);
-      linkname_ptr= linkname;
+      dlinkname_ptr= dlinkname;
       create_flag=0;
     }
     else
     {
-      fn_format(filename,name,"", MARIA_NAME_DEXT,
+      fn_format(dfilename,name,"", MARIA_NAME_DEXT,
                 MY_UNPACK_FILENAME | MY_APPEND_EXT);
-      linkname_ptr= NullS;
+      dlinkname_ptr= NullS;
       create_flag= (flags & HA_CREATE_KEEP_FILES) ? 0 : MY_DELETE_OLD;
     }
     if ((dfile=
-         mysql_file_create_with_symlink(key_file_dfile, linkname_ptr,
-                                        filename, 0, create_mode,
+         mysql_file_create_with_symlink(key_file_dfile, dlinkname_ptr,
+                                        dfilename, 0, create_mode,
                                         MYF(MY_WME | create_flag | sync_dir))) < 0)
       goto err;
     errpos=3;
@@ -1189,19 +1190,21 @@ int maria_create(const char *name, enum data_file_type datafile_type,
     mysql_file_close(dfile, MYF(0));
     /* fall through */
   case 2:
-  if (! (flags & HA_DONT_TOUCH_DATA))
-    mysql_file_delete_with_symlink(key_file_dfile,
-                                   fn_format(filename,name,"",MARIA_NAME_DEXT,
-                                     MY_UNPACK_FILENAME | MY_APPEND_EXT),
-			   sync_dir);
+    if (! (flags & HA_DONT_TOUCH_DATA))
+    {
+      mysql_file_delete(key_file_dfile, dfilename, MYF(sync_dir));
+      if (dlinkname_ptr)
+        mysql_file_delete(key_file_dfile, dlinkname_ptr, MYF(sync_dir));
+    }
     /* fall through */
   case 1:
     mysql_file_close(file, MYF(0));
     if (! (flags & HA_DONT_TOUCH_DATA))
-      mysql_file_delete_with_symlink(key_file_kfile,
-                                     fn_format(filename,name,"",MARIA_NAME_IEXT,
-                                       MY_UNPACK_FILENAME | MY_APPEND_EXT),
-			     sync_dir);
+    {
+      mysql_file_delete(key_file_kfile, kfilename, MYF(sync_dir));
+      if (klinkname_ptr)
+        mysql_file_delete(key_file_kfile, klinkname_ptr, MYF(sync_dir));
+    }
   }
   my_free(log_data);
   my_free(rec_per_key_part);

storage/myisam/mi_create.c

@@ -45,7 +45,8 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
        max_key_block_length,unique_key_parts,fulltext_keys,offset;
   uint aligned_key_start, block_length, res;
   ulong reclength, real_reclength,min_pack_length;
-  char filename[FN_REFLEN],linkname[FN_REFLEN], *linkname_ptr;
+  char kfilename[FN_REFLEN],klinkname[FN_REFLEN], *klinkname_ptr;
+  char dfilename[FN_REFLEN],dlinkname[FN_REFLEN], *dlinkname_ptr;
   ulong pack_reclength;
   ulonglong tot_length,max_rows, tmp;
   enum en_fieldtype type;
@@ -591,19 +592,19 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
       /* chop off the table name, tempory tables use generated name */
       if ((path= strrchr(ci->index_file_name, FN_LIBCHAR)))
         *path= '\0';
-      fn_format(filename, name, ci->index_file_name, MI_NAME_IEXT,
+      fn_format(kfilename, name, ci->index_file_name, MI_NAME_IEXT,
                 MY_REPLACE_DIR | MY_UNPACK_FILENAME |
                 MY_RETURN_REAL_PATH | MY_APPEND_EXT);
     }
     else
     {
-      fn_format(filename, ci->index_file_name, "", MI_NAME_IEXT,
+      fn_format(kfilename, ci->index_file_name, "", MI_NAME_IEXT,
                 MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH |
                 (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT));
     }
-    fn_format(linkname, name, "", MI_NAME_IEXT,
+    fn_format(klinkname, name, "", MI_NAME_IEXT,
               MY_UNPACK_FILENAME|MY_APPEND_EXT);
-    linkname_ptr=linkname;
+    klinkname_ptr= klinkname;
     /*
       Don't create the table if the link or file exists to ensure that one
       doesn't accidently destroy another table.
@@ -614,10 +615,10 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
   {
     char *iext= strrchr(name, '.');
     int have_iext= iext && !strcmp(iext, MI_NAME_IEXT);
-    fn_format(filename, name, "", MI_NAME_IEXT,
+    fn_format(kfilename, name, "", MI_NAME_IEXT,
               MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH |
               (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT));
-    linkname_ptr=0;
+    klinkname_ptr= 0;
     /* Replace the current file */
     create_flag=(flags & HA_CREATE_KEEP_FILES) ? 0 : MY_DELETE_OLD;
   }
@@ -632,7 +633,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(kfilename))
   {
     my_printf_error(HA_ERR_TABLE_EXIST, "MyISAM table '%s' is in use "
                     "(most likely by a MERGE table). Try FLUSH TABLES.",
@@ -642,7 +643,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
   }
 
   if ((file= mysql_file_create_with_symlink(mi_key_file_kfile,
-                                            linkname_ptr, filename, 0,
+                                            klinkname_ptr, kfilename, 0,
                                             create_mode,
                                             MYF(MY_WME | create_flag))) < 0)
     goto err;
@@ -662,31 +663,31 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
           /* chop off the table name, tempory tables use generated name */
           if ((path= strrchr(ci->data_file_name, FN_LIBCHAR)))
             *path= '\0';
-          fn_format(filename, name, ci->data_file_name, MI_NAME_DEXT,
+          fn_format(dfilename, name, ci->data_file_name, MI_NAME_DEXT,
                     MY_REPLACE_DIR | MY_UNPACK_FILENAME | MY_APPEND_EXT);
         }
         else
         {
-          fn_format(filename, ci->data_file_name, "", MI_NAME_DEXT,
+          fn_format(dfilename, ci->data_file_name, "", MI_NAME_DEXT,
                     MY_UNPACK_FILENAME |
                     (have_dext ? MY_REPLACE_EXT : MY_APPEND_EXT));
         }
 
-	fn_format(linkname, name, "",MI_NAME_DEXT,
+	fn_format(dlinkname, name, "",MI_NAME_DEXT,
 	          MY_UNPACK_FILENAME | MY_APPEND_EXT);
-	linkname_ptr=linkname;
+	dlinkname_ptr= dlinkname;
 	create_flag=0;
       }
       else
       {
-	fn_format(filename,name,"", MI_NAME_DEXT,
+	fn_format(dfilename,name,"", MI_NAME_DEXT,
 	          MY_UNPACK_FILENAME | MY_APPEND_EXT);
-	linkname_ptr=0;
+	dlinkname_ptr= 0;
         create_flag=(flags & HA_CREATE_KEEP_FILES) ? 0 : MY_DELETE_OLD;
       }
       if ((dfile=
            mysql_file_create_with_symlink(mi_key_file_dfile,
-                                          linkname_ptr, filename, 0,
+                                          dlinkname_ptr, dfilename, 0,
                                           create_mode,
                                           MYF(MY_WME | create_flag))) < 0)
 	goto err;
@@ -838,19 +839,21 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
     (void) mysql_file_close(dfile, MYF(0));
     /* fall through */
   case 2:
-  if (! (flags & HA_DONT_TOUCH_DATA))
-    mysql_file_delete_with_symlink(mi_key_file_dfile,
-                                   fn_format(filename, name, "", MI_NAME_DEXT,
-                                             MY_UNPACK_FILENAME | MY_APPEND_EXT),
-                                   MYF(0));
+    if (! (flags & HA_DONT_TOUCH_DATA))
+    {
+      mysql_file_delete(mi_key_file_dfile, dfilename, MYF(0));
+      if (dlinkname_ptr)
+        mysql_file_delete(mi_key_file_dfile, dlinkname_ptr, MYF(0));
+    }
     /* fall through */
   case 1:
     (void) mysql_file_close(file, MYF(0));
     if (! (flags & HA_DONT_TOUCH_DATA))
-      mysql_file_delete_with_symlink(mi_key_file_kfile,
-                                     fn_format(filename, name, "", MI_NAME_IEXT,
-                                               MY_UNPACK_FILENAME | MY_APPEND_EXT),
-                                     MYF(0));
+    {
+      mysql_file_delete(mi_key_file_kfile, kfilename, MYF(0));
+      if (klinkname_ptr)
+        mysql_file_delete(mi_key_file_kfile, klinkname_ptr, MYF(0));
+    }
   }
   my_free(rec_per_key_part);
   DBUG_RETURN(my_errno=save_errno);		/* return the fatal errno */