[PATCH] server: Use a free list for unallocated object handles.

[Zebediah Figura]

On 2/15/22 16:15, Zebediah Figura wrote:
> On 2/1/22 14:56, Zebediah Figura wrote:
>> On 2/1/22 13:54, Alexandre Julliard wrote:
>>> Zebediah Figura <zfigura at codeweavers.com> writes:
>>>
>>>> The Legend of Heroes: Trails of Cold Steel III suffers from an application bug,
>>>> where it tries to wait on a handle after closing it. Because of the usage
>>>> patterns of the game and the way Wine allocates handles, the handle is usually
>>>> reused by a separate object, which is effectively never signaled, resulting in a
>>>> hang.
>>>>
>>>> This patch changes our handle allocation strategy to resemble Windows, and in
>>>> the process makes the race much less likely, although still theoretically
>>>> possible.
>>>
>>> Some tests would be nice.
>>>
>>
>> That may be difficult. When I was testing this manually I ran into
>> hiccups due to some background thread in the process opening or closing
>> handles.
>>
> 
> I tried writing tests for this along the following lines: allocate a
> large number of handles, close them all, then allocate the same number
> and check that they're allocated in the reverse order.
> 
> Sometimes (depending on timing, and perhaps also Windows version?) the
> tests pass with flying colours. But sometimes they fail, and fail quite
> randomly at that. Sometimes it's clear that one or two handles were
> allocated out of order, as if another thread allocated and/or freed
> them, but sometimes it seems like hundreds are. Perhaps that's evidence
> that Windows isn't quite using a free list for handle allocation the way
> this patch has it, but the fact that it at least sometimes passes makes
> me think it is.
> 

I accidentally hit send before I was done writing...

Anyway, I'm not sure what else to do here. I could drop the patch since 
it's not fully clear it's correct and it doesn't even make the described 
race impossible, but it doesn't seem likely that there's a more correct 
solution, or that it's going to be easily found if so.

I guess I could also write tests that allocate and close a single handle 
in a loop, and test that it's "usually" the same handle, within some 
tolerance. That doesn't seem particularly helpful, though; there are 
many ways to pass that test (e.g. our current implementation will).

Or perhaps you had ideas for tests already in mind that aren't occurring 
to me?