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@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?