From a74b01ea0e35c9c4a007c64609380844b36d2867 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 23 May 2019 15:09:40 +0400 Subject: [PATCH] MDEV-16548 - Innodb fails to start on older kernels that don't support F_DUPFD_CLOEXEC InnoDB duplicates file descriptor returned by create_temp_file() to workaround further inconsistent use of this descriptor. Use mysys file descriptors consistently for innobase_mysql_tmpfile(path). Mostly close it by appropriate mysys wrappers. --- include/my_sys.h | 1 + .../innodb/r/alter_inplace_perfschema.result | 2 +- mysys/my_winfile.c | 13 ++- storage/innobase/handler/ha_innodb.cc | 90 ------------------- storage/innobase/include/os0file.h | 9 -- storage/innobase/row/row0ftsort.cc | 4 +- storage/innobase/row/row0merge.cc | 15 +++- 7 files changed, 27 insertions(+), 107 deletions(-) diff --git a/include/my_sys.h b/include/my_sys.h index f224431ce3f..09d89607474 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -700,6 +700,7 @@ extern my_bool is_filename_allowed(const char *name, size_t length, #ifdef _WIN32 /* Windows-only functions (CRT equivalents)*/ extern HANDLE my_get_osfhandle(File fd); +extern File my_win_handle2File(HANDLE hFile); extern void my_osmaperr(unsigned long last_error); #endif diff --git a/mysql-test/suite/innodb/r/alter_inplace_perfschema.result b/mysql-test/suite/innodb/r/alter_inplace_perfschema.result index 440a1d0d6b3..4b5a2176b87 100644 --- a/mysql-test/suite/innodb/r/alter_inplace_perfschema.result +++ b/mysql-test/suite/innodb/r/alter_inplace_perfschema.result @@ -12,7 +12,7 @@ SET DEBUG_SYNC = 'now WAIT_FOR go'; select count_star into @final_count from performance_schema.events_waits_summary_global_by_event_name WHERE event_name LIKE '%wait%io%file%innodb%innodb_temp_file%'; SELECT @final_count - @init_count; @final_count - @init_count -11 +10 SET DEBUG_SYNC = 'now SIGNAL gone'; connection ddl; disconnect ddl; diff --git a/mysys/my_winfile.c b/mysys/my_winfile.c index 0c76eb25560..9c8d747adc9 100644 --- a/mysys/my_winfile.c +++ b/mysys/my_winfile.c @@ -501,13 +501,12 @@ static File my_get_stdfile_descriptor(FILE *stream) } -File my_win_fileno(FILE *file) +File my_win_handle2File(HANDLE hFile) { - HANDLE hFile= (HANDLE)_get_osfhandle(fileno(file)); int retval= -1; uint i; - DBUG_ENTER("my_win_fileno"); + DBUG_ENTER("my_win_handle2File"); for(i= MY_FILE_MIN; i < my_file_limit; i++) { @@ -517,6 +516,14 @@ File my_win_fileno(FILE *file) break; } } + DBUG_RETURN(retval); +} + + +File my_win_fileno(FILE *file) +{ + DBUG_ENTER("my_win_fileno"); + int retval= my_win_handle2File((HANDLE) _get_osfhandle(fileno(file))); if(retval == -1) /* try std stream */ DBUG_RETURN(my_get_stdfile_descriptor(file)); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 0ccd4384ab0..b69f90ddf51 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -2345,96 +2345,6 @@ static bool is_mysql_datadir_path(const char *path) TRUE)); } -static int mysql_tmpfile_path(const char *path, const char *prefix) -{ - DBUG_ASSERT(path != NULL); - DBUG_ASSERT((strlen(path) + strlen(prefix)) <= FN_REFLEN); - - char filename[FN_REFLEN]; - File fd = create_temp_file(filename, path, prefix, O_BINARY | O_SEQUENTIAL, - MYF(MY_WME | MY_TEMPORARY)); - return fd; -} - -/** Creates a temporary file in the location specified by the parameter -path. If the path is NULL, then it will be created in tmpdir. -@param[in] path location for creating temporary file -@return temporary file descriptor, or < 0 on error */ -os_file_t -innobase_mysql_tmpfile( - const char* path) -{ -#ifdef WITH_INNODB_DISALLOW_WRITES - os_event_wait(srv_allow_writes_event); -#endif /* WITH_INNODB_DISALLOW_WRITES */ - File fd; - - DBUG_EXECUTE_IF( - "innobase_tmpfile_creation_failure", - return(OS_FILE_CLOSED); - ); - - if (path == NULL) { - fd = mysql_tmpfile("ib"); - } else { - fd = mysql_tmpfile_path(path, "ib"); - } - - if (fd < 0) - return OS_FILE_CLOSED; - - /* Copy the file descriptor, so that the additional resources - allocated by create_temp_file() can be freed by invoking - my_close(). - - Because the file descriptor returned by this function - will be passed to fdopen(), it will be closed by invoking - fclose(), which in turn will invoke close() instead of - my_close(). */ - -#ifdef _WIN32 - /* Note that on Windows, the integer returned by mysql_tmpfile - has no relation to C runtime file descriptor. Here, we need - to call my_get_osfhandle to get the HANDLE and then convert it - to C runtime filedescriptor. */ - - HANDLE hFile = my_get_osfhandle(fd); - HANDLE hDup; - BOOL bOK = DuplicateHandle( - GetCurrentProcess(), - hFile, GetCurrentProcess(), - &hDup, 0, FALSE, DUPLICATE_SAME_ACCESS); - my_close(fd, MYF(MY_WME)); - - if (!bOK) { - my_osmaperr(GetLastError()); - goto error; - } - return hDup; -#else -#ifdef F_DUPFD_CLOEXEC - int fd2 = fcntl(fd, F_DUPFD_CLOEXEC, 0); -#else - int fd2 = dup(fd); -#endif - my_close(fd, MYF(MY_WME)); - if (fd2 < 0) { - set_my_errno(errno); - goto error; - } - return fd2; -#endif - -error: - char errbuf[MYSYS_STRERROR_SIZE]; - - my_error(EE_OUT_OF_FILERESOURCES, - MYF(0), - "ib*", errno, - my_strerror(errbuf, sizeof(errbuf), errno)); - return (OS_FILE_CLOSED); -} - /*********************************************************************//** Wrapper around MySQL's copy_and_convert function. @return number of bytes copied to 'to' */ diff --git a/storage/innobase/include/os0file.h b/storage/innobase/include/os0file.h index 57fb26378c9..812b4697ada 100644 --- a/storage/innobase/include/os0file.h +++ b/storage/innobase/include/os0file.h @@ -1524,15 +1524,6 @@ os_file_get_status( bool check_rw_perm, bool read_only); -/** Creates a temporary file in the location specified by the parameter -path. If the path is NULL then it will be created on --tmpdir location. -This function is defined in ha_innodb.cc. -@param[in] path location for creating temporary file -@return temporary file descriptor, or < 0 on error */ -os_file_t -innobase_mysql_tmpfile( - const char* path); - /** Set the file create umask @param[in] umask The umask to use for file creation. */ void diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/row0ftsort.cc index 8a7485e1064..ab83f928492 100644 --- a/storage/innobase/row/row0ftsort.cc +++ b/storage/innobase/row/row0ftsort.cc @@ -1029,12 +1029,12 @@ exit: crypt_block[i], table->space_id); if (error != DB_SUCCESS) { - os_file_close(tmpfd[i]); + row_merge_file_destroy_low(tmpfd[i]); goto func_exit; } total_rec += merge_file[i]->n_rec; - os_file_close(tmpfd[i]); + row_merge_file_destroy_low(tmpfd[i]); } func_exit: diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index 087f4e89125..d48c35b1ba1 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -4093,6 +4093,9 @@ pfs_os_file_t row_merge_file_create_low( const char* path) { +#ifdef WITH_INNODB_DISALLOW_WRITES + os_event_wait(srv_allow_writes_event); +#endif /* WITH_INNODB_DISALLOW_WRITES */ #ifdef UNIV_PFS_IO /* This temp file open does not go through normal file APIs, add instrumentation to register with @@ -4113,7 +4116,13 @@ row_merge_file_create_low( PSI_FILE_CREATE, path ? name : label, __FILE__, __LINE__); #endif - pfs_os_file_t fd = innobase_mysql_tmpfile(path); + DBUG_ASSERT(strlen(path) + 2 <= FN_REFLEN); + char filename[FN_REFLEN]; + File f = create_temp_file(filename, path, "ib", + O_BINARY | O_SEQUENTIAL, + MYF(MY_WME | MY_TEMPORARY)); + pfs_os_file_t fd = IF_WIN(my_get_osfhandle(f), f); + #ifdef UNIV_PFS_IO register_pfs_file_open_end(locker, fd, (fd == OS_FILE_CLOSED)?NULL:&fd); @@ -4158,7 +4167,9 @@ row_merge_file_destroy_low( const pfs_os_file_t& fd) /*!< in: merge file descriptor */ { if (fd != OS_FILE_CLOSED) { - os_file_close(fd); + int res = mysql_file_close(IF_WIN(my_win_handle2File(fd), fd), + MYF(MY_WME)); + ut_a(res != -1); } } /*********************************************************************//**