pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Check for STATUS_DELETE_PENDING on Windows.
9+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2022-01-11 af9e6331ae ce14dbbcaf 5c0b7581ba Add missing include guard to win32ntdll.h.
* pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2021-12-10 03:22 Thomas Munro <tmunro@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Munro @ 2021-12-10 03:22 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/e2f0f8ed251d02c1eda79e1ca3cb3db2681e7a86
Modified Files
--------------
configure | 6 ++
configure.ac | 1 +
src/backend/port/win32/signal.c | 12 ++-
src/include/port.h | 1 +
src/include/port/win32ntdll.h | 27 +++++++
src/port/open.c | 104 +++++++++++++------------
src/port/win32ntdll.c | 69 +++++++++++++++++
src/port/win32stat.c | 164 ++--------------------------------------
src/tools/msvc/Mkvcbuild.pm | 3 +-
9 files changed, 178 insertions(+), 209 deletions(-)
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-11 19:35 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <tmunro@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2022-01-11 19:35 UTC (permalink / raw)
To: Thomas Munro <tmunro@postgresql.org>; +Cc: pgsql-committers@lists.postgresql.org
Thomas Munro <tmunro@postgresql.org> writes:
> Check for STATUS_DELETE_PENDING on Windows.
The src/include/port/win32ntdll.h file added by this commit has
a couple of deficiencies:
1. It lacks the usual anti-multiple-inclusion guard, i.e.
#ifndef WIN32NTDLL_H
or the like. Was there a specific reason to omit that?
2. headerscheck and cpluspluscheck don't like it, at least
not on non-Windows:
$ src/tools/pginclude/headerscheck
In file included from /tmp/headerscheck.WKh8cz/test.c:2:
./src/include/port/win32ntdll.h:20:10: fatal error: ntstatus.h: No such file or directory
#include <ntstatus.h>
^~~~~~~~~~~~
One way to solve that, perhaps, is to wrap the whole header
in #ifdef WIN32. But I think our more usual practice has
been to add such headers to the exclusion lists in those
two scripts.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-11 21:22 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Munro @ 2022-01-11 21:22 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <tmunro@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On Wed, Jan 12, 2022 at 8:36 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 1. It lacks the usual anti-multiple-inclusion guard, i.e.
> #ifndef WIN32NTDLL_H
> or the like. Was there a specific reason to omit that?
Oops. Fixed.
> 2. headerscheck and cpluspluscheck don't like it, at least
> not on non-Windows:
>
> $ src/tools/pginclude/headerscheck
> In file included from /tmp/headerscheck.WKh8cz/test.c:2:
> ./src/include/port/win32ntdll.h:20:10: fatal error: ntstatus.h: No such file or directory
> #include <ntstatus.h>
> ^~~~~~~~~~~~
>
> One way to solve that, perhaps, is to wrap the whole header
> in #ifdef WIN32. But I think our more usual practice has
> been to add such headers to the exclusion lists in those
> two scripts.
Done.
Those scripts aren't really on my radar... would it be a good idea to
run them as steps in the CompilerWarnings CI task?
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-11 21:44 Andrew Dunstan <andrew@dunslane.net>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Dunstan @ 2022-01-11 21:44 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <tmunro@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On 1/11/22 16:22, Thomas Munro wrote:
> 2. headerscheck and cpluspluscheck don't like it, at least
>> not on non-Windows:
>>
>> $ src/tools/pginclude/headerscheck
>> In file included from /tmp/headerscheck.WKh8cz/test.c:2:
>> ./src/include/port/win32ntdll.h:20:10: fatal error: ntstatus.h: No such file or directory
>> #include <ntstatus.h>
>> ^~~~~~~~~~~~
>>
>> One way to solve that, perhaps, is to wrap the whole header
>> in #ifdef WIN32. But I think our more usual practice has
>> been to add such headers to the exclusion lists in those
>> two scripts.
> Done.
>
> Those scripts aren't really on my radar... would it be a good idea to
> run them as steps in the CompilerWarnings CI task?
>
>
Or in a buildfarm module (c.f. the perl checks)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-11 21:54 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2022-01-11 21:54 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Thomas Munro <tmunro@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
Andrew Dunstan <andrew@dunslane.net> writes:
> On 1/11/22 16:22, Thomas Munro wrote:
>> Those scripts aren't really on my radar... would it be a good idea to
>> run them as steps in the CompilerWarnings CI task?
> Or in a buildfarm module (c.f. the perl checks)
+1 for one or the other, because we break that regularly and it
typically takes awhile to notice.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-13 17:01 Andrew Dunstan <andrew@dunslane.net>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Dunstan @ 2022-01-13 17:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Thomas Munro <tmunro@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
On 1/11/22 16:54, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 1/11/22 16:22, Thomas Munro wrote:
>>> Those scripts aren't really on my radar... would it be a good idea to
>>> run them as steps in the CompilerWarnings CI task?
>> Or in a buildfarm module (c.f. the perl checks)
> +1 for one or the other, because we break that regularly and it
> typically takes awhile to notice.
>
>
Done See
<https://github.com/PGBuildFarm/client-code/commit/51a6f79ec16de87a7eba7a66015985998311d465;
crake will now run this as part of its normal runs on HEAD.
cheers
andew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2022-01-13 17:13 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andrew Dunstan <andrew@dunslane.net>
0 siblings, 0 replies; 9+ messages in thread
From: Tom Lane @ 2022-01-13 17:13 UTC (permalink / raw)
To: Andrew Dunstan <andrew@dunslane.net>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Thomas Munro <tmunro@postgresql.org>; pgsql-committers <pgsql-committers@lists.postgresql.org>
Andrew Dunstan <andrew@dunslane.net> writes:
> Done See
> <https://github.com/PGBuildFarm/client-code/commit/51a6f79ec16de87a7eba7a66015985998311d465;
> crake will now run this as part of its normal runs on HEAD.
Great, thanks!
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2024-11-07 23:19 Andrew Dunstan <andrew@dunslane.net>
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Dunstan @ 2024-11-07 23:19 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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 e2f0f8ed251d02c1eda79e1ca3cb3db2681e7a86)
Author: Thomas Munro <tmunro@postgresql.org>
Author: Alexandra Wang <alexandra.wang.oss@gmail.com>
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/1bf47d89776c2647d896f308687fa13d6c2877e2
Author: Thomas Munro <tmunro@postgresql.org>
Modified Files
--------------
configure | 6 ++
configure.ac | 1 +
src/backend/port/win32/signal.c | 12 ++-
src/include/port.h | 1 +
src/include/port/win32ntdll.h | 27 +++++++
src/port/open.c | 104 ++++++++++++++------------
src/port/win32ntdll.c | 69 +++++++++++++++++
src/port/win32stat.c | 159 ++--------------------------------------
src/tools/msvc/Mkvcbuild.pm | 2 +-
9 files changed, 177 insertions(+), 204 deletions(-)
^ permalink raw reply [nested|flat] 9+ messages in thread
* pgsql: Check for STATUS_DELETE_PENDING on Windows.
@ 2024-11-07 23:19 Andrew Dunstan <andrew@dunslane.net>
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Dunstan @ 2024-11-07 23:19 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
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 e2f0f8ed251d02c1eda79e1ca3cb3db2681e7a86)
Author: Thomas Munro <tmunro@postgresql.org>
Author: Alexandra Wang <alexandra.wang.oss@gmail.com>
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/56b39cce778f93cd95a01df0da083e937424662d
Author: Thomas Munro <tmunro@postgresql.org>
Modified Files
--------------
configure | 6 +++
configure.in | 1 +
src/backend/port/win32/signal.c | 12 ++++-
src/include/port.h | 1 +
src/include/port/win32ntdll.h | 27 +++++++++++
src/port/open.c | 104 +++++++++++++++++++++-------------------
src/port/win32ntdll.c | 69 ++++++++++++++++++++++++++
src/port/win32stat.c | 78 +++---------------------------
src/tools/msvc/Mkvcbuild.pm | 3 +-
9 files changed, 179 insertions(+), 122 deletions(-)
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2024-11-07 23:19 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 03:22 pgsql: Check for STATUS_DELETE_PENDING on Windows. Thomas Munro <tmunro@postgresql.org>
2022-01-11 19:35 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-01-11 21:22 ` Thomas Munro <thomas.munro@gmail.com>
2022-01-11 21:44 ` Andrew Dunstan <andrew@dunslane.net>
2022-01-11 21:54 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-01-13 17:01 ` Andrew Dunstan <andrew@dunslane.net>
2022-01-13 17:13 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-07 23:19 pgsql: Check for STATUS_DELETE_PENDING on Windows. Andrew Dunstan <andrew@dunslane.net>
2024-11-07 23:19 pgsql: Check for STATUS_DELETE_PENDING on Windows. Andrew Dunstan <andrew@dunslane.net>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox