v3-0004-Use-POSIX-semantics-for-unlink-and-rename-on-Wind.patch

From 7f5431d7c586cc970aa2279a1fc1d2938b909253 Mon Sep 17 00:00:00 2001
From: Thomas <thomas.munro@gmail.com>
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 <v.spirin@postgrespro.ru>
Author: Thomas Munro <thomas.munro@gmail.com>
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