Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 54fb8c7

Browse files
committed
Fix issues with Windows' stat() for files pending on deletion
The code introduced by bed9075 to enhance the stat() implementation on Windows for file sizes larger than 4GB fails to properly detect files pending for deletion with its method based on NtQueryInformationFile() or GetFileInformationByHandleEx(), as proved by Alexander Lakhin in a custom TAP test of his own. The method used in the implementation of open() to sleep and loop when when failing on ERROR_ACCESS_DENIED (EACCES) is showing much more stability, so switch to this method. This could still lead to issues if the permission problem stays around for much longer than the timeout of 1 second used, but that should (hopefully) never happen in performance-critical paths. Still, there could be a point in increasing the timeouts for the sake of machines that handle heavy loads. Note that WIN32's open() now uses microsoft_native_stat() as it should be similar to stat() when working around issues with concurrent file deletions. I have spent some time testing this patch with pgbench in combination of the SQL functions from genfile.c, as well as running the TAP test provided on the thread with MSVC builds, and this looks much more stable than the previous method. Author: Alexander Lakhin Reviewed-by: Tom Lane, Michael Paquier, Justin Pryzby Discussion: https://postgr.es/m/c3427edf-d7c0-ff57-90f6-b5de3bb62709@gmail.com Backpatch-through: 14
1 parent 127404f commit 54fb8c7

File tree

2 files changed

+49
-132
lines changed

2 files changed

+49
-132
lines changed

src/port/open.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@ pgwin32_open(const char *fileName, int fileFlags,...)
157157
{
158158
if (loops < 10)
159159
{
160-
struct stat st;
160+
struct microsoft_native_stat st;
161161

162-
if (stat(fileName, &st) != 0)
162+
if (microsoft_native_stat(fileName, &st) != 0)
163163
{
164164
pg_usleep(100000);
165165
loops++;

src/port/win32stat.c

+47-130
Original file line numberDiff line numberDiff line change
@@ -18,53 +18,6 @@
1818
#include "c.h"
1919
#include <windows.h>
2020

21-
/*
22-
* In order to support MinGW and MSVC2013 we use NtQueryInformationFile as an
23-
* alternative for GetFileInformationByHandleEx. It is loaded from the ntdll
24-
* library.
25-
*/
26-
#if _WIN32_WINNT < 0x0600
27-
#include <winternl.h>
28-
29-
#if !defined(__MINGW32__) && !defined(__MINGW64__)
30-
/* MinGW includes this in <winternl.h>, but it is missing in MSVC */
31-
typedef struct _FILE_STANDARD_INFORMATION
32-
{
33-
LARGE_INTEGER AllocationSize;
34-
LARGE_INTEGER EndOfFile;
35-
ULONG NumberOfLinks;
36-
BOOLEAN DeletePending;
37-
BOOLEAN Directory;
38-
} FILE_STANDARD_INFORMATION;
39-
#define FileStandardInformation 5
40-
#endif /* !defined(__MINGW32__) &&
41-
* !defined(__MINGW64__) */
42-
43-
typedef NTSTATUS (NTAPI * PFN_NTQUERYINFORMATIONFILE)
44-
(IN HANDLE FileHandle,
45-
OUT PIO_STATUS_BLOCK IoStatusBlock,
46-
OUT PVOID FileInformation,
47-
IN ULONG Length,
48-
IN FILE_INFORMATION_CLASS FileInformationClass);
49-
50-
static PFN_NTQUERYINFORMATIONFILE _NtQueryInformationFile = NULL;
51-
52-
static HMODULE ntdll = NULL;
53-
54-
/*
55-
* Load DLL file just once regardless of how many functions we load/call in it.
56-
*/
57-
static void
58-
LoadNtdll(void)
59-
{
60-
if (ntdll != NULL)
61-
return;
62-
ntdll = LoadLibraryEx("ntdll.dll", NULL, 0);
63-
}
64-
65-
#endif /* _WIN32_WINNT < 0x0600 */
66-
67-
6821
/*
6922
* Convert a FILETIME struct into a 64 bit time_t.
7023
*/
@@ -163,115 +116,79 @@ _pgstat64(const char *name, struct stat *buf)
163116
{
164117
/*
165118
* We must use a handle so lstat() returns the information of the target
166-
* file. To have a reliable test for ERROR_DELETE_PENDING, we use
167-
* NtQueryInformationFile from Windows 2000 or
168-
* GetFileInformationByHandleEx from Server 2008 / Vista.
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.
169122
*/
170123
SECURITY_ATTRIBUTES sa;
171124
HANDLE hFile;
172125
int ret;
173-
#if _WIN32_WINNT < 0x0600
174-
IO_STATUS_BLOCK ioStatus;
175-
FILE_STANDARD_INFORMATION standardInfo;
176-
#else
177-
FILE_STANDARD_INFO standardInfo;
178-
#endif
126+
int loops = 0;
179127

180128
if (name == NULL || buf == NULL)
181129
{
182130
errno = EINVAL;
183131
return -1;
184132
}
185-
186133
/* fast not-exists check */
187134
if (GetFileAttributes(name) == INVALID_FILE_ATTRIBUTES)
188-
{
189-
_dosmaperr(GetLastError());
190-
return -1;
191-
}
192-
193-
/* get a file handle as lightweight as we can */
194-
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
195-
sa.bInheritHandle = TRUE;
196-
sa.lpSecurityDescriptor = NULL;
197-
hFile = CreateFile(name,
198-
GENERIC_READ,
199-
(FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE),
200-
&sa,
201-
OPEN_EXISTING,
202-
(FILE_FLAG_NO_BUFFERING | FILE_FLAG_BACKUP_SEMANTICS |
203-
FILE_FLAG_OVERLAPPED),
204-
NULL);
205-
if (hFile == INVALID_HANDLE_VALUE)
206135
{
207136
DWORD err = GetLastError();
208137

209-
CloseHandle(hFile);
210-
_dosmaperr(err);
211-
return -1;
212-
}
213-
214-
memset(&standardInfo, 0, sizeof(standardInfo));
215-
216-
#if _WIN32_WINNT < 0x0600
217-
if (_NtQueryInformationFile == NULL)
218-
{
219-
/* First time through: load ntdll.dll and find NtQueryInformationFile */
220-
LoadNtdll();
221-
if (ntdll == NULL)
222-
{
223-
DWORD err = GetLastError();
224-
225-
CloseHandle(hFile);
226-
_dosmaperr(err);
227-
return -1;
228-
}
229-
230-
_NtQueryInformationFile = (PFN_NTQUERYINFORMATIONFILE) (pg_funcptr_t)
231-
GetProcAddress(ntdll, "NtQueryInformationFile");
232-
if (_NtQueryInformationFile == NULL)
138+
if (err != ERROR_ACCESS_DENIED)
233139
{
234-
DWORD err = GetLastError();
235-
236-
CloseHandle(hFile);
237140
_dosmaperr(err);
238141
return -1;
239142
}
240143
}
241144

242-
if (!NT_SUCCESS(_NtQueryInformationFile(hFile, &ioStatus, &standardInfo,
243-
sizeof(standardInfo),
244-
FileStandardInformation)))
245-
{
246-
DWORD err = GetLastError();
247-
248-
CloseHandle(hFile);
249-
_dosmaperr(err);
250-
return -1;
251-
}
252-
#else
253-
if (!GetFileInformationByHandleEx(hFile, FileStandardInfo, &standardInfo,
254-
sizeof(standardInfo)))
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)
255157
{
256158
DWORD err = GetLastError();
257159

258-
CloseHandle(hFile);
259-
_dosmaperr(err);
260-
return -1;
261-
}
262-
#endif /* _WIN32_WINNT < 0x0600 */
263-
264-
if (standardInfo.DeletePending)
265-
{
266160
/*
267-
* File has been deleted, but is not gone from the filesystem yet.
268-
* This can happen when some process with FILE_SHARE_DELETE has it
269-
* open, and it will be fully removed once that handle is closed.
270-
* Meanwhile, we can't open it, so indicate that the file just doesn't
271-
* exist.
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.
272175
*/
273-
CloseHandle(hFile);
274-
errno = ENOENT;
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);
275192
return -1;
276193
}
277194

0 commit comments

Comments
 (0)