On Thu, Jul 08, 2021 at 11:00:00PM +0300, Alexander Lakhin wrote: > Beside the aforementioned test I can only propose the extended patch, > that incorporates the undo of the changes brought by bed90759f. > With this patch that test is passed.
Checked and confirmed. It is a nice test with IPC::Run you have here. Making things in win32stat.c more consistent with open.c surely is appealing. One thing that I'd like to introduce in this patch, and also mentioned upthread, is to change the stat() call in open.c to use microsoft_native_stat(). I have let pgbench run for a couple of hours with some concurrent activity using genfile.c, without noticing problems. My environment is not representative of everything we can find out there on Windows, but it brings some confidence. -- Michael
diff --git a/src/port/open.c b/src/port/open.c
index 14c6debba9..4bd11ca9d9 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -157,9 +157,9 @@ pgwin32_open(const char *fileName, int fileFlags,...)
{
if (loops < 10)
{
- struct stat st;
+ struct microsoft_native_stat st;
- if (stat(fileName, &st) != 0)
+ if (microsoft_native_stat(fileName, &st) != 0)
{
pg_usleep(100000);
loops++;
diff --git a/src/port/win32stat.c b/src/port/win32stat.c
index 2ad8ee1359..6028ad809e 100644
--- a/src/port/win32stat.c
+++ b/src/port/win32stat.c
@@ -18,53 +18,6 @@
#include "c.h"
#include <windows.h>
-/*
- * In order to support MinGW and MSVC2013 we use NtQueryInformationFile as an
- * alternative for GetFileInformationByHandleEx. It is loaded from the ntdll
- * library.
- */
-#if _WIN32_WINNT < 0x0600
-#include <winternl.h>
-
-#if !defined(__MINGW32__) && !defined(__MINGW64__)
-/* MinGW includes this in <winternl.h>, but it is missing in MSVC */
-typedef struct _FILE_STANDARD_INFORMATION
-{
- LARGE_INTEGER AllocationSize;
- LARGE_INTEGER EndOfFile;
- ULONG NumberOfLinks;
- BOOLEAN DeletePending;
- BOOLEAN Directory;
-} FILE_STANDARD_INFORMATION;
-#define FileStandardInformation 5
-#endif /* !defined(__MINGW32__) &&
- * !defined(__MINGW64__) */
-
-typedef NTSTATUS (NTAPI * PFN_NTQUERYINFORMATIONFILE)
- (IN HANDLE FileHandle,
- OUT PIO_STATUS_BLOCK IoStatusBlock,
- OUT PVOID FileInformation,
- IN ULONG Length,
- IN FILE_INFORMATION_CLASS FileInformationClass);
-
-static PFN_NTQUERYINFORMATIONFILE _NtQueryInformationFile = NULL;
-
-static HMODULE ntdll = NULL;
-
-/*
- * Load DLL file just once regardless of how many functions we load/call in it.
- */
-static void
-LoadNtdll(void)
-{
- if (ntdll != NULL)
- return;
- ntdll = LoadLibraryEx("ntdll.dll", NULL, 0);
-}
-
-#endif /* _WIN32_WINNT < 0x0600 */
-
-
/*
* Convert a FILETIME struct into a 64 bit time_t.
*/
@@ -170,108 +123,71 @@ _pgstat64(const char *name, struct stat *buf)
SECURITY_ATTRIBUTES sa;
HANDLE hFile;
int ret;
-#if _WIN32_WINNT < 0x0600
- IO_STATUS_BLOCK ioStatus;
- FILE_STANDARD_INFORMATION standardInfo;
-#else
- FILE_STANDARD_INFO standardInfo;
-#endif
+ int loops = 0;
if (name == NULL || buf == NULL)
{
errno = EINVAL;
return -1;
}
-
/* fast not-exists check */
if (GetFileAttributes(name) == INVALID_FILE_ATTRIBUTES)
{
- _dosmaperr(GetLastError());
- return -1;
+ DWORD err = GetLastError();
+
+ if (err != ERROR_ACCESS_DENIED) {
+ _dosmaperr(err);
+ return -1;
+ }
}
/* get a file handle as lightweight as we can */
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
- hFile = CreateFile(name,
- GENERIC_READ,
- (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),
- &sa,
- OPEN_EXISTING,
- (FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |
- FILE_FLAG_OVERLAPPED),
- NULL);
- if (hFile == INVALID_HANDLE_VALUE)
+ while ((hFile = CreateFile(name,
+ GENERIC_READ,
+ (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),
+ &sa,
+ OPEN_EXISTING,
+ (FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |
+ FILE_FLAG_OVERLAPPED),
+ NULL)) == INVALID_HANDLE_VALUE)
{
DWORD err = GetLastError();
- CloseHandle(hFile);
- _dosmaperr(err);
- return -1;
- }
-
- memset(&standardInfo, 0, sizeof(standardInfo));
-
-#if _WIN32_WINNT < 0x0600
- if (_NtQueryInformationFile == NULL)
- {
- /* First time through: load ntdll.dll and find NtQueryInformationFile */
- LoadNtdll();
- if (ntdll == NULL)
- {
- DWORD err = GetLastError();
-
- CloseHandle(hFile);
- _dosmaperr(err);
- return -1;
- }
-
- _NtQueryInformationFile = (PFN_NTQUERYINFORMATIONFILE) (pg_funcptr_t)
- GetProcAddress(ntdll, "NtQueryInformationFile");
- if (_NtQueryInformationFile == NULL)
- {
- DWORD err = GetLastError();
-
- CloseHandle(hFile);
- _dosmaperr(err);
- return -1;
- }
- }
-
- if (!NT_SUCCESS(_NtQueryInformationFile(hFile, &ioStatus, &standardInfo,
- sizeof(standardInfo),
- FileStandardInformation)))
- {
- DWORD err = GetLastError();
-
- CloseHandle(hFile);
- _dosmaperr(err);
- return -1;
- }
-#else
- if (!GetFileInformationByHandleEx(hFile, FileStandardInfo, &standardInfo,
- sizeof(standardInfo)))
- {
- DWORD err = GetLastError();
-
- CloseHandle(hFile);
- _dosmaperr(err);
- return -1;
- }
-#endif /* _WIN32_WINNT < 0x0600 */
-
- if (standardInfo.DeletePending)
- {
/*
- * File has been deleted, but is not gone from the filesystem yet.
- * This can happen when some process with FILE_SHARE_DELETE has it
- * open, and it will be fully removed once that handle is closed.
- * Meanwhile, we can't open it, so indicate that the file just doesn't
- * exist.
+ * ERROR_ACCESS_DENIED is returned if the file is deleted but not yet
+ * gone (Windows NT status code is STATUS_DELETE_PENDING). In that
+ * case we want to wait a bit and try again, giving up after 1 second
+ * (since this condition should never persist very long). However,
+ * there are other commonly-hit cases that return ERROR_ACCESS_DENIED,
+ * so care is needed. In particular that happens if we try to open a
+ * directory, or of course if there's an actual file-permissions
+ * problem. To distinguish these cases, try a stat(). In the
+ * delete-pending case, it will either also get STATUS_DELETE_PENDING,
+ * or it will see the file as gone and fail with ENOENT. In other
+ * cases it will usually succeed. The only somewhat-likely case where
+ * this coding will uselessly wait is if there's a permissions problem
+ * with a containing directory, which we hope will never happen in any
+ * performance-critical code paths.
*/
- CloseHandle(hFile);
- errno = ENOENT;
+ if (err == ERROR_ACCESS_DENIED)
+ {
+ if (loops < 10)
+ {
+ struct microsoft_native_stat st;
+
+ if (microsoft_native_stat(name, &st) != 0)
+ {
+ pg_usleep(100000);
+ loops++;
+ continue;
+ }
+ }
+ }
+
+ _dosmaperr(err);
return -1;
}
signature.asc
Description: PGP signature
