pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
25+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2025-12-29 b3c8119e28 80e8ec772b 45d92b76dc 4da5c33a3a Fix Mkvcbuild.pm builds of test_cloexec.c.
2025-12-21 bec2a0aa30 a7d06e74d5 0451859131 0666ccc16c Clean up test_cloexec.c and Makefile.
2025-12-09 c507ba55f5 bebb281b08 f24af0e04c d62a258cd4 Fix O_CLOEXEC flag handling in Windows port.
* [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
@ 2025-10-31 17:16 Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-10-31 17:16 UTC (permalink / raw)
To: pgsql-hackers
Hello,
While reviewing the pg_ctl CreateProcess patch [1], I started looking
into handle inheritance on Windows and found something that puzzles me.
I think there's an issue with O_CLOEXEC, but wanted to walk through my
reasoning to make sure I'm not missing something obvious.
[1]
https://www.postgresql.org/message-id/flat/TYAPR01MB586654E2D74B838021BE77CAF5EEA@TYAPR01MB5866.jpnp...
The issue appears to be that O_CLOEXEC doesn't actually do anything on
Windows. When PostgreSQL opens a WAL file in xlog.c, it specifies
O_CLOEXEC in the OpenTransientFile() call, expecting the file handle to
be non-inheritable. However, O_CLOEXEC is defined as 0 in win32_port.h,
and our pgwin32_open() function in src/port/open.c unconditionally sets
sa.bInheritHandle = TRUE at line 80. So the flag is simply ignored, and
all file handles are created inheritable.
Now, for a handle to actually be inherited by a child process on
Windows, two conditions must both be true. First, the handle itself must
have been created with bInheritHandle = TRUE (which we do for
everything). Second, the parent must call CreateProcess with
bInheritHandles = TRUE. So the question becomes: does that second
condition ever happen?
It does. When archive_command runs, PostgreSQL calls pgwin32_system(),
which wraps the command in quotes and passes it to the Microsoft C
runtime's system() function. That function needs to make stdio work for
the child process, so it has no choice but to call CreateProcess with
bInheritHandles = TRUE. This means cmd.exe inherits all our file
handles, including any open database or WAL files, and cmd.exe passes
them along to copy.exe or whatever command is being run.
I wrote a test program that links against libpgport.a to verify this
behavior. It opens files with and without O_CLOEXEC using the actual
pgwin32_open() function, then spawns a child process with
bInheritHandles = TRUE (mimicking what system() does) and tries to
access the handles from the child. Both files were accessible from the
child process regardless of whether O_CLOEXEC was specified. The flag
has no effect.
Commit 1da569ca1f (March 2023) added O_CLOEXEC to many call sites
throughout the backend with a comment saying "Our open() replacement
does not create inheritable handles, so it is safe to ignore
O_CLOEXEC." But that doesn't appear to match what the code actually
does. I'm wondering if I've misunderstood something about how handle
inheritance works on Windows, or if the comment was based on a
misunderstanding of the code path.
The practical impact of this seems low. Child processes spawned by
archive_command or COPY PROGRAM operate on file paths passed as
arguments, not on inherited file descriptors, so they don't actually
make use of the handles even though they have them. Even if a child
wanted to use an inherited handle, it would need to somehow learn the
numeric handle value, which isn't passed through our IPC mechanisms.
And Windows users probably employ archive_command less frequently than
Unix users anyway.
Nonetheless, if this is really a bug, it's a correctness issue. It
violates the documented semantics of O_CLOEXEC, contradicts what our
own comments claim, and means PostgreSQL behaves differently on Windows
than on Unix. It also creates unnecessary handle leaks where files
can't be deleted or renamed while child processes are running. While
reviewing my pg_ctl patch, I realized it would make handle inheritance
more explicit and direct, which made me want to understand whether
O_CLOEXEC actually works.
The fix would be straightforward if this is indeed wrong. Define
O_CLOEXEC to a non-zero value like 0x80000 (in the private use range
for open() flags), and then honor it in pgwin32_open() by setting
sa.bInheritHandle based on whether the flag is present:
sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
We also have to add the O_CLOEXEC to the assertion in open.c that
validates that fileFlags only contains known flags.
I've tested this change locally and it works as expected. Files opened
with O_CLOEXEC are not accessible from child processes, while files
opened without it are accessible.
So my questions are: Am I correct that both conditions for handle
inheritance are met, meaning handles really are being inherited by
archive_command children? Is there something in Windows that prevents
inheritance that I don't know about? If this is a real bug, would it
make sense to backpatch to v16 where O_CLOEXEC was added? I'm happy to
provide my test code or do additional testing if that would help.
For reference, the Microsoft documentation on handle inheritance is at:
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createpro...
And I confirmed through research that UCRT's system() does use
bInheritHandles = TRUE, which makes sense since it needs stdio to work.
Bryan Green
Attachments:
0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/plain, 3.8K) download
From 095dd7af43c81f55270db71798ee2f0248f1086e Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 31 Oct 2025 10:11:30 -0600
Subject: [PATCH] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it appears this
may have been based on a misunderstanding of the code path. In
practice, the code was creating inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
use range for open() flags). Then honor it in pgwin32_open_handle()
by setting bInheritHandle according to whether O_CLOEXEC is specified.
We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
Back-patch to v16 where commit 1da569ca1f introduced widespread use of
O_CLOEXEC. While the practical risk is low, having code work as
documented and consistently across platforms is worth the minimal risk.
Bug found while reviewing an unrelated patch to pg_ctl's process
spawning logic.
Author: Bryan Green <dbryan.green@gmail.com>
---
src/include/port/win32_port.h | 2 +-
src/port/open.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index ff7028bdc8..07fece6c46 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -346,7 +346,7 @@ extern int _pglstat64(const char *name, struct stat *buf);
* ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
* necessary to convert this to _O_NOINHERIT.)
*/
-#define O_CLOEXEC 0
+#define O_CLOEXEC 0x80000
/*
* Supplement to <errno.h>.
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b7..71344b9917 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
--
2.46.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-06 13:43 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-11-06 13:43 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
On Sat, Nov 1, 2025 at 6:16 AM Bryan Green <dbryan.green@gmail.com> wrote:
> Hello,
Catching up with all your emails, and I must say it's great to see
some solid investigation of PostgreSQL-on-Windows problems. There are
... more.
> Commit 1da569ca1f (March 2023) added O_CLOEXEC to many call sites
> throughout the backend with a comment saying "Our open() replacement
> does not create inheritable handles, so it is safe to ignore
> O_CLOEXEC." But that doesn't appear to match what the code actually
> does. I'm wondering if I've misunderstood something about how handle
> inheritance works on Windows, or if the comment was based on a
> misunderstanding of the code path.
Yeah, it looks like I was just wrong. Oops. Your analysis looks good to me.
> The fix would be straightforward if this is indeed wrong. Define
> O_CLOEXEC to a non-zero value like 0x80000 (in the private use range
> for open() flags), and then honor it in pgwin32_open() by setting
> sa.bInheritHandle based on whether the flag is present:
>
> sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
Looking at fcntl.h, that's the next free bit, but also the one they'll
presumably define next (I guess "private use range" is just a turn of
phrase and not a real thing?), so why not use the highest free bit
after O_DIRECT? We have three fake open flags, one of which
cybersquats a real flag from fcntl.h, ironically the one that actually
means O_CLOEXEC. We can't change existing values in released
branches, so that'd give:
#define O_DIRECT 0x80000000
#define O_CLOEXEC 0x04000000
#define O_DSYNC _O_NO_INHERIT
Perhaps in master we could rearrange them:
#define O_DIRECT 0x80000000
#define O_DSYNC 0x04000000
#define O_CLOEXEC _O_NO_INHERIT
> So my questions are: Am I correct that both conditions for handle
> inheritance are met, meaning handles really are being inherited by
> archive_command children? Is there something in Windows that prevents
> inheritance that I don't know about? If this is a real bug, would it
> make sense to backpatch to v16 where O_CLOEXEC was added? I'm happy to
> provide my test code or do additional testing if that would help.
Yeah, seems like it, and like we should back-patch this. I vote for
doing that after the upcoming minor releases. Holding files open on
Windows unintentionally is worse on Windows than on Unix (preventing
directories from being unlinked etc). Of course we've done that for
decades so I doubt it's really a big deal, but we should clean up this
mistake.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-11-06 14:42 ` Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-11-06 14:42 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 11/6/2025 7:43 AM, Thomas Munro wrote:
> On Sat, Nov 1, 2025 at 6:16 AM Bryan Green <dbryan.green@gmail.com> wrote:
>> Hello,
>
> Catching up with all your emails, and I must say it's great to see
> some solid investigation of PostgreSQL-on-Windows problems. There are
> ... more.
>
>> Commit 1da569ca1f (March 2023) added O_CLOEXEC to many call sites
>> throughout the backend with a comment saying "Our open() replacement
>> does not create inheritable handles, so it is safe to ignore
>> O_CLOEXEC." But that doesn't appear to match what the code actually
>> does. I'm wondering if I've misunderstood something about how handle
>> inheritance works on Windows, or if the comment was based on a
>> misunderstanding of the code path.
>
> Yeah, it looks like I was just wrong. Oops. Your analysis looks good to me.
>
>> The fix would be straightforward if this is indeed wrong. Define
>> O_CLOEXEC to a non-zero value like 0x80000 (in the private use range
>> for open() flags), and then honor it in pgwin32_open() by setting
>> sa.bInheritHandle based on whether the flag is present:
>>
>> sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>
> Looking at fcntl.h, that's the next free bit, but also the one they'll
> presumably define next (I guess "private use range" is just a turn of
> phrase and not a real thing?), so why not use the highest free bit
> after O_DIRECT? We have three fake open flags, one of which
> cybersquats a real flag from fcntl.h, ironically the one that actually
> means O_CLOEXEC. We can't change existing values in released
> branches, so that'd give:
>
> #define O_DIRECT 0x80000000
> #define O_CLOEXEC 0x04000000
> #define O_DSYNC _O_NO_INHERIT
>
> Perhaps in master we could rearrange them:
>
> #define O_DIRECT 0x80000000
> #define O_DSYNC 0x04000000
> #define O_CLOEXEC _O_NO_INHERIT
>
>> So my questions are: Am I correct that both conditions for handle
>> inheritance are met, meaning handles really are being inherited by
>> archive_command children? Is there something in Windows that prevents
>> inheritance that I don't know about? If this is a real bug, would it
>> make sense to backpatch to v16 where O_CLOEXEC was added? I'm happy to
>> provide my test code or do additional testing if that would help.
>
> Yeah, seems like it, and like we should back-patch this. I vote for
> doing that after the upcoming minor releases. Holding files open on
> Windows unintentionally is worse on Windows than on Unix (preventing
> directories from being unlinked etc). Of course we've done that for
> decades so I doubt it's really a big deal, but we should clean up this
> mistake.
Thanks for reviewing this and confirming the analysis. Good to know I
wasn't missing something about Windows handle inheritance.
Your point about the bit value makes sense - using 0x04000000 (highest
free bit after O_DIRECT) is definitely safer than 0x80000 which could
collide with future fcntl.h additions. I also appreciate the irony you
pointed out - we're currently using _O_NO_INHERIT (which literally
prevents handle inheritance on Windows) for O_DSYNC instead of
O_CLOEXEC. The rearrangement in master to use _O_NO_INHERIT for what it
actually means semantically makes a lot of sense.
So the plan would be:
Backport branches (v16+):
#define O_DIRECT 0x80000000
#define O_CLOEXEC 0x04000000
#define O_DSYNC _O_NO_INHERIT
Master:
#define O_DIRECT 0x80000000
#define O_DSYNC 0x04000000
#define O_CLOEXEC _O_NO_INHERIT
And then in pgwin32_open():
sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
I will prepare a new version of the patch that implements the suggested
change for master.
--
Bryan Green
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-07 17:28 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-11-07 17:28 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 11/6/2025 8:42 AM, Bryan Green wrote:
> On 11/6/2025 7:43 AM, Thomas Munro wrote:
>> On Sat, Nov 1, 2025 at 6:16 AM Bryan Green <dbryan.green@gmail.com> wrote:
>>> Hello,
>>
>> Catching up with all your emails, and I must say it's great to see
>> some solid investigation of PostgreSQL-on-Windows problems. There are
>> ... more.
>>
>>> Commit 1da569ca1f (March 2023) added O_CLOEXEC to many call sites
>>> throughout the backend with a comment saying "Our open() replacement
>>> does not create inheritable handles, so it is safe to ignore
>>> O_CLOEXEC." But that doesn't appear to match what the code actually
>>> does. I'm wondering if I've misunderstood something about how handle
>>> inheritance works on Windows, or if the comment was based on a
>>> misunderstanding of the code path.
>>
>> Yeah, it looks like I was just wrong. Oops. Your analysis looks good to me.
>>
>>> The fix would be straightforward if this is indeed wrong. Define
>>> O_CLOEXEC to a non-zero value like 0x80000 (in the private use range
>>> for open() flags), and then honor it in pgwin32_open() by setting
>>> sa.bInheritHandle based on whether the flag is present:
>>>
>>> sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>>
>> Looking at fcntl.h, that's the next free bit, but also the one they'll
>> presumably define next (I guess "private use range" is just a turn of
>> phrase and not a real thing?), so why not use the highest free bit
>> after O_DIRECT? We have three fake open flags, one of which
>> cybersquats a real flag from fcntl.h, ironically the one that actually
>> means O_CLOEXEC. We can't change existing values in released
>> branches, so that'd give:
>>
>> #define O_DIRECT 0x80000000
>> #define O_CLOEXEC 0x04000000
>> #define O_DSYNC _O_NO_INHERIT
>>
>> Perhaps in master we could rearrange them:
>>
>> #define O_DIRECT 0x80000000
>> #define O_DSYNC 0x04000000
>> #define O_CLOEXEC _O_NO_INHERIT
>>
>>> So my questions are: Am I correct that both conditions for handle
>>> inheritance are met, meaning handles really are being inherited by
>>> archive_command children? Is there something in Windows that prevents
>>> inheritance that I don't know about? If this is a real bug, would it
>>> make sense to backpatch to v16 where O_CLOEXEC was added? I'm happy to
>>> provide my test code or do additional testing if that would help.
>>
>> Yeah, seems like it, and like we should back-patch this. I vote for
>> doing that after the upcoming minor releases. Holding files open on
>> Windows unintentionally is worse on Windows than on Unix (preventing
>> directories from being unlinked etc). Of course we've done that for
>> decades so I doubt it's really a big deal, but we should clean up this
>> mistake.
>
> Thanks for reviewing this and confirming the analysis. Good to know I
> wasn't missing something about Windows handle inheritance.
>
> Your point about the bit value makes sense - using 0x04000000 (highest
> free bit after O_DIRECT) is definitely safer than 0x80000 which could
> collide with future fcntl.h additions. I also appreciate the irony you
> pointed out - we're currently using _O_NO_INHERIT (which literally
> prevents handle inheritance on Windows) for O_DSYNC instead of
> O_CLOEXEC. The rearrangement in master to use _O_NO_INHERIT for what it
> actually means semantically makes a lot of sense.
>
> So the plan would be:
>
> Backport branches (v16+):
> #define O_DIRECT 0x80000000
> #define O_CLOEXEC 0x04000000
> #define O_DSYNC _O_NO_INHERIT
>
> Master:
> #define O_DIRECT 0x80000000
> #define O_DSYNC 0x04000000
> #define O_CLOEXEC _O_NO_INHERIT
>
> And then in pgwin32_open():
> sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>
> I will prepare a new version of the patch that implements the suggested
> change for master.
>
>
The changes for master, along with a tap test, are provided with the
attached patch.
--
Bryan Green
EDB: https://www.enterprisedb.com
Attachments:
v2-0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/plain, 19.2K) download
From bea32ccc8275d5df8f6b7eb5e4b7b37124078873 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 31 Oct 2025 10:11:30 -0600
Subject: [PATCH v2] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it appears this
may have been based on a misunderstanding of the code path. In
practice, the code was creating inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
use range for open() flags). Then honor it in pgwin32_open_handle()
by setting bInheritHandle according to whether O_CLOEXEC is specified.
We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
Author: Bryan Green <dbryan.green@gmail.com>
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/README | 105 +++++++
src/test/modules/test_cloexec/meson.build | 27 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
9 files changed, 510 insertions(+), 11 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/README
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index ff7028bdc8..ad442e1de7 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -335,19 +335,18 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
+ *
*/
-#define O_DSYNC 0x0080
+#define O_DIRECT 0x80000000
+#define O_DSYNC 0x04000000
/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+ * Map O_CLOEXEC to Windows' _O_NOINHERIT flag. Our src/port/open.c
+ * explicitly checks for this flag and sets bInheritHandle accordingly
+ * when calling CreateFile(). Note that CreateFile() doesn't understand
+ * this flag natively - we must handle it in our wrapper.
*/
-#define O_CLOEXEC 0
-
+#define O_CLOEXEC _O_NOINHERIT
/*
* Supplement to <errno.h>.
*
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b7..71344b9917 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 902a795410..3a8e25fafd 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -18,6 +18,7 @@ SUBDIRS = \
test_binaryheap \
test_bitmapset \
test_bloomfilter \
+ test_cloexec \
test_copy_callbacks \
test_custom_rmgrs \
test_ddl_deparse \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 14fc761c4c..e9309ecb3e 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -17,6 +17,7 @@ subdir('test_aio')
subdir('test_binaryheap')
subdir('test_bitmapset')
subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 0000000000..af885dd3a8
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/README b/src/test/modules/test_cloexec/README
new file mode 100644
index 0000000000..1d518eed02
--- /dev/null
+++ b/src/test/modules/test_cloexec/README
@@ -0,0 +1,105 @@
+test_cloexec - Test O_CLOEXEC flag handling on Windows
+=======================================================
+
+This test module verifies that the O_CLOEXEC flag works correctly on Windows
+after the fix in src/port/open.c.
+
+Test Overview
+-------------
+
+The test verifies two critical behaviors:
+
+1. File handles opened WITH O_CLOEXEC are NOT inherited by child processes
+2. File handles opened WITHOUT O_CLOEXEC ARE inherited by child processes
+
+How It Works
+------------
+
+The test consists of a standalone C program (test_cloexec.c) that:
+
+1. Parent Process:
+ - Opens two test files:
+ * File 1: with O_CLOEXEC flag (should NOT be inherited)
+ * File 2: without O_CLOEXEC flag (should be inherited)
+ - Gets the Windows HANDLE values for both file descriptors
+ - Spawns a child process with bInheritHandles=TRUE
+ - Passes both HANDLE values to the child as command-line arguments
+ - Waits for the child to complete and checks the exit code
+
+2. Child Process:
+ - Receives the two HANDLE values as hex strings
+ - Attempts to write to both handles using WriteFile()
+ - Reports which handles were accessible
+ - Exits with status 0 if O_CLOEXEC worked correctly:
+ * Handle 1 (O_CLOEXEC) should fail (not inherited)
+ * Handle 2 (no O_CLOEXEC) should succeed (inherited)
+
+Test Files
+----------
+
+- test_cloexec.c - The test program (parent and child logic)
+- t/001_cloexec.pl - TAP test script that runs the program
+- Makefile - Build rules for make-based builds
+- meson.build - Build rules for meson-based builds
+
+Integration with PostgreSQL
+----------------------------
+
+This test should be placed in:
+ src/test/modules/test_cloexec/
+
+The test is Windows-only and will be skipped on other platforms.
+
+To run the test:
+ make check
+ or
+ meson test test_cloexec
+
+Expected Output
+---------------
+
+On success, the test program outputs:
+ Parent: Opening test files...
+ Parent: fd1=3 (O_CLOEXEC) -> HANDLE=0x...
+ Parent: fd2=4 (no O_CLOEXEC) -> HANDLE=0x...
+ Parent: Spawning child process...
+ Parent: Waiting for child process...
+ Child: Received HANDLE1=0x... (should fail - O_CLOEXEC)
+ Child: Received HANDLE2=0x... (should work - no O_CLOEXEC)
+ Child: Failed to write to HANDLE1 (error 6)
+ Child: Successfully wrote to HANDLE2
+ Child: HANDLE1 (O_CLOEXEC): NOT ACCESSIBLE (GOOD!)
+ Child: HANDLE2 (no O_CLOEXEC): ACCESSIBLE (GOOD!)
+ Child: Test PASSED - O_CLOEXEC working correctly
+ Parent: Child exit code: 0
+ Parent: SUCCESS - O_CLOEXEC behavior verified
+
+Technical Details
+-----------------
+
+The test uses Windows-specific APIs:
+
+- _get_osfhandle() - Convert CRT file descriptor to Windows HANDLE
+- CreateProcess() - Spawn child with bInheritHandles=TRUE
+- WriteFile() - Attempt to write to handles in child process
+
+The test relies on the fact that:
+- Windows Error 6 (ERROR_INVALID_HANDLE) indicates the handle is not accessible
+- Inherited handles work across process boundaries when properly set up
+- O_CLOEXEC should prevent inheritance by setting bInheritHandle=FALSE
+
+Relation to the Patch
+---------------------
+
+This test validates the changes made in the O_CLOEXEC patch:
+
+1. src/include/port/win32_port.h:
+ - Changed O_CLOEXEC from 0 to _O_NOINHERIT (0x80)
+
+2. src/port/open.c (pgwin32_open_handle):
+ - Added O_CLOEXEC to the assertion of handled flags
+ - Changed bInheritHandle from always TRUE to conditional:
+ sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
+
+Without these changes, both handles would be inherited by the child,
+and the test would fail.
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 0000000000..f6a3087bfe
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,27 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ include_directories: [postgres_inc],
+ link_with: [pgport[''], pgcommon['']],
+ c_args: ['-DFRONTEND'],
+ install: false,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 0000000000..4c56eb3a4b
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 0000000000..96cbd9c80e
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-12 01:02 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-11-12 01:02 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 11/7/2025 11:28 AM, Bryan Green wrote:
> On 11/6/2025 8:42 AM, Bryan Green wrote:
>> On 11/6/2025 7:43 AM, Thomas Munro wrote:
...
>> So the plan would be:
>>
>> Backport branches (v16+):
>> #define O_DIRECT 0x80000000
>> #define O_CLOEXEC 0x04000000
>> #define O_DSYNC _O_NO_INHERIT
>>
>> Master:
>> #define O_DIRECT 0x80000000
>> #define O_DSYNC 0x04000000
>> #define O_CLOEXEC _O_NO_INHERIT
>>
>> And then in pgwin32_open():
>> sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>>
>> I will prepare a new version of the patch that implements the suggested
>> change for master.
>>
>>
> The changes for master, along with a tap test, are provided with the
> attached patch.
>
Thanks to CI discovered a mistake in the makefile and meson.build file
for the tests. New patch attached.
--
Bryan Green
EDB: https://www.enterprisedb.com
Attachments:
v3-0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/plain, 15.3K) download
From 4738c4d8cc91d5f9d2d71bf6829bc9437e2728ed Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 31 Oct 2025 10:11:30 -0600
Subject: [PATCH v3] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it appears this
may have been based on a misunderstanding of the code path. In
practice, the code was creating inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
use range for open() flags). Then honor it in pgwin32_open_handle()
by setting bInheritHandle according to whether O_CLOEXEC is specified.
We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
Author: Bryan Green <dbryan.green@gmail.com>
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
8 files changed, 404 insertions(+), 11 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index ff7028bdc8..ad442e1de7 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -335,19 +335,18 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
+ *
*/
-#define O_DSYNC 0x0080
+#define O_DIRECT 0x80000000
+#define O_DSYNC 0x04000000
/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+ * Map O_CLOEXEC to Windows' _O_NOINHERIT flag. Our src/port/open.c
+ * explicitly checks for this flag and sets bInheritHandle accordingly
+ * when calling CreateFile(). Note that CreateFile() doesn't understand
+ * this flag natively - we must handle it in our wrapper.
*/
-#define O_CLOEXEC 0
-
+#define O_CLOEXEC _O_NOINHERIT
/*
* Supplement to <errno.h>.
*
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b7..71344b9917 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 902a795410..3a8e25fafd 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -18,6 +18,7 @@ SUBDIRS = \
test_binaryheap \
test_bitmapset \
test_bloomfilter \
+ test_cloexec \
test_copy_callbacks \
test_custom_rmgrs \
test_ddl_deparse \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 14fc761c4c..e9309ecb3e 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -17,6 +17,7 @@ subdir('test_aio')
subdir('test_binaryheap')
subdir('test_bitmapset')
subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 0000000000..70d38575e2
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 0000000000..63c8658b04
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 0000000000..4c56eb3a4b
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 0000000000..96cbd9c80e
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-12 02:34 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-11-12 02:34 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
On Wed, Nov 12, 2025 at 2:01 PM Bryan Green <dbryan.green@gmail.com> wrote:
> [v3]
"The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it appears this
may have been based on a misunderstanding of the code path. In
practice, the code was creating inheritable handles in all cases."
s/it appears this may have been been/was/ :-)
"To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
use range for open() flags). Then honor it in pgwin32_open_handle()"
Out of date, maybe skip mentioning the value in the commit message?
Maybe add a note here about the back-branches preserving existing
values and master cleaning up? Do you happen to have a fixup that
supplies the difference in the back-branches so we can see it passing
in CI?
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
It also wouldn't be thread-safe. That is meaningful today for
frontend programs (and hopefully some day soon also in the backend).
+ sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
Just out of sheer curiosity, I often see gratuitous FALSE and TRUE in
Windowsian code, not false and true and not reduced to eg !(fileFlags
& O_CLOEXEC). Is that a code convention thing from somewhere in
Windows-land?
+++ b/src/test/modules/test_cloexec/test_cloexec.c
I like the test. Very helpful for people reliant on CI for Windows coverage.
Independently of all this, and just on the off-chance that it might be
interesting to you in future, I have previously tried to write tests
for our whole Windows filesystem shim layer and found lots of bugs and
understood lots of quirks that way, but never got it to be good enough
for inclusion in the tree:
https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg...
There is some overlap with several of your recent patches, as I was
testing some of the same wrappers. One of the main things we've
battled with in this project is the whole asynchronous-unlink thing,
and generally the NT/VMS file locking concept which can't quite be
entirely emulated away, and that was one of my main focuses there
after we got CI and started debugging the madness. Doing so revealed
a bunch of interesting differences in Windows versions and file
systems, and to this day we don't really have a project policy on
which Windows filesystems PostgreSQL supports (cf your mention of
needing NTFS for sparse files in one of your other threads, though I
can't imagine that ReFS AKA DevDrive doesn't have those too being a
COW system).
Speaking of file I/O, and just as an FYI, I have a bunch of
semi-working unfinished patches that bring true scatter/gather I/O
(instead of the simple looping fallback in pg_preadv()) and native
async I/O (for files, but actually also pipes and sockets but let me
stick to talking about files for now) to Windows (traditional
OVERLAPPED and/or IoRing.h, neither can do everything we need would
you believe). Development via trial-by-CI from the safety of my Unix
box is slow and painful going, but... let's say a real Windows
programmer with a systems programming bent showed up and were
interested in this stuff, I would be more than happy to write down
everything I think I know about those subjects and post the unfinished
work and then I bet development would go fast... just sayin'.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-11-12 22:10 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-11-12 22:10 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 11/11/2025 8:34 PM, Thomas Munro wrote:
> On Wed, Nov 12, 2025 at 2:01 PM Bryan Green <dbryan.green@gmail.com> wrote:
>> [v3]
>
> "The original commit included a comment suggesting that our open()
> replacement doesn't create inheritable handles, but it appears this
> may have been based on a misunderstanding of the code path. In
> practice, the code was creating inheritable handles in all cases."
>
> s/it appears this may have been been/was/ :-)
>
Changed.
> "To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
> use range for open() flags). Then honor it in pgwin32_open_handle()"
>
Removed.
> Out of date, maybe skip mentioning the value in the commit message?
> Maybe add a note here about the back-branches preserving existing
> values and master cleaning up? Do you happen to have a fixup that
> supplies the difference in the back-branches so we can see it passing
> in CI?
>
I have attached a back-patch for v16-v18.
> + * Note: We could instead use SetHandleInformation() after CreateFile() to
> + * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
> + * and achieves the same result.
> + */
>
> It also wouldn't be thread-safe. That is meaningful today for
> frontend programs (and hopefully some day soon also in the backend).
>
> + sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>
> Just out of sheer curiosity, I often see gratuitous FALSE and TRUE in
> Windowsian code, not false and true and not reduced to eg !(fileFlags
> & O_CLOEXEC). Is that a code convention thing from somewhere in
> Windows-land?
>
Yes, old habits die hard. I learned this pattern on Windows.
Interestingly, enough when I am not on Windows I write the way you suggest.
> +++ b/src/test/modules/test_cloexec/test_cloexec.c
>
> I like the test. Very helpful for people reliant on CI for Windows coverage.
>
> Independently of all this, and just on the off-chance that it might be
> interesting to you in future, I have previously tried to write tests
> for our whole Windows filesystem shim layer and found lots of bugs and
> understood lots of quirks that way, but never got it to be good enough
> for inclusion in the tree:
>
> https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg...
>
I shall take a look.
> There is some overlap with several of your recent patches, as I was
> testing some of the same wrappers. One of the main things we've
> battled with in this project is the whole asynchronous-unlink thing,
> and generally the NT/VMS file locking concept which can't quite be
> entirely emulated away, and that was one of my main focuses there
> after we got CI and started debugging the madness. Doing so revealed
> a bunch of interesting differences in Windows versions and file
> systems, and to this day we don't really have a project policy on
> which Windows filesystems PostgreSQL supports (cf your mention of
> needing NTFS for sparse files in one of your other threads, though I
> can't imagine that ReFS AKA DevDrive doesn't have those too being a
> COW system).
>
> Speaking of file I/O, and just as an FYI, I have a bunch of
> semi-working unfinished patches that bring true scatter/gather I/O
> (instead of the simple looping fallback in pg_preadv()) and native
> async I/O (for files, but actually also pipes and sockets but let me
> stick to talking about files for now) to Windows (traditional
> OVERLAPPED and/or IoRing.h, neither can do everything we need would
> you believe). Development via trial-by-CI from the safety of my Unix
> box is slow and painful going, but... let's say a real Windows
> programmer with a systems programming bent showed up and were
> interested in this stuff, I would be more than happy to write down
> everything I think I know about those subjects and post the unfinished
> work and then I bet development would go fast... just sayin'.
I would absolutely love to read everything you think you know about
those subjects and contribute to the work.
--
Bryan Green
EDB: https://www.enterprisedb.com
Attachments:
0001-Fix-O_CLOEXEC-v16-v17-v18.patch.txt (text/plain, 14.7K) download
From 87efaf828d14ed70819ba660c2d4586061e22a2d Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Wed, 12 Nov 2025 13:10:35 -0600
Subject: [PATCH] Fix O_CLOEXEC flag handling in Windows port
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it was a mis-
understanding of the code path. In practice, the code was creating
inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, honor O_CLOEXEC in pgwin32_open_handle() by setting
bInheritHandle based on whether O_CLOEXEC is specified. We set
bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
For this backpatch, we use 0x04000000 as the O_CLOEXEC value to avoid
changing existing flag definitions. Master will use _O_NOINHERIT
and reorganize the flags cleanly.
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
7 files changed, 402 insertions(+), 12 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index fe9253d9b1f..b919f64e9a8 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -351,18 +351,15 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
*/
-#define O_DSYNC 0x0080
-
-/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+#define O_DIRECT 0x80000000
+/* Our src/port/open.c explicitly checks for O_CLOEXEC and sets
+ * bInheritHandle accordingly when calling CreateFile().
+ * Note that CreateFile() doesn't understand this flag natively - we must
+ * handle it in our wrapper.
*/
-#define O_CLOEXEC 0
+#define O_CLOEXEC 0x04000000
+#define O_DSYNC 0x0080
/*
* Supplement to <errno.h>.
diff --git a/src/port/open.c b/src/port/open.c
index 0bd97550063..81b356ff7ae 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = !(fileFlags & O_CLOEXEC);
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 9556bac4ad4..55ec0bf903b 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -15,3 +15,4 @@ subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 00000000000..70d38575e26
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 00000000000..63c8658b04e
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 00000000000..4c56eb3a4be
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 00000000000..96cbd9c80e6
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
v3-0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/plain, 15.2K) download
From 4738c4d8cc91d5f9d2d71bf6829bc9437e2728ed Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 31 Oct 2025 10:11:30 -0600
Subject: [PATCH v3] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it was a mis-
understanding of the code path. In practice, the code was creating
inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to _O_NOINHERIT in master. We will define it
to a high available value for back branches. In pgwin32_open_handle()
we set bInheritHandle according to whether O_CLOEXEC is specified.
We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
Author: Bryan Green <dbryan.green@gmail.com>
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
8 files changed, 404 insertions(+), 11 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index ff7028bdc8..ad442e1de7 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -335,19 +335,18 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
+ *
*/
-#define O_DSYNC 0x0080
+#define O_DIRECT 0x80000000
+#define O_DSYNC 0x04000000
/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+ * Map O_CLOEXEC to Windows' _O_NOINHERIT flag. Our src/port/open.c
+ * explicitly checks for this flag and sets bInheritHandle accordingly
+ * when calling CreateFile(). Note that CreateFile() doesn't understand
+ * this flag natively - we must handle it in our wrapper.
*/
-#define O_CLOEXEC 0
-
+#define O_CLOEXEC _O_NOINHERIT
/*
* Supplement to <errno.h>.
*
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b7..71344b9917 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = !(fileFlags & O_CLOEXEC);
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 902a795410..3a8e25fafd 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -18,6 +18,7 @@ SUBDIRS = \
test_binaryheap \
test_bitmapset \
test_bloomfilter \
+ test_cloexec \
test_copy_callbacks \
test_custom_rmgrs \
test_ddl_deparse \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 14fc761c4c..e9309ecb3e 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -17,6 +17,7 @@ subdir('test_aio')
subdir('test_binaryheap')
subdir('test_bitmapset')
subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 0000000000..70d38575e2
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 0000000000..63c8658b04
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 0000000000..4c56eb3a4b
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 0000000000..96cbd9c80e
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-12 22:17 ` Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-11-12 22:17 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 11/12/2025 4:10 PM, Bryan Green wrote:
> On 11/11/2025 8:34 PM, Thomas Munro wrote:
>> On Wed, Nov 12, 2025 at 2:01 PM Bryan Green <dbryan.green@gmail.com> wrote:
>>> [v3]
>>
>> "The original commit included a comment suggesting that our open()
>> replacement doesn't create inheritable handles, but it appears this
>> may have been based on a misunderstanding of the code path. In
>> practice, the code was creating inheritable handles in all cases."
>>
>> s/it appears this may have been been/was/ :-)
>>
>
> Changed.
>
>> "To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private
>> use range for open() flags). Then honor it in pgwin32_open_handle()"
>>
>
> Removed.
>> Out of date, maybe skip mentioning the value in the commit message?
>> Maybe add a note here about the back-branches preserving existing
>> values and master cleaning up? Do you happen to have a fixup that
>> supplies the difference in the back-branches so we can see it passing
>> in CI?
>>
>
> I have attached a back-patch for v16-v18.
>
>> + * Note: We could instead use SetHandleInformation() after CreateFile() to
>> + * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
>> + * and achieves the same result.
>> + */
>>
>> It also wouldn't be thread-safe. That is meaningful today for
>> frontend programs (and hopefully some day soon also in the backend).
>>
>> + sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE;
>>
>> Just out of sheer curiosity, I often see gratuitous FALSE and TRUE in
>> Windowsian code, not false and true and not reduced to eg !(fileFlags
>> & O_CLOEXEC). Is that a code convention thing from somewhere in
>> Windows-land?
>>
>
> Yes, old habits die hard. I learned this pattern on Windows.
> Interestingly, enough when I am not on Windows I write the way you suggest.
>
>> +++ b/src/test/modules/test_cloexec/test_cloexec.c
>>
>> I like the test. Very helpful for people reliant on CI for Windows coverage.
>>
>> Independently of all this, and just on the off-chance that it might be
>> interesting to you in future, I have previously tried to write tests
>> for our whole Windows filesystem shim layer and found lots of bugs and
>> understood lots of quirks that way, but never got it to be good enough
>> for inclusion in the tree:
>>
>> https://www.postgresql.org/message-id/flat/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg...
>>
>
> I shall take a look.
>
>> There is some overlap with several of your recent patches, as I was
>> testing some of the same wrappers. One of the main things we've
>> battled with in this project is the whole asynchronous-unlink thing,
>> and generally the NT/VMS file locking concept which can't quite be
>> entirely emulated away, and that was one of my main focuses there
>> after we got CI and started debugging the madness. Doing so revealed
>> a bunch of interesting differences in Windows versions and file
>> systems, and to this day we don't really have a project policy on
>> which Windows filesystems PostgreSQL supports (cf your mention of
>> needing NTFS for sparse files in one of your other threads, though I
>> can't imagine that ReFS AKA DevDrive doesn't have those too being a
>> COW system).
>>
>> Speaking of file I/O, and just as an FYI, I have a bunch of
>> semi-working unfinished patches that bring true scatter/gather I/O
>> (instead of the simple looping fallback in pg_preadv()) and native
>> async I/O (for files, but actually also pipes and sockets but let me
>> stick to talking about files for now) to Windows (traditional
>> OVERLAPPED and/or IoRing.h, neither can do everything we need would
>> you believe). Development via trial-by-CI from the safety of my Unix
>> box is slow and painful going, but... let's say a real Windows
>> programmer with a systems programming bent showed up and were
>> interested in this stuff, I would be more than happy to write down
>> everything I think I know about those subjects and post the unfinished
>> work and then I bet development would go fast... just sayin'.
>
> I would absolutely love to read everything you think you know about
> those subjects and contribute to the work.
>
>
Corrected master patch and back patch for v16-v18.
--
Bryan Green
EDB: https://www.enterprisedb.com
Attachments:
v4-0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/plain, 15.2K) download
From 4738c4d8cc91d5f9d2d71bf6829bc9437e2728ed Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 31 Oct 2025 10:11:30 -0600
Subject: [PATCH v4] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it was a mis-
understanding of the code path. In practice, the code was creating
inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to _O_NOINHERIT in master. We will define it
to a high available value for back branches. In pgwin32_open_handle()
we set bInheritHandle according to whether O_CLOEXEC is specified.
We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
Author: Bryan Green <dbryan.green@gmail.com>
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
8 files changed, 404 insertions(+), 11 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index ff7028bdc8..ad442e1de7 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -335,19 +335,18 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
+ *
*/
-#define O_DSYNC 0x0080
+#define O_DIRECT 0x80000000
+#define O_DSYNC 0x04000000
/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+ * Map O_CLOEXEC to Windows' _O_NOINHERIT flag. Our src/port/open.c
+ * explicitly checks for this flag and sets bInheritHandle accordingly
+ * when calling CreateFile(). Note that CreateFile() doesn't understand
+ * this flag natively - we must handle it in our wrapper.
*/
-#define O_CLOEXEC 0
-
+#define O_CLOEXEC _O_NOINHERIT
/*
* Supplement to <errno.h>.
*
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b7..71344b9917 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = !(fileFlags & O_CLOEXEC);
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 902a795410..3a8e25fafd 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -18,6 +18,7 @@ SUBDIRS = \
test_binaryheap \
test_bitmapset \
test_bloomfilter \
+ test_cloexec \
test_copy_callbacks \
test_custom_rmgrs \
test_ddl_deparse \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 14fc761c4c..e9309ecb3e 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -17,6 +17,7 @@ subdir('test_aio')
subdir('test_binaryheap')
subdir('test_bitmapset')
subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 0000000000..70d38575e2
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 0000000000..63c8658b04
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 0000000000..4c56eb3a4b
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 0000000000..96cbd9c80e
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
0001-Fix-O_CLOEXEC-v16-v17-v18.patch.txt (text/plain, 14.7K) download
From 87efaf828d14ed70819ba660c2d4586061e22a2d Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Wed, 12 Nov 2025 13:10:35 -0600
Subject: [PATCH] Fix O_CLOEXEC flag handling in Windows port
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f, was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it was a mis-
understanding of the code path. In practice, the code was creating
inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, honor O_CLOEXEC in pgwin32_open_handle() by setting
bInheritHandle based on whether O_CLOEXEC is specified. We set
bInheritHandle directly in SECURITY_ATTRIBUTES rather than using
SetHandleInformation() afterwards, as the former is simpler.
For this backpatch, we use 0x04000000 as the O_CLOEXEC value to avoid
changing existing flag definitions. Master will use _O_NOINHERIT
and reorganize the flags cleanly.
---
src/include/port/win32_port.h | 17 +-
src/port/open.c | 13 +-
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 267 ++++++++++++++++++
7 files changed, 402 insertions(+), 12 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index fe9253d9b1f..b919f64e9a8 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -351,18 +351,15 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
*/
-#define O_DSYNC 0x0080
-
-/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+#define O_DIRECT 0x80000000
+/* Our src/port/open.c explicitly checks for O_CLOEXEC and sets
+ * bInheritHandle accordingly when calling CreateFile().
+ * Note that CreateFile() doesn't understand this flag natively - we must
+ * handle it in our wrapper.
*/
-#define O_CLOEXEC 0
+#define O_CLOEXEC 0x04000000
+#define O_DSYNC 0x0080
/*
* Supplement to <errno.h>.
diff --git a/src/port/open.c b/src/port/open.c
index 0bd97550063..81b356ff7ae 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler
+ * and achieves the same result.
+ */
+ sa.bInheritHandle = !(fileFlags & O_CLOEXEC);
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 9556bac4ad4..55ec0bf903b 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -15,3 +15,4 @@ subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 00000000000..70d38575e26
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 00000000000..63c8658b04e
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 00000000000..4c56eb3a4be
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 00000000000..96cbd9c80e6
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,267 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings
+ * and attempts to write to them
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /*
+ * Parent mode: opens files and spawns child
+ */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values
+ * as hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine()
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /*
+ * Try to write to both handles
+ */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly:
+ * - h1 should NOT be accessible (h1_worked == false)
+ * - h2 SHOULD be accessible (h2_worked == true)
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.46.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-11-30 00:13 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-11-30 00:13 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
On Thu, Nov 13, 2025 at 11:17 AM Bryan Green <dbryan.green@gmail.com> wrote:
> Corrected master patch and back patch for v16-v18.
Thanks.
I wondered what system-generated new handles might appear in a child
process and potentially collide with a non-inherited handle's
numerical value (perhaps a thread handle or something like that?), but
I guess it'd also have to accept a write to confuse the test, which
seems unlikely, so that's probably OK. I also hope that the new test
could eventually be merged with a general port layer test suite as
mentioned earlier.
What do you think about these improvements? See attached.
* moved and adjusted new comment about flag conversion to cover all
three flags, since it's true for all of them
* adjusted the comment about why we don't use SetHandleInformation()
to highlight that it would be (slightly) leaky
* removed O_DIRECT's extra definition from port.h, since it's now in
win32_port.h
One question is why on earth the open() redirection is in port.h while
the "supplements to fcntl.h" are in win32_port.h. Obviously those are
tightly coupled. As far as I know there are two forces keeping some
Windows porting magic in port.h that we'd ideally isolate in
win32_port.h, and this case doesn't appear to qualify for either as
far as I can guess, anyway:
* some sleep/retry wrappers were thought to be needed by Cygwin too:
API-wise it's a POSIX, but it couldn't hide the underlying NT file
locking semantics
* sometimes we need a later definition time: I recall battling that
for #define ftruncate and/or lseek (if you define them before certain
system headers are included, you break them)
Cygwin's <fcntl.h> defines these flags, if I've found the right
file[1], and has its own open() that we're using directly. If it
didn't, our code would have failed to compile when Cygwin was being
tested in the build farm up until a year or so ago (and it will be
tested again soon[2]). So we could probably move at least open() into
win32_port.h, in a separate commit. I think it's also quite likely
that Cygwin turns on the Windows 10 POSIX directory entry semantics,
so even rename() etc could probably be moved over too. (Whether our
own porting layer should turn that stuff on too and delete the retry
stuff entirely is an open question which no Windows expert has ever
opined on, only us Unix hackers battling against random failures in
the build farm.) We should probably also set up a CI task for Cygwin
if we're keeping support.
[1] https://github.com/cygwin/cygwin/blob/main/newlib/libc/include/sys/_default_fcntl.h
[2] https://www.postgresql.org/message-id/flat/916d0fd1-a99b-41c4-a017-ff2428bf8cca%40dunslane.net
Attachments:
v5-0001-Fix-O_CLOEXEC-flag-handling-in-Windows-port.patch (text/x-patch, 15.9K) download
From a316123d7136223e24c8ab39c92038aafc09ea49 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 30 Nov 2025 11:23:39 +1300
Subject: [PATCH v5] Fix O_CLOEXEC flag handling in Windows port.
PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE
when opening files on Windows, making all file descriptors inheritable
by child processes. This meant the O_CLOEXEC flag, added to many call
sites by commit 1da569ca1f (v16), was silently ignored.
The original commit included a comment suggesting that our open()
replacement doesn't create inheritable handles, but it was a mis-
understanding of the code path. In practice, the code was creating
inheritable handles in all cases.
This hasn't caused widespread problems because most child processes
(archive_command, COPY PROGRAM, etc.) operate on file paths passed as
arguments rather than inherited file descriptors. Even if a child
wanted to use an inherited handle, it would need to learn the numeric
handle value, which isn't passed through our IPC mechanisms.
Nonetheless, the current behavior is wrong. It violates documented
O_CLOEXEC semantics, contradicts our own code comments, and makes
PostgreSQL behave differently on Windows than on Unix. It also creates
potential issues with future code or security auditing tools.
To fix, define O_CLOEXEC to _O_NOINHERIT in master, previously used by
O_DSYNC. We use different values in the back branches to preserve
existing values. In pgwin32_open_handle() we set bInheritHandle
according to whether O_CLOEXEC is specified, for the same atomic
semantics as POSIX in multi-threaded programs that create processes.
Backpatch-through: 16
Author: Bryan Green <dbryan.green@gmail.com>
Discussion: https://postgr.es/m/e2b16375-7430-4053-bda3-5d2194ff1880%40gmail.com
---
src/include/port.h | 1 -
src/include/port/win32_port.h | 19 +-
src/port/open.c | 14 +-
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_cloexec/Makefile | 30 ++
src/test/modules/test_cloexec/meson.build | 26 ++
.../modules/test_cloexec/t/001_cloexec.pl | 60 ++++
src/test/modules/test_cloexec/test_cloexec.c | 262 ++++++++++++++++++
9 files changed, 400 insertions(+), 14 deletions(-)
create mode 100644 src/test/modules/test_cloexec/Makefile
create mode 100644 src/test/modules/test_cloexec/meson.build
create mode 100644 src/test/modules/test_cloexec/t/001_cloexec.pl
create mode 100644 src/test/modules/test_cloexec/test_cloexec.c
diff --git a/src/include/port.h b/src/include/port.h
index 159c2bcd7e3..672b880d40b 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -359,7 +359,6 @@ extern bool rmtree(const char *path, bool rmtopdir);
* open() and fopen() replacements to allow deletion of open files and
* passing of other special options.
*/
-#define O_DIRECT 0x80000000
extern HANDLE pgwin32_open_handle(const char *, int, bool);
extern int pgwin32_open(const char *, int,...);
extern FILE *pgwin32_fopen(const char *, const char *);
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index f54ccef7db8..8194714976f 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -335,18 +335,15 @@ extern int _pglstat64(const char *name, struct stat *buf);
/*
* Supplement to <fcntl.h>.
- * This is the same value as _O_NOINHERIT in the MS header file. This is
- * to ensure that we don't collide with a future definition. It means
- * we cannot use _O_NOINHERIT ourselves.
- */
-#define O_DSYNC 0x0080
-
-/*
- * Our open() replacement does not create inheritable handles, so it is safe to
- * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
- * necessary to convert this to _O_NOINHERIT.)
+ *
+ * We borrow bits from the high end when we have to, to avoid colliding with
+ * the system-defined values. Our open() replacement in src/port/open.c
+ * converts these to the equivalent CreateFile() flags, along with the ones
+ * from fcntl.h.
*/
-#define O_CLOEXEC 0
+#define O_CLOEXEC _O_NOINHERIT
+#define O_DIRECT 0x80000000
+#define O_DSYNC 0x04000000
/*
* Supplement to <errno.h>.
diff --git a/src/port/open.c b/src/port/open.c
index 4a31c5d7b77..ec0fbf29e92 100644
--- a/src/port/open.c
+++ b/src/port/open.c
@@ -74,13 +74,23 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics)
/* Check that we can handle the request */
assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND |
(O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) |
- _O_SHORT_LIVED | O_DSYNC | O_DIRECT |
+ _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC |
(O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags);
sa.nLength = sizeof(sa);
- sa.bInheritHandle = TRUE;
sa.lpSecurityDescriptor = NULL;
+ /*
+ * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise,
+ * create an inheritable handle (the default Windows behavior).
+ *
+ * Note: We could instead use SetHandleInformation() after CreateFile() to
+ * clear HANDLE_FLAG_INHERIT, but this way avoids rare leaks in
+ * multi-threaded programs that create processes, just like POSIX
+ * O_CLOEXEC.
+ */
+ sa.bInheritHandle = !(fileFlags & O_CLOEXEC);
+
while ((h = CreateFile(fileName,
/* cannot use O_RDONLY, as it == 0 */
(fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index d079b91b1a2..220e46c3ec3 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -19,6 +19,7 @@ SUBDIRS = \
test_binaryheap \
test_bitmapset \
test_bloomfilter \
+ test_cloexec \
test_copy_callbacks \
test_custom_rmgrs \
test_ddl_deparse \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index f5114469b92..f8b4b8343ea 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -18,6 +18,7 @@ subdir('test_aio')
subdir('test_binaryheap')
subdir('test_bitmapset')
subdir('test_bloomfilter')
+subdir('test_cloexec')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
subdir('test_ddl_deparse')
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
new file mode 100644
index 00000000000..70d38575e26
--- /dev/null
+++ b/src/test/modules/test_cloexec/Makefile
@@ -0,0 +1,30 @@
+# src/test/modules/test_cloexec/Makefile
+
+# This module is for Windows only
+ifeq ($(PORTNAME),win32)
+
+MODULE_big = test_cloexec
+OBJS = \
+ $(WIN32RES) \
+ test_cloexec.o
+
+PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+
+# Build as a standalone program, not a shared library
+PROGRAM = test_cloexec
+override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+
+TAP_TESTS = 1
+
+endif
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_cloexec
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build
new file mode 100644
index 00000000000..63c8658b04e
--- /dev/null
+++ b/src/test/modules/test_cloexec/meson.build
@@ -0,0 +1,26 @@
+# src/test/modules/test_cloexec/meson.build
+
+# This test is Windows-only
+if host_system != 'windows'
+ subdir_done()
+endif
+
+test_cloexec_sources = files('test_cloexec.c')
+
+test_cloexec = executable('test_cloexec',
+ test_cloexec_sources,
+ dependencies: [frontend_code],
+ link_with: [pgport[''], pgcommon['']],
+ kwargs: default_bin_args,
+)
+
+tests += {
+ 'name': 'test_cloexec',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_cloexec.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl
new file mode 100644
index 00000000000..4c56eb3a4be
--- /dev/null
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -0,0 +1,60 @@
+# Test O_CLOEXEC flag handling on Windows
+#
+# This test verifies that file handles opened with O_CLOEXEC are not
+# inherited by child processes, while handles opened without O_CLOEXEC
+# are inherited.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use Test::More;
+use IPC::Run qw(run);
+use File::Spec;
+use Cwd 'abs_path';
+
+if (!$PostgreSQL::Test::Utils::windows_os)
+{
+ plan skip_all => 'test is Windows-specific';
+}
+
+plan tests => 1;
+
+my $test_prog;
+foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH}))
+{
+ my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe');
+ if (-f $candidate && -x $candidate)
+ {
+ $test_prog = $candidate;
+ last;
+ }
+}
+
+if (!$test_prog)
+{
+ $test_prog = './test_cloexec.exe';
+}
+
+if (!-f $test_prog)
+{
+ BAIL_OUT("test program not found: $test_prog");
+}
+
+note("Using test program: $test_prog");
+
+my ($stdout, $stderr);
+my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr;
+
+note("Test program output:");
+note($stdout) if $stdout;
+
+if ($stderr)
+{
+ diag("Test program stderr:");
+ diag($stderr);
+}
+
+ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
+ "O_CLOEXEC prevents handle inheritance");
+
+done_testing();
\ No newline at end of file
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
new file mode 100644
index 00000000000..9f645451684
--- /dev/null
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -0,0 +1,262 @@
+/*-------------------------------------------------------------------------
+ *
+ * test_cloexec.c
+ * Test O_CLOEXEC flag handling on Windows
+ *
+ * This program tests that:
+ * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes
+ * 2. File handles opened without O_CLOEXEC ARE inherited by child processes
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifdef WIN32
+#include <windows.h>
+#endif
+
+#include "common/file_utils.h"
+#include "port.h"
+
+static void run_parent_tests(const char *testfile1, const char *testfile2);
+static void run_child_tests(const char *handle1_str, const char *handle2_str);
+static bool try_write_to_handle(HANDLE h, const char *label);
+
+int
+main(int argc, char *argv[])
+{
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
+
+ /* Windows-only test */
+#ifndef WIN32
+ fprintf(stderr, "This test only runs on Windows\n");
+ return 0;
+#endif
+
+ if (argc == 3)
+ {
+ /*
+ * Child mode: receives two handle values as hex strings and attempts
+ * to write to them.
+ */
+ run_child_tests(argv[1], argv[2]);
+ return 0;
+ }
+ else if (argc == 1)
+ {
+ /* Parent mode: opens files and spawns child */
+ snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid());
+ snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid());
+
+ run_parent_tests(testfile1, testfile2);
+
+ /* Clean up test files */
+ unlink(testfile1);
+ unlink(testfile2);
+
+ return 0;
+ }
+ else
+ {
+ fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
+ return 1;
+ }
+}
+
+static void
+run_parent_tests(const char *testfile1, const char *testfile2)
+{
+#ifdef WIN32
+ int fd1,
+ fd2;
+ HANDLE h1,
+ h2;
+ char cmdline[1024];
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ DWORD exit_code;
+
+ printf("Parent: Opening test files...\n");
+
+ /*
+ * Open first file WITH O_CLOEXEC - should NOT be inherited
+ */
+ fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+ if (fd1 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno));
+ exit(1);
+ }
+
+ /*
+ * Open second file WITHOUT O_CLOEXEC - should be inherited
+ */
+ fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
+ if (fd2 < 0)
+ {
+ fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno));
+ close(fd1);
+ exit(1);
+ }
+
+ /* Get Windows HANDLEs from file descriptors */
+ h1 = (HANDLE) _get_osfhandle(fd1);
+ h2 = (HANDLE) _get_osfhandle(fd2);
+
+ if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE)
+ {
+ fprintf(stderr, "Failed to get OS handles\n");
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
+ printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
+
+ /*
+ * Spawn child process with bInheritHandles=TRUE, passing handle values as
+ * hex strings
+ */
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ GetCommandLine(), h1, h2);
+
+ /*
+ * Find the actual executable path by removing any arguments from
+ * GetCommandLine().
+ */
+ {
+ char exe_path[MAX_PATH];
+ char *space_pos;
+
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
+ exe_path, h1, h2);
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ memset(&pi, 0, sizeof(pi));
+
+ printf("Parent: Spawning child process...\n");
+ printf("Parent: Command line: %s\n", cmdline);
+
+ if (!CreateProcess(NULL, /* application name */
+ cmdline, /* command line */
+ NULL, /* process security attributes */
+ NULL, /* thread security attributes */
+ TRUE, /* bInheritHandles - CRITICAL! */
+ 0, /* creation flags */
+ NULL, /* environment */
+ NULL, /* current directory */
+ &si, /* startup info */
+ &pi)) /* process information */
+ {
+ fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError());
+ close(fd1);
+ close(fd2);
+ exit(1);
+ }
+
+ printf("Parent: Waiting for child process...\n");
+
+ /* Wait for child to complete */
+ WaitForSingleObject(pi.hProcess, INFINITE);
+ GetExitCodeProcess(pi.hProcess, &exit_code);
+
+ CloseHandle(pi.hProcess);
+ CloseHandle(pi.hThread);
+
+ close(fd1);
+ close(fd2);
+
+ printf("Parent: Child exit code: %lu\n", exit_code);
+
+ if (exit_code == 0)
+ printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ else
+ {
+ printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static void
+run_child_tests(const char *handle1_str, const char *handle2_str)
+{
+#ifdef WIN32
+ HANDLE h1,
+ h2;
+ bool h1_worked,
+ h2_worked;
+
+ /* Parse handle values from hex strings */
+ if (sscanf(handle1_str, "%p", &h1) != 1 ||
+ sscanf(handle2_str, "%p", &h2) != 1)
+ {
+ fprintf(stderr, "Child: Failed to parse handle values\n");
+ exit(1);
+ }
+
+ printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
+ printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
+
+ /* Try to write to both handles */
+ h1_worked = try_write_to_handle(h1, "HANDLE1");
+ h2_worked = try_write_to_handle(h2, "HANDLE2");
+
+ printf("Child: HANDLE1 (O_CLOEXEC): %s\n",
+ h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)");
+ printf("Child: HANDLE2 (no O_CLOEXEC): %s\n",
+ h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)");
+
+ /*
+ * For O_CLOEXEC to work correctly, h1 should NOT be accessible (h1_worked
+ * == false) and h2 SHOULD be accessible (h2_worked == true).
+ */
+ if (!h1_worked && h2_worked)
+ {
+ printf("Child: Test PASSED - O_CLOEXEC working correctly\n");
+ exit(0);
+ }
+ else
+ {
+ printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
+ exit(1);
+ }
+#endif
+}
+
+static bool
+try_write_to_handle(HANDLE h, const char *label)
+{
+#ifdef WIN32
+ const char *test_data = "test\n";
+ DWORD bytes_written;
+ BOOL result;
+
+ result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL);
+
+ if (result && bytes_written == strlen(test_data))
+ {
+ printf("Child: Successfully wrote to %s\n", label);
+ return true;
+ }
+ else
+ {
+ printf("Child: Failed to write to %s (error %lu)\n",
+ label, GetLastError());
+ return false;
+ }
+#else
+ return false;
+#endif
+}
--
2.51.2
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-09 21:03 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-09 22:04 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 25+ messages in thread
From: Thomas Munro @ 2025-12-09 21:03 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
On Sun, Nov 30, 2025 at 1:13 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> What do you think about these improvements? See attached.
>
> * moved and adjusted new comment about flag conversion to cover all
> three flags, since it's true for all of them
> * adjusted the comment about why we don't use SetHandleInformation()
> to highlight that it would be (slightly) leaky
> * removed O_DIRECT's extra definition from port.h, since it's now in
> win32_port.h
Hearing nothing, pushed.
I realised while back-patching that REL_16_STABLE was the last release
that also had the old Windows-only build/test machinery under
src/tools/msvc. It never ran all the tests anyway, and I don't think
we'd learn anything new by adding it, given that CI uses meson on that
branch, so I didn't worry about remembering how to adjust that for
now. If someone feels strongly about it, of course we can.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-09 22:04 ` Bryan Green <dbryan.green@gmail.com>
1 sibling, 0 replies; 25+ messages in thread
From: Bryan Green @ 2025-12-09 22:04 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: pgsql-hackers
On 12/9/2025 3:03 PM, Thomas Munro wrote:
> On Sun, Nov 30, 2025 at 1:13 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> What do you think about these improvements? See attached.
>>
>> * moved and adjusted new comment about flag conversion to cover all
>> three flags, since it's true for all of them
>> * adjusted the comment about why we don't use SetHandleInformation()
>> to highlight that it would be (slightly) leaky
>> * removed O_DIRECT's extra definition from port.h, since it's now in
>> win32_port.h
>
> Hearing nothing, pushed.
>
> I realised while back-patching that REL_16_STABLE was the last release
> that also had the old Windows-only build/test machinery under
> src/tools/msvc. It never ran all the tests anyway, and I don't think
> we'd learn anything new by adding it, given that CI uses meson on that
> branch, so I didn't worry about remembering how to adjust that for
> now. If someone feels strongly about it, of course we can.
Well, my drafts folder had nothing but agreement in it...
Thanks,
--
Bryan Green
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-13 02:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2026-06-20 07:00 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Alexander Lakhin <exclusion@gmail.com>
1 sibling, 2 replies; 25+ messages in thread
From: Tom Lane @ 2025-12-13 02:44 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
Thomas Munro <thomas.munro@gmail.com> writes:
> Hearing nothing, pushed.
fairywren is unimpressed:
../pgsql/src/test/modules/test_cloexec/test_cloexec.c: In function 'run_parent_tests':
../pgsql/src/test/modules/test_cloexec/test_cloexec.c:137:29: warning: unused variable 'space_pos' [-Wunused-variable]
137 | char *space_pos;
| ^~~~~~~~~
It's right, but it seems to me that this stanza needs more help than
that:
/*
* Spawn child process with bInheritHandles=TRUE, passing handle values as
* hex strings
*/
snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
GetCommandLine(), h1, h2);
/*
* Find the actual executable path by removing any arguments from
* GetCommandLine().
*/
{
char exe_path[MAX_PATH];
char *space_pos;
GetModuleFileName(NULL, exe_path, sizeof(exe_path));
snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
exe_path, h1, h2);
}
What is the point of that first snprintf(cmdline, ...), when its
result is guaranteed to be overwritten just below?
I'm also dubious about using MAX_PATH here; see the commentary
about MAXPGPATH in pg_config_manual.h. Also, what's the point of
using MAX_PATH when the result is going to be transferred into
cmdline (with a hardwired size of 1024)?
regards, tom lane
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-12-13 03:57 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
1 sibling, 2 replies; 25+ messages in thread
From: Thomas Munro @ 2025-12-13 03:57 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Sat, Dec 13, 2025 at 3:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > Hearing nothing, pushed.
>
> fairywren is unimpressed:
>
> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c: In function 'run_parent_tests':
> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c:137:29: warning: unused variable 'space_pos' [-Wunused-variable]
> 137 | char *space_pos;
> | ^~~~~~~~~
The CI MinGW task also shows this warning, but it doesn't use -Werror.
The separate CompileWarnings task does, being its purpose, and it
includes a MinGW cross-build step, but that uses configure, and this
test is built only by meson. That wasn't a great idea... we knew we
were only dealing with Windows but forgot about MinGW, so I'll go and
write a patch to fix that aspect later today so we're covered for
warnings. I'll also think about whether it's worth checking for MinGW
warnings in both assert and non-assert builds (as we do for regular
Linux gcc/clang), and I'd also like to try to catch warnings from MSVC
and had an idea for how to do that... I might also try to think about
meson-vs-configure cross checks...
> What is the point of that first snprintf(cmdline, ...), when its
> result is guaranteed to be overwritten just below?
>
> I'm also dubious about using MAX_PATH here; see the commentary
> about MAXPGPATH in pg_config_manual.h. Also, what's the point of
> using MAX_PATH when the result is going to be transferred into
> cmdline (with a hardwired size of 1024)?
Fair points, I'll wait and see if Bryan is free to write a patch on
Monday (US), and otherwise write one myself.
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-13 04:10 ` Bryan Green <dbryan.green@gmail.com>
1 sibling, 0 replies; 25+ messages in thread
From: Bryan Green @ 2025-12-13 04:10 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On 12/12/2025 9:57 PM, Thomas Munro wrote:
> On Sat, Dec 13, 2025 at 3:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> Hearing nothing, pushed.
>>
>> fairywren is unimpressed:
>>
>> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c: In function 'run_parent_tests':
>> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c:137:29: warning: unused variable 'space_pos' [-Wunused-variable]
>> 137 | char *space_pos;
>> | ^~~~~~~~~
>
> The CI MinGW task also shows this warning, but it doesn't use -Werror.
> The separate CompileWarnings task does, being its purpose, and it
> includes a MinGW cross-build step, but that uses configure, and this
> test is built only by meson. That wasn't a great idea... we knew we
> were only dealing with Windows but forgot about MinGW, so I'll go and
> write a patch to fix that aspect later today so we're covered for
> warnings. I'll also think about whether it's worth checking for MinGW
> warnings in both assert and non-assert builds (as we do for regular
> Linux gcc/clang), and I'd also like to try to catch warnings from MSVC
> and had an idea for how to do that... I might also try to think about
> meson-vs-configure cross checks...
>
>> What is the point of that first snprintf(cmdline, ...), when its
>> result is guaranteed to be overwritten just below?
>>
>> I'm also dubious about using MAX_PATH here; see the commentary
>> about MAXPGPATH in pg_config_manual.h. Also, what's the point of
>> using MAX_PATH when the result is going to be transferred into
>> cmdline (with a hardwired size of 1024)?
>
> Fair points, I'll wait and see if Bryan is free to write a patch on
> Monday (US), and otherwise write one myself.
I will write a patch tonight. This was my sloppiness from doing
incremental changes and not cleaning up behind myself. I'll clean it
up. Thanks for the checks...
--
Bryan Green
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-13 04:46 ` Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 25+ messages in thread
From: Bryan Green @ 2025-12-13 04:46 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On 12/12/2025 9:57 PM, Thomas Munro wrote:
> On Sat, Dec 13, 2025 at 3:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Thomas Munro <thomas.munro@gmail.com> writes:
>>> Hearing nothing, pushed.
>>
>> fairywren is unimpressed:
>>
>> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c: In function 'run_parent_tests':
>> ../pgsql/src/test/modules/test_cloexec/test_cloexec.c:137:29: warning: unused variable 'space_pos' [-Wunused-variable]
>> 137 | char *space_pos;
>> | ^~~~~~~~~
>
> The CI MinGW task also shows this warning, but it doesn't use -Werror.
> The separate CompileWarnings task does, being its purpose, and it
> includes a MinGW cross-build step, but that uses configure, and this
> test is built only by meson. That wasn't a great idea... we knew we
> were only dealing with Windows but forgot about MinGW, so I'll go and
> write a patch to fix that aspect later today so we're covered for
> warnings. I'll also think about whether it's worth checking for MinGW
> warnings in both assert and non-assert builds (as we do for regular
> Linux gcc/clang), and I'd also like to try to catch warnings from MSVC
> and had an idea for how to do that... I might also try to think about
> meson-vs-configure cross checks...
>
>> What is the point of that first snprintf(cmdline, ...), when its
>> result is guaranteed to be overwritten just below?
>>
>> I'm also dubious about using MAX_PATH here; see the commentary
>> about MAXPGPATH in pg_config_manual.h. Also, what's the point of
>> using MAX_PATH when the result is going to be transferred into
>> cmdline (with a hardwired size of 1024)?
>
> Fair points, I'll wait and see if Bryan is free to write a patch on
> Monday (US), and otherwise write one myself.
Thomas,
A sanity check would be appreciated after the somewhat embarrassing
sloppy code.
I removed the useless snprintf() call that was using GetCommandLine().
That was left in place when I moved to GetModuleFileName(). Also,
removed the unused 'space_pos' variable and the unneeded scope block. I
decided to just use 1024 for the exe_path size since that is what
cmdline is set to use. I also removed some self-evident comments that
were leftover from my practice of writing comments and then coding.
Apologies for the loss of time.
Thanks,
--
Bryan Green
EDB: https://www.enterprisedb.com
Attachments:
v1-0001-Clean-up-sloppy-code-in-test_cloexec.patch (text/plain, 4.3K) download
From 722d675cae77d046b8e53abbcf9f5218b375b180 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 12 Dec 2025 22:31:15 -0600
Subject: [PATCH v1] Clean up sloppy code in test_cloexec.c
The command line construction code in run_parent_tests() had several
issues:
1. A useless snprintf() call that built a command line using
GetCommandLine(), which was immediately overwritten by a second
snprintf() that correctly used GetModuleFileName() instead.
2. An unused variable 'space_pos' that was declared but never used.
3. An unnecessary scope block around the GetModuleFileName() call.
4. Inconsistent buffer sizing: exe_path used MAX_PATH while cmdline used
1024..
This appears to be code that was refactored when it was realized
GetCommandLine() wouldn't work correctly (it returns the full command
line with arguments, not just the executable path), but the old
approach was never fully removed.
Clean this up by:
- Removing the redundant first snprintf() call
- Removing the unused space_pos variable
- Removing the unnecessary scope block
- Using consistent 1024-byte buffer size for both exe_path and cmdline
- Adding a clearer comment explaining the purpose of the code
- Removed some simplistic, unneeded comments
---
src/test/modules/test_cloexec/test_cloexec.c | 34 +++-----------------
1 file changed, 5 insertions(+), 29 deletions(-)
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
index 9f64545168..a18d53a5d3 100644
--- a/src/test/modules/test_cloexec/test_cloexec.c
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -34,7 +34,6 @@ main(int argc, char *argv[])
char testfile1[MAXPGPATH];
char testfile2[MAXPGPATH];
- /* Windows-only test */
#ifndef WIN32
fprintf(stderr, "This test only runs on Windows\n");
return 0;
@@ -57,7 +56,6 @@ main(int argc, char *argv[])
run_parent_tests(testfile1, testfile2);
- /* Clean up test files */
unlink(testfile1);
unlink(testfile2);
@@ -78,6 +76,7 @@ run_parent_tests(const char *testfile1, const char *testfile2)
fd2;
HANDLE h1,
h2;
+ char exe_path[1024];
char cmdline[1024];
STARTUPINFO si;
PROCESS_INFORMATION pi;
@@ -106,7 +105,6 @@ run_parent_tests(const char *testfile1, const char *testfile2)
exit(1);
}
- /* Get Windows HANDLEs from file descriptors */
h1 = (HANDLE) _get_osfhandle(fd1);
h2 = (HANDLE) _get_osfhandle(fd2);
@@ -121,25 +119,8 @@ run_parent_tests(const char *testfile1, const char *testfile2)
printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
- /*
- * Spawn child process with bInheritHandles=TRUE, passing handle values as
- * hex strings
- */
- snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
- GetCommandLine(), h1, h2);
-
- /*
- * Find the actual executable path by removing any arguments from
- * GetCommandLine().
- */
- {
- char exe_path[MAX_PATH];
- char *space_pos;
-
- GetModuleFileName(NULL, exe_path, sizeof(exe_path));
- snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
- exe_path, h1, h2);
- }
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p", exe_path, h1, h2);
memset(&si, 0, sizeof(si));
si.cb = sizeof(si);
@@ -167,7 +148,6 @@ run_parent_tests(const char *testfile1, const char *testfile2)
printf("Parent: Waiting for child process...\n");
- /* Wait for child to complete */
WaitForSingleObject(pi.hProcess, INFINITE);
GetExitCodeProcess(pi.hProcess, &exit_code);
@@ -193,12 +173,9 @@ static void
run_child_tests(const char *handle1_str, const char *handle2_str)
{
#ifdef WIN32
- HANDLE h1,
- h2;
- bool h1_worked,
- h2_worked;
+ HANDLE h1, h2;
+ bool h1_worked, h2_worked;
- /* Parse handle values from hex strings */
if (sscanf(handle1_str, "%p", &h1) != 1 ||
sscanf(handle2_str, "%p", &h2) != 1)
{
@@ -209,7 +186,6 @@ run_child_tests(const char *handle1_str, const char *handle2_str)
printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1);
printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2);
- /* Try to write to both handles */
h1_worked = try_write_to_handle(h1, "HANDLE1");
h2_worked = try_write_to_handle(h2, "HANDLE2");
--
2.52.0.windows.1
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
@ 2025-12-13 06:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2025-12-13 06:33 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: Thomas Munro <thomas.munro@gmail.com>; pgsql-hackers
Bryan Green <dbryan.green@gmail.com> writes:
> I removed the useless snprintf() call that was using GetCommandLine().
> That was left in place when I moved to GetModuleFileName(). Also,
> removed the unused 'space_pos' variable and the unneeded scope block.
All good to my eye.
> I decided to just use 1024 for the exe_path size since that is what
> cmdline is set to use.
Personally I'd have gone the other way, say
char exe_path[MAXPGPATH];
char cmdline[MAXPGPATH + 100];
> I also removed some self-evident comments that
> were leftover from my practice of writing comments and then coding.
I think you went way overboard on removing "self-evident" comments.
Signposts as to what the code intends to do are pretty helpful IMO.
They do have to be accurate though, for instance this previous
comment:
- * Find the actual executable path by removing any arguments from
- * GetCommandLine().
didn't seem to convey what the code was doing (which I neglected
to complain about before).
BTW, pgindent will undo some of the whitespace changes you made
here.
regards, tom lane
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-12-14 05:09 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-12-14 05:09 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Sat, Dec 13, 2025 at 7:33 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Bryan Green <dbryan.green@gmail.com> writes:
> > I removed the useless snprintf() call that was using GetCommandLine().
> > That was left in place when I moved to GetModuleFileName(). Also,
> > removed the unused 'space_pos' variable and the unneeded scope block.
>
> All good to my eye.
Thanks both.
> > I decided to just use 1024 for the exe_path size since that is what
> > cmdline is set to use.
>
> Personally I'd have gone the other way, say
>
> char exe_path[MAXPGPATH];
> char cmdline[MAXPGPATH + 100];
Done in this version.
> > I also removed some self-evident comments that
> > were leftover from my practice of writing comments and then coding.
>
> I think you went way overboard on removing "self-evident" comments.
> Signposts as to what the code intends to do are pretty helpful IMO.
> They do have to be accurate though, for instance this previous
> comment:
>
> - * Find the actual executable path by removing any arguments from
> - * GetCommandLine().
>
> didn't seem to convey what the code was doing (which I neglected
> to complain about before).
Comments restored in the attached version of Bryan's patch.
My earlier guess about the Makefile was wrong, and when I looked into
it the actual problems were (1) that the CompilerWarnings task in CI
runs make world-bin, which doesn't descend into src/test, and (2) that
the test ifeq ($(PORTNAME), win32) was not satisfied due to make's
rules for variable evaluation. I thought about how to fix that but
realised that this is going to be much easier to maintain if it's not
different on Unix, so here are some fixes in that direction. With
just 0001 and 0002 applied, we'd have known about the compiler warning
before commit, with a failure like this:
https://cirrus-ci.com/task/5863371716689920
With 0003 applied on top, it's green and there are no warnings from
either Windows task:
https://cirrus-ci.com/build/4775547869331456
I also changed the comment style of some single-line comments.
replaced the memset() with initializer syntax and ran pgindent which
undid a change or two.
Attachments: download all (3 files, .tar.gz)
0001-ci-Check-src-test-in-CompilerWarnings-task.patch (text/x-patch, 1.9K) download
From b0767473504c0c313150ad47b531967879314fbf Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 14 Dec 2025 12:30:24 +1300
Subject: [PATCH 1/3] ci: Check src/test in CompilerWarnings task.
Since make world-bin doesn't visit src/test, CI was not checking for
compiler warnings in test modules.
Backpatch-through: 15, where CI began
Discussion: https://postgr.es/m/1086088.1765593851%40sss.pgh.pa.us
---
.cirrus.tasks.yml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 038d043d00e..29c7a6c0a07 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -944,6 +944,7 @@ task:
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/test -s -j${BUILD_JOBS}
# gcc, cassert on, dtrace off
always:
@@ -955,6 +956,7 @@ task:
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/test -s -j${BUILD_JOBS}
# clang, cassert off, dtrace off
always:
@@ -965,6 +967,7 @@ task:
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/test -s -j${BUILD_JOBS}
# clang, cassert on, dtrace on
always:
@@ -977,6 +980,7 @@ task:
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/test -s -j${BUILD_JOBS}
# cross-compile to windows
always:
@@ -989,6 +993,7 @@ task:
CXX="ccache x86_64-w64-mingw32ucrt-g++"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/test -s -j${BUILD_JOBS}
###
# Verify docs can be built
--
2.51.2
0002-Fix-Makefile-used-for-test_cloexec.patch (text/x-patch, 3.8K) download
From 897d47122ef39f826eae3f3b7db8cb85e45d3c84 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 14 Dec 2025 17:14:38 +1300
Subject: [PATCH 2/3] Fix Makefile used for test_cloexec.
The previous test for ifeq ($(PORTNAME), win32) never succeeded, so only
meson-based builds were building and testing our O_CLOEXEC emulation.
Defect in commit c507ba55.
That could probably be made to work, but it seems better to compile
test_cloexec.c unconditionally and skip the test for non-Windows
systems. Though the test is skipped on Unix and the executable wouldn't
test anything if run (there is not much point in testing that the system
O_CLOEXEC works on Unix), at least this way the configure-based build
machinery is consistent, for ease of maintenance by Unix-based
developers.
Backpatch-through: 16, like commit c507ba55
---
src/test/modules/test_cloexec/Makefile | 16 ++++-----------
src/test/modules/test_cloexec/test_cloexec.c | 21 +++++++++-----------
2 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
index 70d38575e26..94342319782 100644
--- a/src/test/modules/test_cloexec/Makefile
+++ b/src/test/modules/test_cloexec/Makefile
@@ -1,22 +1,14 @@
# src/test/modules/test_cloexec/Makefile
-# This module is for Windows only
-ifeq ($(PORTNAME),win32)
-
-MODULE_big = test_cloexec
-OBJS = \
- $(WIN32RES) \
- test_cloexec.o
-
PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
+PGAPPICON = win32
-# Build as a standalone program, not a shared library
-PROGRAM = test_cloexec
-override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
+PROGRAM += test_cloexec
+OBJS += $(WIN32RES) test_cloexec.o
+NO_INSTALLCHECK = 1
TAP_TESTS = 1
-endif
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
index 9f645451684..7ff6585d710 100644
--- a/src/test/modules/test_cloexec/test_cloexec.c
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -24,21 +24,22 @@
#include "common/file_utils.h"
#include "port.h"
+#ifdef WIN32
static void run_parent_tests(const char *testfile1, const char *testfile2);
static void run_child_tests(const char *handle1_str, const char *handle2_str);
static bool try_write_to_handle(HANDLE h, const char *label);
+#endif
int
main(int argc, char *argv[])
{
- char testfile1[MAXPGPATH];
- char testfile2[MAXPGPATH];
-
- /* Windows-only test */
#ifndef WIN32
+ /* Windows-only test */
fprintf(stderr, "This test only runs on Windows\n");
return 0;
-#endif
+#else
+ char testfile1[MAXPGPATH];
+ char testfile2[MAXPGPATH];
if (argc == 3)
{
@@ -68,12 +69,13 @@ main(int argc, char *argv[])
fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]);
return 1;
}
+#endif
}
+#ifdef WIN32
static void
run_parent_tests(const char *testfile1, const char *testfile2)
{
-#ifdef WIN32
int fd1,
fd2;
HANDLE h1,
@@ -186,13 +188,11 @@ run_parent_tests(const char *testfile1, const char *testfile2)
printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
exit(1);
}
-#endif
}
static void
run_child_tests(const char *handle1_str, const char *handle2_str)
{
-#ifdef WIN32
HANDLE h1,
h2;
bool h1_worked,
@@ -232,13 +232,11 @@ run_child_tests(const char *handle1_str, const char *handle2_str)
printf("Child: Test FAILED - O_CLOEXEC not working correctly\n");
exit(1);
}
-#endif
}
static bool
try_write_to_handle(HANDLE h, const char *label)
{
-#ifdef WIN32
const char *test_data = "test\n";
DWORD bytes_written;
BOOL result;
@@ -256,7 +254,6 @@ try_write_to_handle(HANDLE h, const char *label)
label, GetLastError());
return false;
}
-#else
return false;
-#endif
}
+#endif
--
2.51.2
0003-Clean-up-sloppy-code-in-test_cloexec.c.patch (text/x-patch, 4.1K) download
From 47a7e7406c0896e985435360013d99f0553dd607 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Fri, 12 Dec 2025 22:31:15 -0600
Subject: [PATCH 3/3] Clean up sloppy code in test_cloexec.c.
The command line construction code in run_parent_tests() had several
issues:
1. A useless snprintf() call that buitlt a command line using
GetCommandLine(), which was immediately overwritten by a second
snprintf() that correctly used GetModuleFileName() instead.
2. An unused variable 'space_pos' that was declared but never used.
3. An unnecessary scope block around the GetModuleFileName() call.
4. Inconsistent buffer sizing: exe_path used MAX_PATH while cmdline used
1024.
This appears to be code that was refactored when it was realized
GetCommandLine() wouldn't work correctly (it returns the full command
line with arguments, not just the executable path), but the old approach
was never fully removed.
Clean this up by:
- Removing the redundant first snprintf() call
- Removing the unused space_pos variable
- Removing the unnecessary scope block
- Using consistent 1024-byte buffer size for both exe_path and cmdline
- Adding a clearer comment explaining the purpose of the code
- Removed some simplistic, unneeded comments
Per code review from Tom Lane.
Backpatch-through: 16, like commit c507ba55
Author: Bryan Green <dbryan.green@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Thomas Munro <tmunro@gmail.com>
Discussion: https://postgr.es/m/1086088.1765593851%40sss.pgh.pa.us
---
src/test/modules/test_cloexec/test_cloexec.c | 40 ++++++--------------
1 file changed, 11 insertions(+), 29 deletions(-)
diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c
index 7ff6585d710..621a3cc02f5 100644
--- a/src/test/modules/test_cloexec/test_cloexec.c
+++ b/src/test/modules/test_cloexec/test_cloexec.c
@@ -80,16 +80,15 @@ run_parent_tests(const char *testfile1, const char *testfile2)
fd2;
HANDLE h1,
h2;
- char cmdline[1024];
- STARTUPINFO si;
- PROCESS_INFORMATION pi;
+ char exe_path[MAXPGPATH];
+ char cmdline[MAXPGPATH + 100];
+ STARTUPINFO si = {.cb = sizeof(si)};
+ PROCESS_INFORMATION pi = {0};
DWORD exit_code;
printf("Parent: Opening test files...\n");
- /*
- * Open first file WITH O_CLOEXEC - should NOT be inherited
- */
+ /* Open first file WITH O_CLOEXEC - should NOT be inherited */
fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
if (fd1 < 0)
{
@@ -97,9 +96,7 @@ run_parent_tests(const char *testfile1, const char *testfile2)
exit(1);
}
- /*
- * Open second file WITHOUT O_CLOEXEC - should be inherited
- */
+ /* Open second file WITHOUT O_CLOEXEC - should be inherited */
fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600);
if (fd2 < 0)
{
@@ -123,29 +120,12 @@ run_parent_tests(const char *testfile1, const char *testfile2)
printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1);
printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2);
- /*
- * Spawn child process with bInheritHandles=TRUE, passing handle values as
- * hex strings
- */
- snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
- GetCommandLine(), h1, h2);
-
/*
* Find the actual executable path by removing any arguments from
- * GetCommandLine().
+ * GetCommandLine(), and add our new arguments.
*/
- {
- char exe_path[MAX_PATH];
- char *space_pos;
-
- GetModuleFileName(NULL, exe_path, sizeof(exe_path));
- snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p",
- exe_path, h1, h2);
- }
-
- memset(&si, 0, sizeof(si));
- si.cb = sizeof(si);
- memset(&pi, 0, sizeof(pi));
+ GetModuleFileName(NULL, exe_path, sizeof(exe_path));
+ snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p", exe_path, h1, h2);
printf("Parent: Spawning child process...\n");
printf("Parent: Command line: %s\n", cmdline);
@@ -182,7 +162,9 @@ run_parent_tests(const char *testfile1, const char *testfile2)
printf("Parent: Child exit code: %lu\n", exit_code);
if (exit_code == 0)
+ {
printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n");
+ }
else
{
printf("Parent: FAILURE - O_CLOEXEC not working correctly\n");
--
2.51.2
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-21 10:41 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-12-21 10:41 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Sun, Dec 14, 2025 at 6:09 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> My earlier guess about the Makefile was wrong, and when I looked into
> it the actual problems were (1) that the CompilerWarnings task in CI
> runs make world-bin, which doesn't descend into src/test, and (2) that
> the test ifeq ($(PORTNAME), win32) was not satisfied due to make's
> rules for variable evaluation. I thought about how to fix that but
> realised that this is going to be much easier to maintain if it's not
> different on Unix, so here are some fixes in that direction. With
> just 0001 and 0002 applied, we'd have known about the compiler warning
> before commit, with a failure like this:
I pushed the cleanup patch.
I wondered if there might be any other C code that could be checked
for compiler warnings by CI and isn't yet, and the only thing I have
some up with so far is the .pgc -> .c stuff. Here is a new version
that does that too. I also back-patched a fix for a warning (see
eab2323c) that would break if we back-patched this. Is there anything
else like this hiding somewhere?
Attachments:
0001-ci-Compile-test-C-in-CompilerWarnings-task.patch (application/octet-stream, 2.4K) download
From 1698861e93f2c403d14a0988b38731b3e3e87eb6 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sun, 14 Dec 2025 12:30:24 +1300
Subject: [PATCH] ci: Compile test C in CompilerWarnings task.
Since the CompilerWarnings task is still using configure/make and C code
under there isn't compiled until you actually try to run the tests,
compile src/test and the ECPG preprocessed C explicitly. The lack of
these has hidden warnings that could have been revealed by CI.
Backpatch-through: 15, where CI began
Discussion: https://postgr.es/m/1086088.1765593851%40sss.pgh.pa.us
---
.cirrus.tasks.yml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 038d043d00e..b57a8033b16 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -944,6 +944,8 @@ task:
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/interfaces/ecpg/test/sql -s -j${BUILD_JOBS}
+ time make -C src/test -s -j${BUILD_JOBS}
# gcc, cassert on, dtrace off
always:
@@ -955,6 +957,8 @@ task:
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/interfaces/ecpg/test/sql -s -j${BUILD_JOBS}
+ time make -C src/test -s -j${BUILD_JOBS}
# clang, cassert off, dtrace off
always:
@@ -965,6 +969,8 @@ task:
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/interfaces/ecpg/test/sql -s -j${BUILD_JOBS}
+ time make -C src/test -s -j${BUILD_JOBS}
# clang, cassert on, dtrace on
always:
@@ -977,6 +983,8 @@ task:
CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/interfaces/ecpg/test/sql -s -j${BUILD_JOBS}
+ time make -C src/test -s -j${BUILD_JOBS}
# cross-compile to windows
always:
@@ -989,6 +997,8 @@ task:
CXX="ccache x86_64-w64-mingw32ucrt-g++"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} world-bin
+ time make -C src/interfaces/ecpg/test/sql -s -j${BUILD_JOBS}
+ time make -C src/test -s -j${BUILD_JOBS}
###
# Verify docs can be built
--
2.50.1 (Apple Git-155)
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-21 22:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 00:24 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2025-12-21 22:48 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
Thomas Munro <thomas.munro@gmail.com> writes:
> I pushed the cleanup patch.
fairywren's still not happy, though only in the v16 branch:
Could not determine contrib module type for test_cloexec
at build.pl line 54.
This evidently is because the old MSVC build system doesn't
recognize
PROGRAM += test_cloexec
OBJS += $(WIN32RES) test_cloexec.o
Is there a reason for using += not just = here? We could certainly
modify Mkvcbuild.pm to parse this if we need to, but it looks more
like a gratuitous difference from everyplace else than a useful
behavior.
regards, tom lane
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-12-22 00:24 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:38 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-12-22 00:24 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Mon, Dec 22, 2025 at 11:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> fairywren's still not happy, though only in the v16 branch:
>
> Could not determine contrib module type for test_cloexec
> at build.pl line 54.
>
> This evidently is because the old MSVC build system doesn't
> recognize
>
> PROGRAM += test_cloexec
> OBJS += $(WIN32RES) test_cloexec.o
>
> Is there a reason for using += not just = here? We could certainly
> modify Mkvcbuild.pm to parse this if we need to, but it looks more
> like a gratuitous difference from everyplace else than a useful
> behavior.
Yeah. Will drop the + signs. I've also written a patch to enable a
separate "Windows - Server 2022, VS 2019 - Mkvcbuild.pm" CI task
alongside the Meson one in the REL_16_STABLE branch. I had run the
affected branches through CI, but of course 16 switched to Meson so it
wasn't testing the third build system... without that, this is just
too painful. I need to step away for a couple of hours, but more in a
bit...
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 00:24 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-22 21:38 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:50 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Munro @ 2025-12-22 21:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Mon, Dec 22, 2025 at 1:24 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Mon, Dec 22, 2025 at 11:48 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > fairywren's still not happy, though only in the v16 branch:
> >
> > Could not determine contrib module type for test_cloexec
> > at build.pl line 54.
> >
> > This evidently is because the old MSVC build system doesn't
> > recognize
> >
> > PROGRAM += test_cloexec
> > OBJS += $(WIN32RES) test_cloexec.o
> >
> > Is there a reason for using += not just = here? We could certainly
> > modify Mkvcbuild.pm to parse this if we need to, but it looks more
> > like a gratuitous difference from everyplace else than a useful
> > behavior.
>
> Yeah. Will drop the + signs. I've also written a patch to enable a
> separate "Windows - Server 2022, VS 2019 - Mkvcbuild.pm" CI task
> alongside the Meson one in the REL_16_STABLE branch. I had run the
> affected branches through CI, but of course 16 switched to Meson so it
> wasn't testing the third build system... without that, this is just
> too painful. I need to step away for a couple of hours, but more in a
> bit...
That revealed another problem: Mkvcbuild.pm didn't add -lpgport. It
looks out for the pattern PG_LIBS_INTERNAL = $(libpq_pgport), so
that's an easy way to fix that -- is there a better way? I couldn't
figure out how to tell it that we need libpqport but not libpq.
Here's a CI run of these patches on top of REL_16_STABLE, showing the
new Mkvcbuild.pm task passing:
https://cirrus-ci.com/build/5900754273173504
Attachments: download all (2 files, .tar.gz)
0001-ci-Test-legacy-Windows-build-in-REL_16_STABLE.patch (application/octet-stream, 5.9K) download
From bea1cdf10ea865f68c306f25aeb121c6977c3332 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 22 Dec 2025 12:50:48 +1300
Subject: [PATCH 1/2] ci: Test legacy Windows build in REL_16_STABLE.
* REL_15_STABLE introduced CI and tested Windows with the legacy
build scripts written in Perl.
* REL_16_STABLE introduced Meson and switched Windows CI task to that.
* REL_17_STABLE dropped the legacy Windows build scripts.
This left a blind spot for the non-Windows-enabled developers relying
on CI for testing back-patches of build script changes into 16. They
wouldn't get any testing of the effects on Mkvcbuild.pm until they hit
fairywren in the build farm.
Copy REL_15_STABLE's Windows task into REL_16_STABLE as a separate
task in self-defense, with slight adjustments to share environment
variables with the other Windows tasks in that branch.
Discussion: https://postgr.es/m/175163.1766357334%40sss.pgh.pa.us
---
.cirrus.tasks.yml | 110 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index f670caace1f..caa572d346e 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -655,6 +655,116 @@ task:
type: text/plain
+task:
+ << : *WINDOWS_ENVIRONMENT_BASE
+ name: Windows - Server 2022, VS 2019 - Mkvcbuild.pm
+
+ env:
+ # Our legacy build system doesn't have test concurrency above the level
+ # of a single vcregress test target. Due to that, it's useful to run prove
+ # with multiple jobs. For the other tasks it isn't, because two sources
+ # (make and prove) of concurrency can overload machines.
+ #
+ # The concrete choice of 10 is based on a small bit of experimentation and
+ # likely can be improved upon further.
+ PROVE_FLAGS: -j10 --timer
+
+ # Avoid re-installing over and over
+ NO_TEMP_INSTALL: 1
+ # -m enables parallelism
+ # verbosity:minimal + Summary reduce verbosity, while keeping a summary of
+ # errors/warnings
+ # ForceNoAlign prevents msbuild from introducing line-breaks for long lines
+ # disable file tracker, we're never going to rebuild, and it slows down the
+ # build
+ MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo
+
+ # If tests hang forever, cirrus eventually times out. In that case log
+ # output etc is not uploaded, making the problem hard to debug. Of course
+ # tests internally should have shorter timeouts, but that's proven to not
+ # be sufficient. 15min currently is fast enough to finish individual test
+ # "suites".
+ T_C: "\"C:/Program Files/Git/usr/bin/timeout.exe\" -v -k60s 15m"
+
+ # startcreate_script starts a postgres instance that we don't want to get
+ # killed at the end of that script (it's stopped in stop_script). Can't
+ # trivially use background_scripts because a) need pg_ctl's dropping of
+ # permissions b) need to wait for startup to have finished, and we don't
+ # currently have a tool for that...
+ CIRRUS_ESCAPING_PROCESSES: 1
+
+ # Cirrus defaults to SetErrorMode(SEM_NOGPFAULTERRORBOX | ...). That
+ # prevents crash reporting from working unless binaries do SetErrorMode()
+ # themselves. Furthermore, it appears that either python or, more likely,
+ # the C runtime has a bug where SEM_NOGPFAULTERRORBOX can very
+ # occasionally *trigger* a crash on process exit - which is hard to debug,
+ # given that it explicitly prevents crash dumps from working...
+ # 0x8001 is SEM_FAILCRITICALERRORS | SEM_NOOPENFILEERRORBOX
+ CIRRUS_WINDOWS_ERROR_MODE: 0x8001
+
+ <<: *windows_task_template
+
+ only_if: $CI_WINDOWS_ENABLED
+
+ setup_additional_packages_script: |
+ REM choco install -y --no-progress ...
+
+ configure_script:
+ # copy errors out when using forward slashes
+ - copy src\tools\ci\windows_build_config.pl src\tools\msvc\config.pl
+ - vcvarsall x64
+ - perl src/tools/msvc/mkvcbuild.pl
+ build_script:
+ - vcvarsall x64
+ - msbuild %MSBFLAGS% pgsql.sln
+ tempinstall_script:
+ # Installation on windows currently only completely works from src/tools/msvc
+ - cd src/tools/msvc && perl install.pl %CIRRUS_WORKING_DIR%/tmp_install
+
+ test_regress_parallel_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl check parallel
+ startcreate_script: |
+ rem paths to binaries need backslashes
+ tmp_install\bin\pg_ctl.exe initdb -D tmp_check/db -l tmp_check/initdb.log --options=--no-sync
+ echo include '%TEMP_CONFIG%' >> tmp_check/db/postgresql.conf
+ tmp_install\bin\pg_ctl.exe start -D tmp_check/db -l tmp_check/postmaster.log
+
+ test_pl_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl plcheck
+ test_isolation_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl isolationcheck
+ test_modules_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl modulescheck
+ test_contrib_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl contribcheck
+ stop_script: |
+ tmp_install\bin\pg_ctl.exe stop -D tmp_check/db -l tmp_check/postmaster.log
+ test_ssl_script: |
+ set with_ssl=openssl
+ %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/ssl/
+ test_subscription_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/subscription/
+ test_authentication_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl taptest ./src/test/authentication/
+ test_recovery_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl recoverycheck
+ test_bin_script: |
+ %T_C% perl src/tools/msvc/vcregress.pl bincheck
+ test_ecpg_script: |
+ rem tries to build additional stuff
+ vcvarsall x64
+ rem References ecpg_regression.proj in the current dir
+ cd src/tools/msvc
+ %T_C% perl vcregress.pl ecpgcheck
+
+ # It's not really "_ac" but that has the same patterns as REL_15_STABLE used.
+ on_failure:
+ <<: *on_failure_ac
+ crashlog_artifacts:
+ path: "crashlog-*.txt"
+ type: text/plain
+
+
task:
name: CompilerWarnings
--
2.50.1 (Apple Git-155)
0002-Fix-Mkvcbuild.pm-builds-of-test_cloexec.c.patch (application/octet-stream, 1.6K) download
From 0904001e7d4843998b4430f235c4389657375f44 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 22 Dec 2025 12:13:51 +1300
Subject: [PATCH 2/2] Fix Mkvcbuild.pm builds of test_cloexec.c.
Mkvcbuild.pm was not happy with the Makefile change in commit bec2a0aa,
as revealed by build farm animal fairywren on branch REL_16_STABLE:
1. It used += instead of = in test_cloexec/Makefile. It's valid make
syntax, but the Mkvcbuild.pm's scraping regexes didn't like that
pattern. Drop the unnecessary + signs.
2. Mkvcbuild.pm builds didn't automatically add libpgport. Although
this program doesn't use libpq, setting PG_LIBS_INTERNAL to
$(libpq_pgport) is a simple way to trigger Mkvcbuild.pm to add it.
"vcregress.pl modulescheck" still doesn't actually run the new test (or
other similar tests), because it only supports pg_regress tests.
Backpatch-through: 16
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/175163.1766357334%40sss.pgh.pa.us
---
src/test/modules/test_cloexec/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile
index cd16a59add5..4f0ed58714e 100644
--- a/src/test/modules/test_cloexec/Makefile
+++ b/src/test/modules/test_cloexec/Makefile
@@ -3,8 +3,9 @@
PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling"
PGAPPICON = win32
-PROGRAM += test_cloexec
-OBJS += $(WIN32RES) test_cloexec.o
+PROGRAM = test_cloexec
+OBJS = $(WIN32RES) test_cloexec.o
+PG_LIBS_INTERNAL = $(libpq_pgport)
NO_INSTALLCHECK = 1
TAP_TESTS = 1
--
2.50.1 (Apple Git-155)
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 00:24 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:38 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
@ 2025-12-22 21:50 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 23:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 25+ messages in thread
From: Tom Lane @ 2025-12-22 21:50 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
Thomas Munro <thomas.munro@gmail.com> writes:
> That revealed another problem: Mkvcbuild.pm didn't add -lpgport. It
> looks out for the pattern PG_LIBS_INTERNAL = $(libpq_pgport), so
> that's an easy way to fix that -- is there a better way? I couldn't
> figure out how to tell it that we need libpqport but not libpq.
AFAICT from looking at v16 Mkvcbuild.pm, PG_LIBS_INTERNAL = $(libpq_pgport)
will do exactly what you want because that only triggers it to add
libpgport and libpgcommon (cf. lines 1053ff). I'm a little baffled
by that --- shouldn't it be pulling in libpq as well? But let's let
sleeping dogs lie.
regards, tom lane
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:46 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 00:24 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:38 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:50 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-12-22 23:02 ` Thomas Munro <thomas.munro@gmail.com>
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Munro @ 2025-12-22 23:02 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bryan Green <dbryan.green@gmail.com>; pgsql-hackers
On Tue, Dec 23, 2025 at 10:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > That revealed another problem: Mkvcbuild.pm didn't add -lpgport. It
> > looks out for the pattern PG_LIBS_INTERNAL = $(libpq_pgport), so
> > that's an easy way to fix that -- is there a better way? I couldn't
> > figure out how to tell it that we need libpqport but not libpq.
>
> AFAICT from looking at v16 Mkvcbuild.pm, PG_LIBS_INTERNAL = $(libpq_pgport)
> will do exactly what you want because that only triggers it to add
> libpgport and libpgcommon (cf. lines 1053ff). I'm a little baffled
> by that --- shouldn't it be pulling in libpq as well? But let's let
> sleeping dogs lie.
Thanks for looking. Yeah. That line should probably be v16-only,
conditional on Windows and have a comment to explain. Will confirm
that and push this later today, along with those two CI changes[1][2]
that would have avoided all this trouble.
[1] https://www.postgresql.org/message-id/CA%2BhUKG%2BFRtqt0ZZVxb4e4h1gRnpECvehGB5JHqK73q8LxhbbVQ%40mail...
[2] https://www.postgresql.org/message-id/CA%2BhUKGJreZe9jzPA6jDsLLB18d7M0rvypdbV18UBMxiFxdsf2g%40mail.g...
^ permalink raw reply [nested|flat] 25+ messages in thread
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-06-20 07:00 ` Alexander Lakhin <exclusion@gmail.com>
2026-06-20 17:48 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
1 sibling, 1 reply; 25+ messages in thread
From: Alexander Lakhin @ 2026-06-20 07:00 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; pgsql-hackers
Hello Bryan,
13.12.2025 04:44, Tom Lane wrote:
> Thomas Munro<thomas.munro@gmail.com> writes:
>> Hearing nothing, pushed.
> fairywren is unimpressed:
Now that fairywren is pretty satisfied with the test, it has produced an
interesting failure [1]:
208/222 postgresql:test_cloexec / test_cloexec/001_cloexec ERROR 4.09s exit status 1
regress_log_001_cloexec
[14:36:36.820](1.047s) 1..1
[14:36:36.821](0.001s) # Using test program:
C:\\tools\\xmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\tmp_install\\tools\\xmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\inst\\bin\\test_cloexec.exe
[14:36:37.445](0.624s) # Test program output:
[14:36:37.446](0.001s) # Child: Received HANDLE1=00000000000000B0 (should fail - O_CLOEXEC)
# Child: Received HANDLE2=00000000000000C0 (should work - no O_CLOEXEC)
# Child: Successfully wrote to HANDLE1
# Child: Successfully wrote to HANDLE2
# Child: HANDLE1 (O_CLOEXEC): ACCESSIBLE (BAD!)
# Child: HANDLE2 (no O_CLOEXEC): ACCESSIBLE (GOOD!)
# Child: Test FAILED - O_CLOEXEC not working correctly
# Parent: Opening test files...
# Parent: fd1=3 (O_CLOEXEC) -> HANDLE=00000000000000B0
# Parent: fd2=4 (no O_CLOEXEC) -> HANDLE=00000000000000C0
# Parent: Spawning child process...
# Parent: Command line:
"C:\\tools\\xmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\tmp_install\\tools\\xmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\inst\\bin\\test_cloexec.exe"
00000000000000B0 00000000000000C0
# Parent: Waiting for child process...
# Parent: Child exit code: 1
# Parent: FAILURE - O_CLOEXEC not working correctly
[14:36:37.447](0.002s) not ok 1 - O_CLOEXEC prevents handle inheritance
[14:36:37.448](0.001s) # Failed test 'O_CLOEXEC prevents handle inheritance'
# at C:/tools/xmsys64/home/pgrunner/bf/root/REL_16_STABLE/pgsql/src/test/modules/test_cloexec/t/001_cloexec.pl line 57.
[14:36:37.449](0.001s) # Looks like you failed 1 test of 1.
I've managed to reproduce it locally with:
inline patch 1: src/test/modules/test_cloexec/t/001_cloexec.pl (+4/-2 lines) download inline-1.patch
--- a/src/test/modules/test_cloexec/t/001_cloexec.pl
+++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
@@ -19,3 +19,3 @@ if (!$PostgreSQL::Test::Utils::windows_os)
-plan tests => 1;
+plan tests => 1000;
@@ -44,2 +44,4 @@ note("Using test program: $test_prog");
+for (my $i = 1; $i <= 1000; $i++)
+{
my ($stdout, $stderr);
@@ -58,3 +60,3 @@ ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s,
"O_CLOEXEC prevents handle inheritance");
-
+}
done_testing();
* Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Thomas Munro <thomas.munro@gmail.com>
2025-12-13 02:44 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Tom Lane <tgl@sss.pgh.pa.us>
2026-06-20 07:00 ` Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Alexander Lakhin <exclusion@gmail.com>
@ 2026-06-20 17:48 ` Bryan Green <dbryan.green@gmail.com>
0 siblings, 0 replies; 25+ messages in thread
From: Bryan Green @ 2026-06-20 17:48 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Thomas Munro <thomas.munro@gmail.com>; pgsql-hackers
On 6/20/2026 2:00 AM, Alexander Lakhin wrote:
> Hello Bryan,
>
> 13.12.2025 04:44, Tom Lane wrote:
>> Thomas Munro<thomas.munro@gmail.com> writes:
>>> Hearing nothing, pushed.
>> fairywren is unimpressed:
>
> Now that fairywren is pretty satisfied with the test, it has produced an
> interesting failure [1]:
> 208/222 postgresql:test_cloexec /
> test_cloexec/001_cloexec ERROR 4.09s exit status 1
>
> regress_log_001_cloexec
> [14:36:36.820](1.047s) 1..1
> [14:36:36.821](0.001s) # Using test program: C:\\tools\\xmsys64\\home\
> \pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\tmp_install\\tools\
> \xmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\inst\\bin\
> \test_cloexec.exe
> [14:36:37.445](0.624s) # Test program output:
> [14:36:37.446](0.001s) # Child: Received HANDLE1=00000000000000B0
> (should fail - O_CLOEXEC)
> # Child: Received HANDLE2=00000000000000C0 (should work - no O_CLOEXEC)
> # Child: Successfully wrote to HANDLE1
> # Child: Successfully wrote to HANDLE2
> # Child: HANDLE1 (O_CLOEXEC): ACCESSIBLE (BAD!)
> # Child: HANDLE2 (no O_CLOEXEC): ACCESSIBLE (GOOD!)
> # Child: Test FAILED - O_CLOEXEC not working correctly
> # Parent: Opening test files...
> # Parent: fd1=3 (O_CLOEXEC) -> HANDLE=00000000000000B0
> # Parent: fd2=4 (no O_CLOEXEC) -> HANDLE=00000000000000C0
> # Parent: Spawning child process...
> # Parent: Command line: "C:\\tools\\xmsys64\\home\\pgrunner\\bf\\root\
> \REL_16_STABLE\\pgsql.build\\tmp_install\\tools\\xmsys64\\home\
> \pgrunner\\bf\\root\\REL_16_STABLE\\inst\\bin\\test_cloexec.exe"
> 00000000000000B0 00000000000000C0
> # Parent: Waiting for child process...
> # Parent: Child exit code: 1
> # Parent: FAILURE - O_CLOEXEC not working correctly
> [14:36:37.447](0.002s) not ok 1 - O_CLOEXEC prevents handle inheritance
> [14:36:37.448](0.001s) # Failed test 'O_CLOEXEC prevents handle
> inheritance'
> # at C:/tools/xmsys64/home/pgrunner/bf/root/REL_16_STABLE/pgsql/src/
> test/modules/test_cloexec/t/001_cloexec.pl line 57.
> [14:36:37.449](0.001s) # Looks like you failed 1 test of 1.
>
> I've managed to reproduce it locally with:
> --- a/src/test/modules/test_cloexec/t/001_cloexec.pl
> +++ b/src/test/modules/test_cloexec/t/001_cloexec.pl
> @@ -19,3 +19,3 @@ if (!$PostgreSQL::Test::Utils::windows_os)
>
> -plan tests => 1;
> +plan tests => 1000;
>
> @@ -44,2 +44,4 @@ note("Using test program: $test_prog");
>
> +for (my $i = 1; $i <= 1000; $i++)
> +{
> my ($stdout, $stderr);
> @@ -58,3 +60,3 @@ ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior
> verified/s,
> "O_CLOEXEC prevents handle inheritance");
> -
> +}
> done_testing();
>
> and with the test cloned:
> perl -i.bak -ne "print unless /test_cloexec_/" "src/test/modules/
> meson.build" && python3 -c "for i in range(1,50+1):import os;import
> shutil;sd=\"src/test/modules/test_cloexec/\";td=f\"src/test/modules/
> test_cloexec_{i}\";shutil.rmtree(td,ignore_errors=1);shutil.copytree(sd,
> td);assert(os.system(f'perl -i.bak -pe \"s#(subdir.
> \'test_cloexec\'.)#subdir(\'test_cloexec_{i}\'){chr(92)}n{chr(92)}1#\"
> \"src/test/modules/meson.build\"')==0);assert(os.system(f'perl -i.bak -
> pe \"s#: \'test_cloexec\',#: \'test_cloexec_{i}\',#\" \"{td}/
> meson.build\"')==0)"
>
> meson test test_cloexec_*/001_cloexec --num-processes 50
> failed for me as below:
> Ok: 19
> Fail: 31
>
> # grep 'not ok' meson-logs/testlog.txt
> not ok 541 - O_CLOEXEC prevents handle inheritance
> not ok 303 - O_CLOEXEC prevents handle inheritance
> not ok 798 - O_CLOEXEC prevents handle inheritance
> not ok 901 - O_CLOEXEC prevents handle inheritance
> not ok 634 - O_CLOEXEC prevents handle inheritance
> not ok 640 - O_CLOEXEC prevents handle inheritance
> not ok 770 - O_CLOEXEC prevents handle inheritance
> not ok 938 - O_CLOEXEC prevents handle inheritance
> not ok 420 - O_CLOEXEC prevents handle inheritance
> not ok 674 - O_CLOEXEC prevents handle inheritance
> not ok 114 - O_CLOEXEC prevents handle inheritance
> not ok 649 - O_CLOEXEC prevents handle inheritance
> not ok 51 - O_CLOEXEC prevents handle inheritance
> not ok 692 - O_CLOEXEC prevents handle inheritance
> not ok 874 - O_CLOEXEC prevents handle inheritance
> not ok 955 - O_CLOEXEC prevents handle inheritance
> not ok 690 - O_CLOEXEC prevents handle inheritance
> not ok 33 - O_CLOEXEC prevents handle inheritance
> not ok 116 - O_CLOEXEC prevents handle inheritance
> not ok 398 - O_CLOEXEC prevents handle inheritance
> not ok 773 - O_CLOEXEC prevents handle inheritance
> not ok 2 - O_CLOEXEC prevents handle inheritance
> not ok 253 - O_CLOEXEC prevents handle inheritance
> not ok 10 - O_CLOEXEC prevents handle inheritance
> not ok 902 - O_CLOEXEC prevents handle inheritance
> not ok 848 - O_CLOEXEC prevents handle inheritance
> not ok 832 - O_CLOEXEC prevents handle inheritance
> not ok 38 - O_CLOEXEC prevents handle inheritance
> not ok 986 - O_CLOEXEC prevents handle inheritance
> not ok 688 - O_CLOEXEC prevents handle inheritance
> not ok 872 - O_CLOEXEC prevents handle inheritance
> not ok 582 - O_CLOEXEC prevents handle inheritance
> not ok 816 - O_CLOEXEC prevents handle inheritance
> not ok 277 - O_CLOEXEC prevents handle inheritance
> not ok 570 - O_CLOEXEC prevents handle inheritance
> not ok 691 - O_CLOEXEC prevents handle inheritance
> not ok 871 - O_CLOEXEC prevents handle inheritance
> not ok 770 - O_CLOEXEC prevents handle inheritance
> not ok 108 - O_CLOEXEC prevents handle inheritance
> not ok 610 - O_CLOEXEC prevents handle inheritance
> not ok 866 - O_CLOEXEC prevents handle inheritance
> not ok 909 - O_CLOEXEC prevents handle inheritance
> not ok 276 - O_CLOEXEC prevents handle inheritance
> not ok 190 - O_CLOEXEC prevents handle inheritance
>
> It looks like HANDLE1 passed from parent to child can accidentally point
> to a child slot occupied by some other writable object, not inherited from
> parent.
>
> [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?
> nm=fairywren&dt=2026-06-19%2013%3A28%3A44
>
> Best regards,
> Alexander
Yeah, the test script has problems. Passing raw numeric handle values
is problematic. When using CreateProcess with bInheritHandles=TRUE, the
kernel duplicates "inheritable" handles into the child at the same
numeric value. So, HANDLE2 works reliably since 0xc0 is he same file
object in both processes.
The non-inherited handle is simply unmanaged in the child. O_CLOEXEC
means that 0xB0 will not be duplicated into the child process. The
problem is that we have no promise that some other kernel object handle
is not at that offset in the child. The child's own startup populates
its handle table and windows reuses low handle values aggressively.
Assuming, the handle was marked as not inheritable in the parent but the
child's handle table has an entry at that same offset for another kernel
object: try_write_to_handle() does a writefile against another kernel
object and succeeds. This is a false failure. This test can't tell the
difference between the actual inherited file and another kernel object
that just happened to have this offset in the handle table assigned to it.
Running in parallel will hit this problem often.
Also, by writing into whatever random kernel object that is referenced
by this offset in the handle table in the child could cause real side
effects when it misfires.
We have a couple of options: 1) use GetHandleInformation for each
handle and test the HANDLE_FLAG_INHERIT bit. This would be a completely
deterministic test and no spawn shenanigans. 2) If we want to keep an
end to end spawn test-- we pass the file paths to the child as well as
the expected handles and use GetFinalPathNameByHandle() to test that the
passed in handles resolve to the correct paths in the child.
The end to end test just seems to be testing that CreateProcess honors
the O_CLOEXEC, which is testing the OS behavior. The
GetHandleInformation(handle1/handle2) check for HANDLE_FLAG_INHERIT is
simpler and would show that the handle has been marked as inheritable or
not. We should trust the kernel to do what it says it does and just
test that our code path for O_CLOEXEC actually creates the handle as
either inheritable or not. This is what GetHandleInformation can tell
us and it removes the need for the spawn.
I am busy with an unfortunate family emergency or I would just code this
as a patch now given agreement on path, but I can get to this by the end
of the week if no one else cares to implement.
--
Bryan Green
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 25+ messages in thread
end of thread, other threads:[~2026-06-20 17:48 UTC | newest]
Thread overview: 25+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-10-31 17:16 [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain Bryan Green <dbryan.green@gmail.com>
2025-11-06 13:43 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-06 14:42 ` Bryan Green <dbryan.green@gmail.com>
2025-11-07 17:28 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 01:02 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 02:34 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-12 22:10 ` Bryan Green <dbryan.green@gmail.com>
2025-11-12 22:17 ` Bryan Green <dbryan.green@gmail.com>
2025-11-30 00:13 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-09 21:03 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-09 22:04 ` Bryan Green <dbryan.green@gmail.com>
2025-12-13 02:44 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-13 03:57 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-13 04:10 ` Bryan Green <dbryan.green@gmail.com>
2025-12-13 04:46 ` Bryan Green <dbryan.green@gmail.com>
2025-12-13 06:33 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-14 05:09 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-21 10:41 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-21 22:48 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 00:24 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:38 ` Thomas Munro <thomas.munro@gmail.com>
2025-12-22 21:50 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-12-22 23:02 ` Thomas Munro <thomas.munro@gmail.com>
2026-06-20 07:00 ` Alexander Lakhin <exclusion@gmail.com>
2026-06-20 17:48 ` Bryan Green <dbryan.green@gmail.com>
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