Re: [PATCH v2] Cygwin: Ensure unused fd available for open()
Jon Turney <[email protected]> Sun, 7 Jun 2026 19:29:09 +0100
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
On 07/06/2026 08:54, Mark Geisert wrote: > Hi Jon, > > On 6/1/2026 6:30 AM, Jon Turney wrote: >> On 28/05/2026 06:42, Mark Geisert wrote: >>> The existing logic for open() assumes an fd is always available in >>> the fdtable for a created file. This leads to a situation where, if >>> there is no fd available due to the OPEN_MAX limit being hit, the >>> file is created but cannot be referenced by a Cygwin fd. >>> >>> Move the fd reservation code to an earlier location within open(). >> >> Hmm... the more I stare at cygheap_fdnew, the less sure I am I >> understand what's going on. >> >> I'm sure you considered this, but just so I can tell myself I've done >> due diligence, perhaps you can briefly explain why this doesn't create >> the opposite leak? (i.e. the reserved fd is released if actually >> opening the file fails). > > Sure. What happens is that cygheap_fdnew doesn't mark the chosen fd > reserved (i.e. the fdtable is not updated at all, yet), it's that the > calling thread has locked the fdtable and knows where the first unused > fd in fdtable is. > > All the validations of open() parameters are done and eventually a file, > pipe, device, socket, whatever open attempt at Windows level is done. If > that succeeds, fdtable[fd] is updated with a pointer to the fhandler_XXX > stuff being carried along in variable fh. The fdtable is unlocked at > the end of the __try block by a dtor (see below). > > If that Windows-level open attempt fails, a __leave is performed to exit > the enclosing __try block. The destructor for cygheap_fdmanip, > superclass of cygheap_fdnew, unlocks the fdtable. fdtable[fd] is left > as it was, NULL. > > That's my story and I'm sticking to it, but I'm at the limits of my C++ > knowledge. The overloading of "fd" really makes it difficult to follow > things.. but I have to admit this seems like tight code to me. H/T to > CGF warranted. Thanks. Yes, that all makes sense. Thanks for looking into this. I pushed the patch. Is this a candidate for 3.6 branch as well?