Skip to content

Commit 56b39cc

Browse files
macdiceadunstan
macdice
authored and
adunstan
committed
Check for STATUS_DELETE_PENDING on Windows.
1. Update our open() wrapper to check for NT's STATUS_DELETE_PENDING and translate it to Unix-like errors. This is done with RtlGetLastNtStatus(), which is dynamically loaded from ntdll. A new file win32ntdll.c centralizes lookup of NT functions, in case we decide to add more in the future. 2. Remove non-working code that was trying to do something similar for stat(), and just reuse the open() wrapper code. As a side effect, stat() also gains resilience against "sharing violation" errors. 3. Since stat() is used very early in process startup, remove the requirement that the Win32 signal event has been created before pgwin32_open_handle() is reached. Instead, teach pg_usleep() to fall back to a non-interruptible sleep if reached before the signal event is available. This could be back-patched, but for now it's in master only. The problem has apparently been with us for a long time and generated only a few complaints. Proposed patches trigger it more often, which led to this investigation and fix. Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Juan José Santamaría Flecha <juanjo.santamaria@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGJz_pZTF9mckn6XgSv69%2BjGwdgLkxZ6b3NWGLBCVjqUZA%40mail.gmail.com (cherry picked from commit e2f0f8e) Author: Thomas Munro <tmunro@postgresql.org> Author: Alexandra Wang <alexandra.wang.oss@gmail.com>
1 parent b4363fc commit 56b39cc

File tree

9 files changed

+179
-122
lines changed

9 files changed

+179
-122
lines changed

‎configure

+6
Original file line numberDiff line numberDiff line change
@@ -16638,6 +16638,12 @@ esac
1663816638
;;
1663916639
esac
1664016640

16641+
case " $LIBOBJS " in
16642+
*" win32ntdll.$ac_objext "* ) ;;
16643+
*) LIBOBJS="$LIBOBJS win32ntdll.$ac_objext"
16644+
;;
16645+
esac
16646+
1664116647
case " $LIBOBJS " in
1664216648
*" win32security.$ac_objext "* ) ;;
1664316649
*) LIBOBJS="$LIBOBJS win32security.$ac_objext"

‎configure.in

+1
Original file line numberDiff line numberDiff line change
@@ -1879,6 +1879,7 @@ if test "$PORTNAME" = "win32"; then
18791879
AC_LIBOBJ(system)
18801880
AC_LIBOBJ(win32env)
18811881
AC_LIBOBJ(win32error)
1882+
AC_LIBOBJ(win32ntdll)
18821883
AC_LIBOBJ(win32security)
18831884
AC_LIBOBJ(win32setlocale)
18841885
AC_LIBOBJ(win32stat)

‎src/backend/port/win32/signal.c

+11-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,17 @@ static BOOL WINAPI pg_console_handler(DWORD dwCtrlType);
5252
void
5353
pg_usleep(long microsec)
5454
{
55-
Assert(pgwin32_signal_event != NULL);
55+
if (unlikely(pgwin32_signal_event == NULL))
56+
{
57+
/*
58+
* If we're reached by pgwin32_open_handle() early in startup before
59+
* the signal event is set up, just fall back to a regular
60+
* non-interruptible sleep.
61+
*/
62+
SleepEx((microsec < 500 ? 1 : (microsec + 500) / 1000), FALSE);
63+
return;
64+
}
65+
5666
if (WaitForSingleObject(pgwin32_signal_event,
5767
(microsec < 500 ? 1 : (microsec + 500) / 1000))
5868
== WAIT_OBJECT_0)

‎src/include/port.h

+1
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ extern bool rmtree(const char *path, bool rmtopdir);
280280
* passing of other special options.
281281
*/
282282
#define O_DIRECT 0x80000000
283+
extern HANDLE pgwin32_open_handle(const char *, int, bool);
283284
extern int pgwin32_open(const char *, int,...);
284285
extern FILE *pgwin32_fopen(const char *, const char *);
285286
#define open(a,b,c) pgwin32_open(a,b,c)

‎src/include/port/win32ntdll.h

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* win32ntdll.h
4+
* Dynamically loaded Windows NT functions.
5+
*
6+
* Portions Copyright (c) 2021, PostgreSQL Global Development Group
7+
* Portions Copyright (c) 1994, Regents of the University of California
8+
*
9+
* src/include/port/win32ntdll.h
10+
*
11+
*-------------------------------------------------------------------------
12+
*/
13+
14+
/*
15+
* Because this includes NT headers that normally conflict with Win32 headers,
16+
* any translation unit that includes it should #define UMDF_USING_NTSTATUS
17+
* before including <windows.h>.
18+
*/
19+
20+
#include <ntstatus.h>
21+
#include <winternl.h>
22+
23+
typedef NTSTATUS (__stdcall *RtlGetLastNtStatus_t) (void);
24+
25+
extern RtlGetLastNtStatus_t pg_RtlGetLastNtStatus;
26+
27+
extern int initialize_ntdll(void);

‎src/port/open.c

+56-48
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,20 @@
1313

1414
#ifdef WIN32
1515

16+
#define UMDF_USING_NTSTATUS
17+
1618
#ifndef FRONTEND
1719
#include "postgres.h"
1820
#else
1921
#include "postgres_fe.h"
2022
#endif
2123

24+
#include "port/win32ntdll.h"
25+
2226
#include <fcntl.h>
2327
#include <assert.h>
2428
#include <sys/stat.h>
2529

26-
2730
static int
2831
openFlagsToCreateFileFlags(int openFlags)
2932
{
@@ -56,38 +59,25 @@ openFlagsToCreateFileFlags(int openFlags)
5659
}
5760

5861
/*
59-
* - file attribute setting, based on fileMode?
62+
* Internal function used by pgwin32_open() and _pgstat64(). When
63+
* backup_semantics is true, directories may be opened (for limited uses). On
64+
* failure, INVALID_HANDLE_VALUE is returned and errno is set.
6065
*/
61-
int
62-
pgwin32_open(const char *fileName, int fileFlags,...)
66+
HANDLE
67+
pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
6368
{
64-
int fd;
65-
HANDLE h = INVALID_HANDLE_VALUE;
69+
HANDLE h;
6670
SECURITY_ATTRIBUTES sa;
6771
int loops = 0;
6872

73+
if (initialize_ntdll() < 0)
74+
return INVALID_HANDLE_VALUE;
75+
6976
/* Check that we can handle the request */
7077
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
7178
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
7279
_O_SHORT_LIVED | O_DSYNC | O_DIRECT |
7380
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
74-
#ifndef FRONTEND
75-
Assert(pgwin32_signal_event != NULL); /* small chance of pg_usleep() */
76-
#endif
77-
78-
#ifdef FRONTEND
79-
80-
/*
81-
* Since PostgreSQL 12, those concurrent-safe versions of open() and
82-
* fopen() can be used by frontends, having as side-effect to switch the
83-
* file-translation mode from O_TEXT to O_BINARY if none is specified.
84-
* Caller may want to enforce the binary or text mode, but if nothing is
85-
* defined make sure that the default mode maps with what versions older
86-
* than 12 have been doing.
87-
*/
88-
if ((fileFlags & O_BINARY) == 0)
89-
fileFlags |= O_TEXT;
90-
#endif
9181

9282
sa.nLength = sizeof(sa);
9383
sa.bInheritHandle = TRUE;
@@ -102,6 +92,7 @@ pgwin32_open(const char *fileName, int fileFlags,...)
10292
&sa,
10393
openFlagsToCreateFileFlags(fileFlags),
10494
FILE_ATTRIBUTE_NORMAL |
95+
(backup_semantics ? FILE_FLAG_BACKUP_SEMANTICS : 0) |
10596
((fileFlags & O_RANDOM) ? FILE_FLAG_RANDOM_ACCESS : 0) |
10697
((fileFlags & O_SEQUENTIAL) ? FILE_FLAG_SEQUENTIAL_SCAN : 0) |
10798
((fileFlags & _O_SHORT_LIVED) ? FILE_ATTRIBUTE_TEMPORARY : 0) |
@@ -140,38 +131,55 @@ pgwin32_open(const char *fileName, int fileFlags,...)
140131
/*
141132
* ERROR_ACCESS_DENIED is returned if the file is deleted but not yet
142133
* gone (Windows NT status code is STATUS_DELETE_PENDING). In that
143-
* case we want to wait a bit and try again, giving up after 1 second
144-
* (since this condition should never persist very long). However,
145-
* there are other commonly-hit cases that return ERROR_ACCESS_DENIED,
146-
* so care is needed. In particular that happens if we try to open a
147-
* directory, or of course if there's an actual file-permissions
148-
* problem. To distinguish these cases, try a stat(). In the
149-
* delete-pending case, it will either also get STATUS_DELETE_PENDING,
150-
* or it will see the file as gone and fail with ENOENT. In other
151-
* cases it will usually succeed. The only somewhat-likely case where
152-
* this coding will uselessly wait is if there's a permissions problem
153-
* with a containing directory, which we hope will never happen in any
154-
* performance-critical code paths.
134+
* case, we'd better ask for the NT status too so we can translate it
135+
* to a more Unix-like error. We hope that nothing clobbers the NT
136+
* status in between the internal NtCreateFile() call and CreateFile()
137+
* returning.
138+
*
139+
* If there's no O_CREAT flag, then we'll pretend the file is
140+
* invisible. With O_CREAT, we have no choice but to report that
141+
* there's a file in the way (which wouldn't happen on Unix).
155142
*/
156-
if (err == ERROR_ACCESS_DENIED)
143+
if (err == ERROR_ACCESS_DENIED &&
144+
pg_RtlGetLastNtStatus() == STATUS_DELETE_PENDING)
157145
{
158-
if (loops < 10)
159-
{
160-
struct microsoft_native_stat st;
161-
162-
if (microsoft_native_stat(fileName, &st) != 0)
163-
{
164-
pg_usleep(100000);
165-
loops++;
166-
continue;
167-
}
168-
}
146+
if (fileFlags & O_CREAT)
147+
err = ERROR_FILE_EXISTS;
148+
else
149+
err = ERROR_FILE_NOT_FOUND;
169150
}
170151

171152
_dosmaperr(err);
172-
return -1;
153+
return INVALID_HANDLE_VALUE;
173154
}
174155

156+
return h;
157+
}
158+
159+
int
160+
pgwin32_open(const char *fileName, int fileFlags,...)
161+
{
162+
HANDLE h;
163+
int fd;
164+
165+
h = pgwin32_open_handle(fileName, fileFlags, false);
166+
if (h == INVALID_HANDLE_VALUE)
167+
return -1;
168+
169+
#ifdef FRONTEND
170+
171+
/*
172+
* Since PostgreSQL 12, those concurrent-safe versions of open() and
173+
* fopen() can be used by frontends, having as side-effect to switch the
174+
* file-translation mode from O_TEXT to O_BINARY if none is specified.
175+
* Caller may want to enforce the binary or text mode, but if nothing is
176+
* defined make sure that the default mode maps with what versions older
177+
* than 12 have been doing.
178+
*/
179+
if ((fileFlags & O_BINARY) == 0)
180+
fileFlags |= O_TEXT;
181+
#endif
182+
175183
/* _open_osfhandle will, on error, set errno accordingly */
176184
if ((fd = _open_osfhandle((intptr_t) h, fileFlags & O_APPEND)) < 0)
177185
CloseHandle(h); /* will not affect errno */

‎src/port/win32ntdll.c

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*-------------------------------------------------------------------------
2+
*
3+
* win32ntdll.c
4+
* Dynamically loaded Windows NT functions.
5+
*
6+
* Portions Copyright (c) 2021, PostgreSQL Global Development Group
7+
* Portions Copyright (c) 1994, Regents of the University of California
8+
*
9+
*
10+
* IDENTIFICATION
11+
* src/port/win32ntdll.c
12+
*
13+
*-------------------------------------------------------------------------
14+
*/
15+
16+
#define UMDF_USING_NTSTATUS
17+
18+
#include "c.h"
19+
20+
#include "port/win32ntdll.h"
21+
22+
RtlGetLastNtStatus_t pg_RtlGetLastNtStatus;
23+
24+
typedef struct NtDllRoutine
25+
{
26+
const char *name;
27+
pg_funcptr_t *address;
28+
} NtDllRoutine;
29+
30+
static const NtDllRoutine routines[] = {
31+
{"RtlGetLastNtStatus", (pg_funcptr_t *) &pg_RtlGetLastNtStatus}
32+
};
33+
34+
static bool initialized;
35+
36+
int
37+
initialize_ntdll(void)
38+
{
39+
HMODULE module;
40+
41+
if (initialized)
42+
return 0;
43+
44+
if (!(module = LoadLibraryEx("ntdll.dll", NULL, 0)))
45+
{
46+
_dosmaperr(GetLastError());
47+
return -1;
48+
}
49+
50+
for (int i = 0; i < lengthof(routines); ++i)
51+
{
52+
pg_funcptr_t address;
53+
54+
address = (pg_funcptr_t) GetProcAddress(module, routines[i].name);
55+
if (!address)
56+
{
57+
_dosmaperr(GetLastError());
58+
FreeLibrary(module);
59+
60+
return -1;
61+
}
62+
63+
*(pg_funcptr_t *) routines[i].address = address;
64+
}
65+
66+
initialized = true;
67+
68+
return 0;
69+
}

‎src/port/win32stat.c

+6-72
Original file line numberDiff line numberDiff line change
@@ -115,84 +115,18 @@ int
115115
_pgstat64(const char *name, struct stat *buf)
116116
{
117117
/*
118-
* We must use a handle so lstat() returns the information of the target
119-
* file. To have a reliable test for ERROR_DELETE_PENDING, this uses a
120-
* method similar to open() with a loop using stat() and some waits when
121-
* facing ERROR_ACCESS_DENIED.
118+
* Our open wrapper will report STATUS_DELETE_PENDING as ENOENT. We
119+
* request FILE_FLAG_BACKUP_SEMANTICS so that we can open directories too,
120+
* for limited purposes. We use the private handle-based version, so we
121+
* don't risk running out of fds.
122122
*/
123-
SECURITY_ATTRIBUTES sa;
124123
HANDLE hFile;
125124
int ret;
126-
int loops = 0;
127125

128-
if (name == NULL || buf == NULL)
129-
{
130-
errno = EINVAL;
131-
return -1;
132-
}
133-
/* fast not-exists check */
134-
if (GetFileAttributes(name) == INVALID_FILE_ATTRIBUTES)
135-
{
136-
DWORD err = GetLastError();
137-
138-
if (err != ERROR_ACCESS_DENIED)
139-
{
140-
_dosmaperr(err);
141-
return -1;
142-
}
143-
}
144-
145-
/* get a file handle as lightweight as we can */
146-
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
147-
sa.bInheritHandle = TRUE;
148-
sa.lpSecurityDescriptor = NULL;
149-
while ((hFile = CreateFile(name,
150-
GENERIC_READ,
151-
(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),
152-
&sa,
153-
OPEN_EXISTING,
154-
(FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |
155-
FILE_FLAG_OVERLAPPED),
156-
NULL)) == INVALID_HANDLE_VALUE)
157-
{
158-
DWORD err = GetLastError();
159-
160-
/*
161-
* ERROR_ACCESS_DENIED is returned if the file is deleted but not yet
162-
* gone (Windows NT status code is STATUS_DELETE_PENDING). In that
163-
* case we want to wait a bit and try again, giving up after 1 second
164-
* (since this condition should never persist very long). However,
165-
* there are other commonly-hit cases that return ERROR_ACCESS_DENIED,
166-
* so care is needed. In particular that happens if we try to open a
167-
* directory, or of course if there's an actual file-permissions
168-
* problem. To distinguish these cases, try a stat(). In the
169-
* delete-pending case, it will either also get STATUS_DELETE_PENDING,
170-
* or it will see the file as gone and fail with ENOENT. In other
171-
* cases it will usually succeed. The only somewhat-likely case where
172-
* this coding will uselessly wait is if there's a permissions problem
173-
* with a containing directory, which we hope will never happen in any
174-
* performance-critical code paths.
175-
*/
176-
if (err == ERROR_ACCESS_DENIED)
177-
{
178-
if (loops < 10)
179-
{
180-
struct microsoft_native_stat st;
181-
182-
if (microsoft_native_stat(name, &st) != 0)
183-
{
184-
pg_usleep(100000);
185-
loops++;
186-
continue;
187-
}
188-
}
189-
}
190-
191-
_dosmaperr(err);
126+
hFile = pgwin32_open_handle(name, O_RDONLY, true);
127+
if (hFile == INVALID_HANDLE_VALUE)
192128
return -1;
193-
}
194129

195-
/* At last we can invoke fileinfo_to_stat */
196130
ret = fileinfo_to_stat(hFile, buf);
197131

198132
CloseHandle(hFile);

0 commit comments

Comments
 (0)