From: Kevin Puetz PuetzKevinA@JohnDeere.com
Timeout errors should only occur before/during sleep. If the thread calling CoWaitForMultipleHandles experienced a large scheduling delay (or is simply running slow) it should finish pumping messages/checking handles (and maybe succeed) before declaring WAIT_TIMEOUT/RPC_S_CALLPENDING.
Notably, the previous code had a narrow race (particularly for timeout=0) if GetTickCount increments between start_time and `now`, it could return WAIT_TIMEOUT even when the handles were already signaled before the call. This could also manifest even with longer timeouts, if it dispatched a message whose WNDPROC took quite a while to return control.
The order of operations in CoWaitForMultipleHandles should be:
1. check for completion, and return success if so 2. pump a message that might cause progress toward completion if anything was dispatched, return to step 1 and re-check 3. If there is still timeout remaining, wait for something to change
NOTE: this means CoWaitForMultipleHandles does not time out while there are still relevant messages, and will livelock (and never time out) if dispatching of messages continuously posts additional work. It will exit (successfully) if the handles eventually become signaled.
--
That "overdue success" case is tested, and proves that the CoWait will keep going past the specified timeout. The "indefinitely livelock" when the handles are never signaled has been verified on windows 10 (omit release_semaphore_thread to see it), but I couldn't think of a way to express that as an automated test. --- dlls/combase/combase.c | 108 +++++++++++++++++++------------------ dlls/ole32/tests/compobj.c | 43 +++++++++++++++ 2 files changed, 98 insertions(+), 53 deletions(-)
diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index 42e9fbc2468..fe366b2ba8a 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -2098,7 +2098,7 @@ HRESULT WINAPI CoWaitForMultipleHandles(DWORD flags, DWORD timeout, ULONG handle
if (message_loop) { - DWORD start_time, wait_flags = MWMO_INPUTAVAILABLE; + DWORD start_time, wait_flags = 0; struct tlsdata *tlsdata;
if (FAILED(hr = com_get_tlsdata(&tlsdata))) @@ -2114,74 +2114,76 @@ HRESULT WINAPI CoWaitForMultipleHandles(DWORD flags, DWORD timeout, ULONG handle while (TRUE) { DWORD now = GetTickCount(); - res = WAIT_TIMEOUT; - if (now - start_time > timeout) - { - break; - } + MSG msg;
TRACE("waiting for rpc completion or window message\n");
res = WaitForMultipleObjectsEx(handle_count, handles, !!(flags & COWAIT_WAITALL), 0, !!(flags & COWAIT_ALERTABLE));
- if (res == WAIT_TIMEOUT) - res = MsgWaitForMultipleObjectsEx(handle_count, handles, + if (res != WAIT_TIMEOUT) + { + break; + } + else if (com_peek_message(apt, &msg)) + { + if (msg.message == WM_QUIT) + { + TRACE("Received WM_QUIT message\n"); + post_quit = TRUE; + exit_code = msg.wParam; + } + else + { + TRACE("Received message whilst waiting for RPC: 0x%04x\n", msg.message); + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + } + else + { + if (now - start_time <= timeout) + { + /* nothing to return, no messages to pump, time remaining: + * sleep for the remaining timeout (or until something happens) */ + res = MsgWaitForMultipleObjectsEx(handle_count, handles, timeout == INFINITE ? INFINITE : start_time + timeout - now, QS_SENDMESSAGE | QS_ALLPOSTMESSAGE | QS_PAINT, wait_flags); + }
- if (res == WAIT_OBJECT_0 + handle_count) /* messages available */ - { - MSG msg; - - /* call message filter */ - - if (apt->filter) + if (res == WAIT_OBJECT_0 + handle_count) /* messages available */ { - PENDINGTYPE pendingtype = tlsdata->pending_call_count_server ? PENDINGTYPE_NESTED : PENDINGTYPE_TOPLEVEL; - DWORD be_handled = IMessageFilter_MessagePending(apt->filter, 0 /* FIXME */, now - start_time, pendingtype); - - TRACE("IMessageFilter_MessagePending returned %ld\n", be_handled); + /* call message filter */
- switch (be_handled) + if (apt->filter) { - case PENDINGMSG_CANCELCALL: - WARN("call canceled\n"); - hr = RPC_E_CALL_CANCELED; - goto done; - break; - case PENDINGMSG_WAITNOPROCESS: - case PENDINGMSG_WAITDEFPROCESS: - default: - /* FIXME: MSDN is very vague about the difference - * between WAITNOPROCESS and WAITDEFPROCESS - there - * appears to be none, so it is possibly a left-over - * from the 16-bit world. */ - break; + PENDINGTYPE pendingtype = tlsdata->pending_call_count_server ? PENDINGTYPE_NESTED : PENDINGTYPE_TOPLEVEL; + DWORD be_handled = IMessageFilter_MessagePending(apt->filter, 0 /* FIXME */, now - start_time, pendingtype); + + TRACE("IMessageFilter_MessagePending returned %ld\n", be_handled); + + switch (be_handled) + { + case PENDINGMSG_CANCELCALL: + WARN("call canceled\n"); + hr = RPC_E_CALL_CANCELED; + goto done; + break; + case PENDINGMSG_WAITNOPROCESS: + case PENDINGMSG_WAITDEFPROCESS: + default: + /* FIXME: MSDN is very vague about the difference + * between WAITNOPROCESS and WAITDEFPROCESS - there + * appears to be none, so it is possibly a left-over + * from the 16-bit world. */ + break; + } } } - - if(com_peek_message(apt, &msg)) + else { - wait_flags |= MWMO_INPUTAVAILABLE; - if (msg.message == WM_QUIT) - { - TRACE("Received WM_QUIT message\n"); - post_quit = TRUE; - exit_code = msg.wParam; - } - else - { - TRACE("Received message whilst waiting for RPC: 0x%04x\n", msg.message); - TranslateMessage(&msg); - DispatchMessageW(&msg); - } - } else { - /* no interesting input remains in the queue, so block until *new* messages are posted */ - wait_flags &= ~MWMO_INPUTAVAILABLE; + break; } - continue; } - break; } } else diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index 1e011e1354b..869ab99122c 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -2881,6 +2881,7 @@ static void test_CoWaitForMultipleHandles(void) HGLOBAL execute_apc = globalalloc_string("[apc]"); HGLOBAL execute_postmessage = globalalloc_string("[postmessage]"); HGLOBAL execute_semaphore = globalalloc_string("[semaphore]"); + DWORD start_time;
hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); ok(hr == S_OK, "CoInitializeEx failed with error 0x%08lx\n", hr); @@ -2964,6 +2965,23 @@ static void test_CoWaitForMultipleHandles(void) success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_LAST, PM_REMOVE); ok(!success, "CoWaitForMultipleHandles didn't pump enough messages\n");
+ /* test CoWaitForMultipleHandles will keep working past the timeout if the queue + * still has messages (this is ensured by posting a message that, when dispatched, + * just posts another identical message, so the queue can *never* be drained). + * but it will still exit with success once the handles become signaled */ + index = 0xdeadbeef; + PostMessageA(hWnd, WM_DDE_EXECUTE, 0, (LPARAM)execute_postmessage); + start_time = GetTickCount(); + /* if this thread is disabled, the call to CoWaitForMultipleHandles will livelock forever, on both windows and wine */ + thread = CreateThread(NULL, 0, release_semaphore_thread, handles[0], 0, &tid); + hr = CoWaitForMultipleHandles(0, 50, 1, handles, &index); + ok(GetTickCount() - start_time >= 200, "CoWaitForMultipleHandles exited too soon\n"); + ok(hr == S_OK, "expected S_OK, got 0x%08lx\n", hr); + cowait_msgs_expect_queued(hWnd,WM_DDE_EXECUTE); /* each pumped execute_postmessage added one more back */ + success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_LAST, PM_REMOVE); + ok(!success, "CoWaitForMultipleHandles didn't pump enough messages\n"); + CloseHandle(thread); + /* test PostMessageA/SendMessageA from a different thread */
index = 0xdeadbeef; @@ -3026,6 +3044,31 @@ static void test_CoWaitForMultipleHandles(void) ok(hr == S_OK, "expected S_OK, got 0x%08lx\n", hr); ok(index == 0, "expected index 0, got %lu\n", index);
+ ReleaseSemaphore(handles[1], 1, NULL); + + /* COWAIT_ALL will pump message which are already in the queue, + * (but no longer QS_ALLPOSTMESSAGE), before blocking in MsgWaitForMultipleObjectsEx */ + index = 0xdeadbeef; + PostMessageA(hWnd, WM_DDE_EXECUTE, 0, (LPARAM)execute_semaphore); + PeekMessageA(&msg, hWnd, 0, 0, PM_NOREMOVE); // clear QS_ALLPOSTMESSAGE + hr = CoWaitForMultipleHandles(COWAIT_WAITALL, 50, 1, handles, &index); + ok(hr == S_OK, "expected S_OK, got 0x%08lx\n", hr); + ok(index == 0, "expected index 0, got %lu\n", index); + success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_FIRST, PM_REMOVE); + ok(!success, "CoWaitForMultipleHandles didn't pump any messages\n"); + + ReleaseSemaphore(handles[1], 1, NULL); + + /* test early completion (rather than blocking in MsgWaitForMultipleObjectsEx again) + * if pumping a message results in all handles becoming signaled) */ + index = 0xdeadbeef; + PostMessageA(hWnd, WM_DDE_EXECUTE, 0, (LPARAM)execute_semaphore); + hr = CoWaitForMultipleHandles(COWAIT_WAITALL, 50, 2, handles, &index); + ok(hr == S_OK, "expected S_OK, got 0x%08lx\n", hr); + ok(index == 0, "expected index 0, got %lu\n", index); + success = PeekMessageA(&msg, hWnd, WM_DDE_FIRST, WM_DDE_FIRST, PM_REMOVE); + ok(!success, "CoWaitForMultipleHandles didn't pump any messages\n"); + ReleaseSemaphore(handles[0], 1, NULL); ReleaseSemaphore(handles[1], 1, NULL);