From 7f5431d7c586cc970aa2279a1fc1d2938b909253 Mon Sep 17 00:00:00 2001 From: Thomas Date: Mon, 17 Oct 2022 22:41:18 -0700 Subject: [PATCH v3 4/4] Use POSIX semantics for unlink() and rename() on Windows. Use SetInformationByHandle() to implement unlink and rename with POSIX semantics. This should work on all Windows 10-based systems using NTFS. On ReFS and SMB, it currently falls back to non-POSIX semantics for file links, so we still maintain tests that demonstrate that behavior. Author: Victor Spirin Author: Thomas Munro Discussion: https://postgr.es/m/a529b660-da15-5b62-21a0-9936768210fd%40postgrespro.ru Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com --- src/port/dirmod.c | 239 +++++++++++++++++++++++++++--------- src/port/t/001_filesystem.c | 35 +++--- 2 files changed, 197 insertions(+), 77 deletions(-) diff --git a/src/port/dirmod.c b/src/port/dirmod.c index 0eb1e9566e..8a0aa32f9d 100644 --- a/src/port/dirmod.c +++ b/src/port/dirmod.c @@ -39,15 +39,125 @@ #endif #endif -#if defined(WIN32) && !defined(__CYGWIN__) -#include "port/win32ntdll.h" -#endif - #if defined(WIN32) || defined(__CYGWIN__) /* Externally visable only to allow testing. */ int pgwin32_dirmod_loops = 100; +#ifdef WIN32 + +/* + * XXX Because mingw doesn't yet define struct FILE_RENAME_INFO with the Flags + * member, we'll define a layout-compatible struct ourselves for now. See: + * + * https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_rename_info + */ +typedef struct XXX_FILE_RENAME_INFO +{ + union + { + BOOLEAN ReplaceIfExists; + DWORD Flags; + } DUMMYUNIONNAME; + HANDLE RootDirectory; + DWORD FileNameLength; + WCHAR FileName[1]; +} XXX_FILE_RENAME_INFO; + +/* + * XXX Because mingw seems to believe we need a higher _WIN32_WINNT than the + * Windows SDK requires for some of these macros, define them ourselves if + * necessary. + */ +#ifndef FILE_RENAME_FLAG_REPLACE_IF_EXISTS +#define FILE_RENAME_FLAG_REPLACE_IF_EXISTS 0x00000001 +#endif +#ifndef FILE_RENAME_FLAG_POSIX_SEMANTICS +#define FILE_RENAME_FLAG_POSIX_SEMANTICS 0x00000002 +#endif +#ifndef FILE_DISPOSITION_DELETE +#define FILE_DISPOSITION_DELETE 0x00000001 +#endif +#ifndef FILE_DISPOSITION_POSIX_SEMANTICS +#define FILE_DISPOSITION_POSIX_SEMANTICS 0x00000002 +#endif +/* Can't use macro tricks for FILE_INFO_BY_HANDLE_CLASS enumator names. */ +#define XXX_FileDispositionInfoEx 0x15 +#define XXX_FileRenameInfoEx 0x16 + +/* + * A container for FILE_RENAME_INFO that adds trailing space for FileName. + */ +typedef struct FILE_RENAME_INFO_EXT +{ + XXX_FILE_RENAME_INFO fri; + wchar_t extra_space[MAXPGPATH]; +} FILE_RENAME_INFO_EXT; + +static int +pgwin32_posix_rename(const char *from, const char *to) +{ + FILE_RENAME_INFO_EXT rename_info = {{.Flags = 0}}; + HANDLE handle; + + if (MultiByteToWideChar(CP_ACP, 0, to, -1, rename_info.fri.FileName, MAXPGPATH) == 0) + { + _dosmaperr(GetLastError()); + return -1; + } + rename_info.fri.Flags = FILE_RENAME_FLAG_POSIX_SEMANTICS | FILE_RENAME_FLAG_REPLACE_IF_EXISTS; + rename_info.fri.FileNameLength = wcslen(rename_info.fri.FileName); + + handle = CreateFile(from, + GENERIC_READ | GENERIC_WRITE | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + NULL); + if (handle == INVALID_HANDLE_VALUE) + { + _dosmaperr(GetLastError()); + return -1; + } + + if (!SetFileInformationByHandle(handle, + XXX_FileRenameInfoEx, + &rename_info, + sizeof(FILE_RENAME_INFO_EXT))) + { + DWORD error = GetLastError(); + + /* + * ReFS currently fails, so we'll try again without POSIX semantics. + * Likewise for SMB, except it helpfully fails with a different more + * general error. + */ + if (error == ERROR_NOT_SUPPORTED || error == ERROR_INVALID_PARAMETER) + { + /* Try the older FileRenameInfo (no "Ex", no Flags). */ + rename_info.fri.ReplaceIfExists = true; + if (!SetFileInformationByHandle(handle, FileRenameInfo, &rename_info, + sizeof(FILE_RENAME_INFO_EXT))) + { + _dosmaperr(GetLastError()); + CloseHandle(handle); + return -1; + } + } + else + { + _dosmaperr(error); + CloseHandle(handle); + return -1; + } + } + CloseHandle(handle); + return 0; +} + +#endif + /* * pgrename */ @@ -64,7 +174,7 @@ pgrename(const char *from, const char *to) * and blocking other backends. */ #if defined(WIN32) && !defined(__CYGWIN__) - while (!MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING)) + while (pgwin32_posix_rename(from, to) < 0) #else while (rename(from, to) < 0) #endif @@ -98,70 +208,75 @@ pgrename(const char *from, const char *to) return 0; } -/* - * Check if _pglstat64()'s reason for failure was STATUS_DELETE_PENDING. - * This doesn't apply to Cygwin, which has its own lstat() that would report - * the case as EACCES. -*/ -static bool -lstat_error_was_status_delete_pending(void) -{ - if (errno != ENOENT) - return false; #if defined(WIN32) && !defined(__CYGWIN__) - if (pg_RtlGetLastNtStatus() == STATUS_DELETE_PENDING) - return true; -#endif - return false; + +static int +pgwin32_posix_unlink(const char *path) +{ + BY_HANDLE_FILE_INFORMATION info; + HANDLE handle; + ULONG flags; + + flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS; + handle = CreateFile(path, + GENERIC_READ | GENERIC_WRITE | DELETE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + NULL); + if (handle == INVALID_HANDLE_VALUE) + { + _dosmaperr(GetLastError()); + return -1; + } + if (!GetFileInformationByHandle(handle, &info)) + { + _dosmaperr(GetLastError()); + CloseHandle(handle); + return -1; + } + /* Let junction points be unlinked this way, but not directories. */ + if ((info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && + !(info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) + { + CloseHandle(handle); + errno = EPERM; + return -1; + } + if (!SetFileInformationByHandle(handle, + XXX_FileDispositionInfoEx, + &flags, + sizeof(flags))) + { + _dosmaperr(GetLastError()); + + if (errno == EINVAL) + { + /* + * SMB filesystems fail like this. Fall back to (presumably) + * non-POSIX variant via C library. + */ + CloseHandle(handle); + return unlink(path); + } + + CloseHandle(handle); + return -1; + } + CloseHandle(handle); + return 0; } +#endif + /* * pgunlink */ int pgunlink(const char *path) { - bool is_lnk; int loops = 0; - struct stat st; - - /* - * This function might be called for a regular file or for a junction - * point (which we use to emulate symlinks). The latter must be unlinked - * with rmdir() on Windows. Before we worry about any of that, let's see - * if we can unlink directly, since that's expected to be the most common - * case. - */ - if (unlink(path) == 0) - return 0; - if (errno != EACCES) - return -1; - - /* - * EACCES is reported for many reasons including unlink() of a junction - * point. Check if that's the case so we can redirect to rmdir(). - * - * Note that by checking only once, we can't cope with a path that changes - * from regular file to junction point underneath us while we're retrying - * due to sharing violations, but that seems unlikely. We could perhaps - * prevent that by holding a file handle ourselves across the lstat() and - * the retry loop, but that seems like over-engineering for now. - * - * In the special case of a STATUS_DELETE_PENDING error (file already - * unlinked, but someone still has it open), we don't want to report ENOENT - * to the caller immediately, because rmdir(parent) would probably fail. - * We want to wait until the file truly goes away so that simple recursive - * directory unlink algorithms work. - */ - if (lstat(path, &st) < 0) - { - if (lstat_error_was_status_delete_pending()) - is_lnk = false; - else - return -1; - } - else - is_lnk = S_ISLNK(st.st_mode); /* * We need to loop because even though PostgreSQL uses flags that allow @@ -170,7 +285,11 @@ pgunlink(const char *path) * someone else to close the file, as the caller might be holding locks * and blocking other backends. */ - while ((is_lnk ? rmdir(path) : unlink(path)) < 0) +#ifdef WIN32 + while (pgwin32_posix_unlink(path) < 0) +#else + while (unlink(path) < 0) +#endif { if (errno != EACCES) return -1; diff --git a/src/port/t/001_filesystem.c b/src/port/t/001_filesystem.c index 99bfd6bdd5..fd842c55d1 100644 --- a/src/port/t/001_filesystem.c +++ b/src/port/t/001_filesystem.c @@ -3,7 +3,7 @@ * Windows. * * Currently, have_posix_unlink_semantics is expected to be true on all Unix - * systems and some Windows 10-based operatings using NTFS, and false on other + * systems and all Windows 10-based operatings using NTFS, and false on other * Windows ReFS and SMB filesystems. */ @@ -442,10 +442,10 @@ filesystem_metadata_tests(void) * Linux), though AIX/JFS1 is rumored to succeed. However, our Windows * emulation doesn't allow it, because we want to avoid surprises by * behaving like nearly all Unix systems. So we check this on Windows - * only, where it fails with non-standard EACCES. + * only, where our wrapper fails with EPERM. */ PG_EXPECT_SYS(unlink(path2) == -1, "Windows: can't unlink() a directory"); - PG_EXPECT_EQ(errno, EACCES); + PG_EXPECT_EQ(errno, EPERM); #endif #ifdef WIN32 @@ -539,22 +539,23 @@ filesystem_metadata_tests(void) fd = open(path2, O_RDWR | PG_BINARY, 0777); PG_EXPECT_SYS(fd >= 0, "open name2.txt"); make_path(path2, "name2.txt"); -#ifdef WIN32 - /* - * Windows can't rename over an open non-unlinked file, even with - * have_posix_unlink_semantics. - */ - pgwin32_dirmod_loops = 2; /* minimize looping to fail fast in testing */ - PG_EXPECT_SYS(rename(path, path2) == -1, - "Windows: can't rename name1.txt -> name2.txt while name2.txt is open"); - PG_EXPECT_EQ(errno, EACCES); - PG_EXPECT_SYS(unlink(path) == 0, "unlink name1.txt"); -#else - PG_EXPECT_SYS(rename(path, path2) == 0, - "POSIX: can rename name1.txt -> name2.txt while name2.txt is open"); + if (!have_posix_unlink_semantics) + { +#ifdef WIN32 + pgwin32_dirmod_loops = 2; /* minimize looping to fail fast in testing */ #endif - PG_EXPECT_SYS(close(fd) == 0); + PG_EXPECT_SYS(rename(path, path2) == -1, + "Windows non-POSIX: can't rename name1.txt -> name2.txt while name2.txt is open"); + PG_EXPECT_EQ(errno, EACCES); + PG_EXPECT_SYS(unlink(path) == 0, "unlink name1.txt"); + } + else + { + PG_EXPECT_SYS(rename(path, path2) == 0, + "POSIX: can rename name1.txt -> name2.txt while name2.txt is open"); + } + PG_REQUIRE_SYS(fd < 0 || close(fd) == 0); make_path(path, "name1.txt"); fd = open(path, O_CREAT | O_EXCL | O_RDWR | PG_BINARY, 0777); -- 2.35.1