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.
