Thread

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

Thomas Munro <thomas.munro@gmail.com> — 2025-11-30T00:13:16Z

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