From d360fa6fa897d9556dc3813e6f3366e01df8e715 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Mon, 5 Dec 2022 15:07:50 +0100 Subject: MDEV-30162 Fix occasional "Permission denied" on Windows caused by buggy 3rd party Add retry logic for CreateFile, DeleteFile, or MoveFile when GetLastError() is ERROR_SHARING_VIOLATION. --- mysys/my_delete.c | 173 +++++++++++++++++++++++++++++------------------------ mysys/my_rename.c | 29 ++------- mysys/my_winfile.c | 38 +++++++++++- mysys/mysys_priv.h | 39 ++++++++++++ 4 files changed, 176 insertions(+), 103 deletions(-) (limited to 'mysys') diff --git a/mysys/my_delete.c b/mysys/my_delete.c index cce26643ef7..0e9da9f50ca 100644 --- a/mysys/my_delete.c +++ b/mysys/my_delete.c @@ -79,7 +79,8 @@ int my_delete(const char *name, myf MyFlags) a file to unique name. Symbolic link are deleted without renaming. Directories are not deleted. - */ +*/ + static int my_win_unlink(const char *name) { HANDLE handle= INVALID_HANDLE_VALUE; @@ -87,99 +88,113 @@ static int my_win_unlink(const char *name) uint last_error; char unique_filename[MAX_PATH + 35]; unsigned long long tsc; /* time stamp counter, for unique filename*/ - + int retries; DBUG_ENTER("my_win_unlink"); - attributes= GetFileAttributes(name); - if (attributes == INVALID_FILE_ATTRIBUTES) - { - last_error= GetLastError(); - DBUG_PRINT("error",("GetFileAttributes(%s) failed with %u\n", name, last_error)); - goto error; - } - if (attributes & FILE_ATTRIBUTE_DIRECTORY) - { - DBUG_PRINT("error",("can't remove %s - it is a directory\n", name)); - errno= EINVAL; - DBUG_RETURN(-1); - } - - if (attributes & FILE_ATTRIBUTE_REPARSE_POINT) + DBUG_INJECT_FILE_SHARING_VIOLATION(name); + + for (retries= FILE_SHARING_VIOLATION_RETRIES; ; retries--) { - /* Symbolic link. Delete link, the not target */ - if (!DeleteFile(name)) + attributes= GetFileAttributes(name); + if (attributes == INVALID_FILE_ATTRIBUTES) { - last_error= GetLastError(); - DBUG_PRINT("error",("DeleteFile(%s) failed with %u\n", name,last_error)); - goto error; + last_error= GetLastError(); + DBUG_PRINT("error", + ("GetFileAttributes(%s) failed with %u\n", name, last_error)); + goto error; } - DBUG_RETURN(0); - } - /* - Try Windows 10 method, delete with "posix semantics" (file is not visible, and creating - a file with the same name won't fail, even if it the fiile was open) - */ - struct - { - DWORD _Flags; - } disp={0x3}; - /* 0x3 = FILE_DISPOSITION_FLAG_DELETE | FILE_DISPOSITION_FLAG_POSIX_SEMANTICS */ + if (attributes & FILE_ATTRIBUTE_DIRECTORY) + { + DBUG_PRINT("error", ("can't remove %s - it is a directory\n", name)); + errno= EINVAL; + DBUG_RETURN(-1); + } - handle= CreateFile(name, DELETE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, - NULL, OPEN_EXISTING, 0, NULL); - if (handle != INVALID_HANDLE_VALUE) - { - BOOL ok= SetFileInformationByHandle(handle, - (FILE_INFO_BY_HANDLE_CLASS) 21, &disp, sizeof(disp)); - CloseHandle(handle); - if (ok) + if (attributes & FILE_ATTRIBUTE_REPARSE_POINT) + { + /* Symbolic link. Delete link, the not target */ + if (!DeleteFile(name)) + { + last_error= GetLastError(); + DBUG_PRINT("error", + ("DeleteFile(%s) failed with %u\n", name, last_error)); + goto error; + } DBUG_RETURN(0); - } + } - handle= CreateFile(name, DELETE, 0, NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL); - if (handle != INVALID_HANDLE_VALUE) - { /* - We opened file without sharing flags (exclusive), no one else has this file - opened, thus it is save to close handle to remove it. No renaming is - necessary. + Try Windows 10 method, delete with "posix semantics" (file is not + visible, and creating a file with the same name won't fail, even if it + the file was open) */ - CloseHandle(handle); - DBUG_RETURN(0); - } + handle= CreateFile(name, DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, 0, NULL); + if (handle != INVALID_HANDLE_VALUE) + { + /* 0x3 = FILE_DISPOSITION_FLAG_DELETE | FILE_DISPOSITION_FLAG_POSIX_SEMANTICS */ + struct {DWORD _Flags;} disp= {0x3}; + BOOL ok= SetFileInformationByHandle( + handle, (FILE_INFO_BY_HANDLE_CLASS) 21, &disp, sizeof(disp)); + CloseHandle(handle); + if (ok) + DBUG_RETURN(0); + } - /* - Can't open file exclusively, hence the file must be already opened by - someone else. Open it for delete (with all FILE_SHARE flags set), - rename to unique name, close. - */ - handle= CreateFile(name, DELETE, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, - NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL); - if (handle == INVALID_HANDLE_VALUE) - { - last_error= GetLastError(); - DBUG_PRINT("error", - ("CreateFile(%s) with FILE_FLAG_DELETE_ON_CLOSE failed with %u\n", - name,last_error)); - goto error; - } + handle= CreateFile(name, DELETE, 0, NULL, OPEN_EXISTING, + FILE_FLAG_DELETE_ON_CLOSE, NULL); + if (handle != INVALID_HANDLE_VALUE) + { + /* + We opened file without sharing flags (exclusive), no one else has this + file opened, thus it is safe to close handle to remove it. No renaming + is necessary. + */ + CloseHandle(handle); + DBUG_RETURN(0); + } - tsc= __rdtsc(); - my_snprintf(unique_filename,sizeof(unique_filename),"%s.%llx.deleted", - name, tsc); - if (!MoveFile(name, unique_filename)) - { - DBUG_PRINT("warning", ("moving %s to unique filename failed, error %lu\n", - name,GetLastError())); - } + /* + Can't open file exclusively, hence the file must be already opened by + someone else. Open it for delete (with all FILE_SHARE flags set), + rename to unique name, close. + */ + handle= CreateFile(name, DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, NULL); + if (handle == INVALID_HANDLE_VALUE) + { + last_error= GetLastError(); + DBUG_PRINT( + "error", + ("CreateFile(%s) with FILE_FLAG_DELETE_ON_CLOSE failed with %u\n", + name, last_error)); + goto error; + } + + tsc= __rdtsc(); + my_snprintf(unique_filename, sizeof(unique_filename), "%s.%llx.deleted", + name, tsc); + if (!MoveFile(name, unique_filename)) + { + DBUG_PRINT("warning", + ("moving %s to unique filename failed, error %lu\n", name, + GetLastError())); + } + CloseHandle(handle); + DBUG_RETURN(0); - CloseHandle(handle); - DBUG_RETURN(0); - error: - my_osmaperr(last_error); - DBUG_RETURN(-1); + if (last_error != ERROR_SHARING_VIOLATION || retries == 0) + { + my_osmaperr(last_error); + DBUG_RETURN(-1); + } + DBUG_CLEAR_FILE_SHARING_VIOLATION(); + Sleep(FILE_SHARING_VIOLATION_DELAY_MS); + } } #endif diff --git a/mysys/my_rename.c b/mysys/my_rename.c index 23dbec2d7ff..56a1f2a7cec 100644 --- a/mysys/my_rename.c +++ b/mysys/my_rename.c @@ -34,41 +34,24 @@ */ static BOOL win_rename_with_retries(const char *from, const char *to) { -#ifndef DBUG_OFF - FILE *fp = NULL; - DBUG_EXECUTE_IF("rename_sharing_violation", - { - fp= fopen(from, "r"); - DBUG_ASSERT(fp); - } - ); -#endif + DBUG_INJECT_FILE_SHARING_VIOLATION(from); - for (int retry= RENAME_MAX_RETRIES; retry--;) + for (int retry= FILE_SHARING_VIOLATION_RETRIES; retry--;) { BOOL ret= MoveFileEx(from, to, MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING); + DBUG_CLEAR_FILE_SHARING_VIOLATION(); + if (ret) return ret; DWORD last_error= GetLastError(); + if (last_error == ERROR_SHARING_VIOLATION || last_error == ERROR_ACCESS_DENIED) { -#ifndef DBUG_OFF - /* - If error was injected in via DBUG_EXECUTE_IF, close the file - that is causing ERROR_SHARING_VIOLATION, so that retry succeeds. - */ - if (fp) - { - fclose(fp); - fp= NULL; - } -#endif - - Sleep(10); + Sleep(FILE_SHARING_VIOLATION_DELAY_MS); } else return ret; diff --git a/mysys/my_winfile.c b/mysys/my_winfile.c index 0baca6d62d7..a2b621c7aa3 100644 --- a/mysys/my_winfile.c +++ b/mysys/my_winfile.c @@ -102,6 +102,42 @@ static int my_get_open_flags(File fd) DBUG_RETURN(my_file_info[fd].oflag); } +/* + CreateFile with retry logic. + + Uses retries, to avoid or reduce CreateFile errors + with ERROR_SHARING_VIOLATION, in case the file is opened + by another process, which used incompatible sharing + flags when opening. + + See Windows' CreateFile() documentation for details. +*/ +static HANDLE my_create_file_with_retries( + LPCSTR lpFileName, DWORD dwDesiredAccess, + DWORD dwShareMode, + LPSECURITY_ATTRIBUTES lpSecurityAttributes, + DWORD dwCreationDisposition, + DWORD dwFlagsAndAttributes, + HANDLE hTemplateFile) +{ + int retries; + DBUG_INJECT_FILE_SHARING_VIOLATION(lpFileName); + + for (retries = FILE_SHARING_VIOLATION_RETRIES;;) + { + HANDLE h= CreateFile(lpFileName, dwDesiredAccess, dwShareMode, + lpSecurityAttributes, dwCreationDisposition, + dwFlagsAndAttributes, hTemplateFile); + DBUG_CLEAR_FILE_SHARING_VIOLATION(); + + if (h != INVALID_HANDLE_VALUE || + GetLastError() != ERROR_SHARING_VIOLATION || --retries == 0) + return h; + + Sleep(FILE_SHARING_VIOLATION_DELAY_MS); + } + return INVALID_HANDLE_VALUE; +} /* Open a file with sharing. Similar to _sopen() from libc, but allows managing @@ -247,7 +283,7 @@ File my_win_sopen(const char *path, int oflag, int shflag, int pmode) fileattrib|= FILE_FLAG_RANDOM_ACCESS; /* try to open/create the file */ - if ((osfh= CreateFile(path, fileaccess, fileshare, &SecurityAttributes, + if ((osfh= my_create_file_with_retries(path, fileaccess, fileshare, &SecurityAttributes, filecreate, fileattrib, NULL)) == INVALID_HANDLE_VALUE) { DWORD last_error= GetLastError(); diff --git a/mysys/mysys_priv.h b/mysys/mysys_priv.h index 0fa6a4c553f..57e734321fa 100644 --- a/mysys/mysys_priv.h +++ b/mysys/mysys_priv.h @@ -176,6 +176,45 @@ extern int my_win_fsync(File fd); extern File my_win_dup(File fd); extern File my_win_sopen(const char *path, int oflag, int shflag, int perm); extern File my_open_osfhandle(HANDLE handle, int oflag); + + +/* + The following constants are related to retries when file operation fails with + ERROR_FILE_SHARING_VIOLATION +*/ +#define FILE_SHARING_VIOLATION_RETRIES 50 +#define FILE_SHARING_VIOLATION_DELAY_MS 10 + + +/* DBUG injecting of ERROR_FILE_SHARING_VIOLATION */ +#ifndef DBUG_OFF +/* Open file, without sharing. if specific DBUG keyword is set */ +#define DBUG_INJECT_FILE_SHARING_VIOLATION(filename) \ + FILE *fp= NULL; \ + do \ + { \ + DBUG_EXECUTE_IF("file_sharing_violation", \ + fp= _fsopen(filename, "r", _SH_DENYRW);); \ + } while (0) + +/* Close the file that causes ERROR_FILE_SHARING_VIOLATION.*/ +#define DBUG_CLEAR_FILE_SHARING_VIOLATION() \ + do \ + { \ + if (fp) \ + { \ + DWORD tmp_err= GetLastError(); \ + fclose(fp); \ + SetLastError(tmp_err); \ + fp= NULL; \ + } \ + } while (0) + +#else +#define DBUG_INJECT_FILE_SHARING_VIOLATION(filename) do {} while (0) +#define DBUG_CLEAR_FILE_SHARING_VIOLATION() do {} while (0) +#endif + #endif C_MODE_END -- cgit v1.2.1