[PATCH 1/5] user32/tests: Add more tests for windows destroyed by thread exit.

Rémi Bernon rbernon at codeweavers.com
Tue Feb 8 05:53:10 CST 2022


On 2/8/22 12:39, Jacek Caban wrote:
> Hi Rémi,
> 
> On 2/1/22 21:59, Jacek Caban wrote:
>> Hi Rémi,
>>
>> On 2/1/22 15:52, Rémi Bernon wrote:
>>> On 2/1/22 15:12, Jacek Caban wrote:
>>>> Signed-off-by: Jacek Caban <jacek at codeweavers.com>
>>>> ---
>>>>   dlls/user32/tests/msg.c | 33 +++++++++++++++++++++++++++++++++
>>>>   1 file changed, 33 insertions(+)
>>>>
>>>
>>> Hi Jacek!
>>>
>>> I've been trying to fix this for quite some time as I believe we have 
>>> a race condition in thread destroy and that causes some spurious 
>>> user32:msg test failures already.
>>>
>>> I wrote a few more tests (attached) before to check Windows behavior 
>>> and they don't pass with your changes.
>>>
>>> I also had several variations of the fix, and I'm attaching the last 
>>> one for reference, although I understand PATCH 3 and 4 which are 
>>> changing the parent pointer semantics on the wineserver side is a bit 
>>> risky.
>>>
>>> From the errors returned by window functions it doesn't look 
>>> completely off though (some functions fail in the same way for 
>>> desktop window and for orphaned windows, both not having a parent).
>>>
>>> Note that my fix then doesn't pass your WM_NCDESTROY parent test either. 
>>
>>
>> Thanks, those patches look interesting. I tested your tests from the 
>> first two patches. Most of failures are not strictly related to 
>> destroying window on thread exit, they fail the same way when normal 
>> DestroyWindow() is used. I attached a patch that confirms that with 
>> tests and has a quick fix for that. I still need to look at todo_wine 
>> around IsWindow(child3) test.
> 
> 
> The remaining todo_wine is addressed by your patch 6 and 7, which still 
> works fine. I left it out for now. Also more testing showed that things 
> like MoveWindow() should still work. We implement it on top of 
> SetWindowPos() (which is supposed to fail), so we'd need to change that 
> a bit to make SetWindowPos fail on orphaned windows.
> 
> 
> Also, looking closer at this, I think that we really should keep parent 
> object around until its children are properly destroyed. I submitted a 
> new series implementing it.
> 

I saw that, didn't look very close (or at your tree, yet) but I trust it 
does the right thing, thanks for taking over!
-- 
Rémi Bernon <rbernon at codeweavers.com>



More information about the wine-devel mailing list