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?