Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain
| От | Thomas Munro |
|---|---|
| Тема | Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain |
| Дата | |
| Msg-id | CA+hUKGJh0a_6EP8NLh6jjQjzzg=Nm_h9HMC19-tBww7ZqQDWVg@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: [PATCH] O_CLOEXEC not honored on Windows - handle inheritance chain (Bryan Green <dbryan.green@gmail.com>) |
| Список | 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
Вложения
В списке pgsql-hackers по дате отправления: