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

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%40mail.gmail.com
> 

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