Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain

On Wed, Nov 12, 2025 at 2:01 PM Bryan Green <dbryan(dot)green(at)gmail(dot)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%40mail.gmail.com

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'.