ole32:fix CoWaitForMultipleHandles cause RPC hang

Changhui Liu liuchanghui at linuxdeepin.com
Sun Dec 14 22:34:33 CST 2014


Hi,
I got it. The CoWaitForMultipleHandles of Microsoft implement is this:
The message loop continue although has received WM_QUIT,
but just dispatch message who sent to the OleMainThreadWndClass Window.


I attached a latest patch and a verify function.
Run the test_CoWaitForMultipleHandlesDispatchMsg, We will only get the
message who send to cowait_test_class window .  


It also pass your PeekMessage test at:
https://github.com/wine-compholio/wine-staging/blob/master/patches/ole32-CoWaitForMultipleHandles/0001-ole32-tests-Add-additional-tests-for-CoWaitForMultip.patch


Please help me review it again when you are free.
Thank you.









------------------
Regards.
 

 
 
 
------------------ Original ------------------
From:  "Sebastian Lackner"<sebastian at fds-team.de>;
Date:  Sat, Dec 13, 2014 11:32 PM
To:  "Changhui Liu"<liuchanghui at linuxdeepin.com>; 
Cc:  "wine-devel"<wine-devel at winehq.org>; 
Subject:  Re: ole32:fix CoWaitForMultipleHandles cause RPC hang

 
Hi,

On 13.12.2014 16:15, Changhui Liu wrote:
> Hi Sebastian,
> 
> 
> I tested your current work-in-progress patch:
> https://github.com/wine-compholio/wine-staging/blob/master/patches/ole32-CoWaitForMultipleHandles/0001-ole32-tests-Add-additional-tests-for-CoWaitForMultip.patch
> 
> I have two puzzles about these code:
>>       index = 0xdeadbeef;
>>       PostMessageA(hWnd, WM_DDE_FIRST, 0, 0);
>>       PostQuitMessage(44);
>>       thread = CreateThread(NULL, 0, post_message_thread, hWnd, 0, &tid);
>>       ok(thread != NULL, "CreateThread failed, error %u\n", GetLastError());
>>       hr = CoWaitForMultipleHandles(0, 100, 2, handles, &index);
>>       ok(hr == RPC_S_CALLPENDING, "expected RPC_S_CALLPENDING, got 0x%08x\n", hr);
>>       ok(index == 0 || broken(index == 0xdeadbeef) /* Win 8 */, "expected index 0, got %u\n", index);
>>       success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_FIRST, PM_REMOVE);
>>       ok(success, "PeekMessageA failed, error %u\n", GetLastError());
> 
> 
> The first, why use the PeekMessage to check if received a WM_DDE_FIRST message?
> Is there an real application do like that way?

Of course there are multiple ways to test for window messages in the message queue, I just picked PeekMessageA because it doesn't require setting up a Wndproc.

> 
> 
> In fact CoWaitForMultipleHandles has a message loop, the WM_DDE_FIRST message has been
> dispatched to the window's WNDPROC. So the PeekMessageA certainly failed. 

No, it doesn't. Feel free to run the tests yourself on a Windows machine, I have tested them with the Wine testbot and all tests pass on 2000/XP/Vista/8/2008/... - which means that Windows does not process all WM_DDE_FIRST messages.

This is also why I am unsure about how to fix it. When changing the behaviour in order to fix one of the tests which fails in Wine (marked with todo_wine), it breaks other tests which were working well before.

> 
> 
> I think the correct test should be this: 
> 1, Define a custom WNDPROC function named cowait_test_wnd_proc .
> 2, Define a global int variable named g_count_of_wm_dde_first.
> 3, Set the value of g_count_of_wm_dde_first to zero before call CoWaitForMultipleHandles.
> 4, Increase the value of g_count_of_wm_dde_first by one, once received a WM_DDE_FIRST message in cowait_test_wnd_proc.
> 5, Check if the value of g_count_of_wm_dde_first is equal to 2 after CoWaitForMultipleHandles returned. 
> 

This testing method would also be fine, but you would get 1 instead of 2. My patch doesn't show any test failures on Windows, which means that one of the WM_DDE_FIRST messages is still in the queue.

> 
> The second  is why use todo_wine ?
>         todo_wine
>         ok(!success, "PeekMessageA succeeded\n");
> why not just write?
>         ok(!success, "PeekMessageA succeeded\n"); 
> 

The Wine coding guideline does not allow to add new test failures, instead all tests which do not pass yet have to be marked with "todo_wine". This makes easily visible which tests need further work just by looking at the source code, moreover it avoids introducing regressions (breaking tests which were working well before). Take a look at some of the other Wine tests, this is not the only place where its used.

> 
> 
> 
> Thank you.

Regards,
Sebastian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20141215/aab6ad7b/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ole32-fix-CoWaitForMultipleHandles-cause-RPC-hang.txt
Type: application/octet-stream
Size: 9368 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20141215/aab6ad7b/attachment.obj>


More information about the wine-devel mailing list