This test is failing from time to time. Making sure the parent thread has terminated before continuing triggers the underlying race condition, and makes the test to always fail.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/user32/tests/msg.c | 61 +++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 24 deletions(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 423157b623d..55b4fe2228b 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8539,6 +8539,8 @@ static void test_paint_messages(void) struct wnd_event { HWND hwnd; + HWND child_hwnd; + HANDLE child; HANDLE grand_child; HANDLE start_event; HANDLE stop_event; @@ -8571,44 +8573,50 @@ static DWORD CALLBACK create_grand_child_thread( void *param ) struct wnd_event *wnd_event = param; HWND hchild; MSG msg; + DWORD ret;
hchild = CreateWindowExA(0, "TestWindowClass", "Test child", - WS_CHILD | WS_VISIBLE, 0, 0, 10, 10, wnd_event->hwnd, 0, 0, NULL); + WS_CHILD | WS_VISIBLE, 0, 0, 10, 10, wnd_event->child_hwnd, 0, 0, NULL); ok (hchild != 0, "Failed to create child window\n"); flush_events(); flush_sequence(); SetEvent( wnd_event->start_event );
- for (;;) - { - MsgWaitForMultipleObjects(0, NULL, FALSE, 1000, QS_ALLINPUT); - if (!IsWindow( hchild )) break; /* will be destroyed when parent thread exits */ - while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - } + ret = WaitForSingleObject( wnd_event->child, 5000 ); + ok( !ret, "WaitForSingleObject returned %x, error: %u\n", ret, GetLastError() ); + + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); + todo_wine + ok( !IsWindow( hchild ), "Child window not destroyed\n" ); + return 0; }
static DWORD CALLBACK create_child_thread( void *param ) { struct wnd_event *wnd_event = param; - struct wnd_event child_event; DWORD ret, tid; MSG msg;
- child_event.hwnd = CreateWindowExA(0, "TestWindowClass", "Test child", - WS_CHILD | WS_VISIBLE, 0, 0, 10, 10, wnd_event->hwnd, 0, 0, NULL); - ok (child_event.hwnd != 0, "Failed to create child window\n"); - SetFocus( child_event.hwnd ); + wnd_event->child_hwnd = CreateWindowExA( 0, "TestWindowClass", "Test child", + WS_CHILD | WS_VISIBLE, 0, 0, 10, 10, + wnd_event->hwnd, 0, 0, NULL ); + ok (wnd_event->child_hwnd != 0, "Failed to create child windows\n"); + SetFocus( wnd_event->child_hwnd ); + + wnd_event->grand_child = CreateThread( NULL, 0, create_grand_child_thread, wnd_event, 0, &tid ); + ok( wnd_event->grand_child != 0, "CreateThread failed, error %u\n", GetLastError() ); + flush_events(); flush_sequence(); - child_event.start_event = wnd_event->start_event; - wnd_event->grand_child = CreateThread(NULL, 0, create_grand_child_thread, &child_event, 0, &tid); for (;;) { - DWORD ret = MsgWaitForMultipleObjects(1, &child_event.start_event, FALSE, 1000, QS_SENDMESSAGE); + ret = MsgWaitForMultipleObjects(1, &wnd_event->start_event, FALSE, 5000, QS_SENDMESSAGE); if (ret != 1) break; while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); } + ok( !ret, "MsgWaitForMultipleObjects failed %x, error: %u\n", ret, GetLastError() ); + ret = WaitForSingleObject( wnd_event->stop_event, 5000 ); ok( !ret, "WaitForSingleObject failed %x\n", ret ); return 0; @@ -8742,33 +8750,38 @@ static void test_interthread_messages(void) wnd_event.hwnd = CreateWindowExA(0, "TestParentClass", "Test parent", WS_OVERLAPPEDWINDOW | WS_VISIBLE, 100, 100, 200, 200, 0, 0, 0, NULL); ok (wnd_event.hwnd != 0, "Failed to create parent window\n"); + + wnd_event.start_event = CreateEventA( NULL, TRUE, FALSE, NULL ); + wnd_event.stop_event = CreateEventA( NULL, TRUE, FALSE, NULL ); + + wnd_event.child = CreateThread( NULL, 0, create_child_thread, &wnd_event, 0, &tid ); + ok( wnd_event.child != 0, "CreateThread failed, error %u\n", GetLastError() ); + flush_events(); flush_sequence(); log_all_parent_messages++; - wnd_event.start_event = CreateEventA( NULL, TRUE, FALSE, NULL ); - wnd_event.stop_event = CreateEventA( NULL, TRUE, FALSE, NULL ); - hThread = CreateThread( NULL, 0, create_child_thread, &wnd_event, 0, &tid ); for (;;) { - ret = MsgWaitForMultipleObjects(1, &wnd_event.start_event, FALSE, 1000, QS_SENDMESSAGE); + ret = MsgWaitForMultipleObjects(1, &wnd_event.start_event, FALSE, 5000, QS_SENDMESSAGE); if (ret != 1) break; while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); } ok( !ret, "MsgWaitForMultipleObjects failed %x\n", ret ); /* now wait for the thread without processing messages; this shouldn't deadlock */ SetEvent( wnd_event.stop_event ); - ret = WaitForSingleObject( hThread, 5000 ); + + ret = WaitForSingleObject( wnd_event.child, 5000 ); ok( !ret, "WaitForSingleObject failed %x\n", ret ); - CloseHandle( hThread );
ret = WaitForSingleObject( wnd_event.grand_child, 5000 ); - ok( !ret, "WaitForSingleObject failed %x\n", ret ); - CloseHandle( wnd_event.grand_child ); + ok( !ret, "WaitForSingleObject failed %x, error: %u\n", ret, GetLastError() );
+ CloseHandle( wnd_event.child ); + CloseHandle( wnd_event.grand_child ); CloseHandle( wnd_event.start_event ); CloseHandle( wnd_event.stop_event ); flush_events(); - ok_sequence(WmExitThreadSeq, "destroy child on thread exit", FALSE); + ok_sequence(WmExitThreadSeq, "destroy child on thread exit", TRUE); log_all_parent_messages--; DestroyWindow( wnd_event.hwnd );
On thread destroy, a WM_WINE_DESTROYWINDOW is sent to the child windows living in other threads. However there's then a race condition between these threads peeking for messages and the current thread detaching its child windows from their threads and clearing their message queues.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/user32/tests/msg.c | 3 +-- server/window.c | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 55b4fe2228b..3aa8c83f64d 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8586,7 +8586,6 @@ static DWORD CALLBACK create_grand_child_thread( void *param ) ok( !ret, "WaitForSingleObject returned %x, error: %u\n", ret, GetLastError() );
while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); - todo_wine ok( !IsWindow( hchild ), "Child window not destroyed\n" );
return 0; @@ -8781,7 +8780,7 @@ static void test_interthread_messages(void) CloseHandle( wnd_event.start_event ); CloseHandle( wnd_event.stop_event ); flush_events(); - ok_sequence(WmExitThreadSeq, "destroy child on thread exit", TRUE); + ok_sequence(WmExitThreadSeq, "destroy child on thread exit", FALSE); log_all_parent_messages--; DestroyWindow( wnd_event.hwnd );
diff --git a/server/window.c b/server/window.c index c9b131cba5d..cd413ba1023 100644 --- a/server/window.c +++ b/server/window.c @@ -1888,9 +1888,21 @@ void destroy_window( struct window *win )
/* destroy all children */ while (!list_empty(&win->children)) - destroy_window( LIST_ENTRY( list_head(&win->children), struct window, entry )); + { + struct window *child = LIST_ENTRY( list_head(&win->children), struct window, entry ); + if (!child->thread || child->thread == win->thread || child->thread->state != RUNNING) + destroy_window( child ); + else + list_remove(&child->entry); + } while (!list_empty(&win->unlinked)) - destroy_window( LIST_ENTRY( list_head(&win->unlinked), struct window, entry )); + { + struct window *child = LIST_ENTRY( list_head(&win->unlinked), struct window, entry ); + if (!child->thread || child->thread == win->thread || child->thread->state != RUNNING) + destroy_window( child ); + else + list_remove(&child->entry); + }
/* reset global window pointers, if the corresponding window is destroyed */ if (win == shell_window) shell_window = NULL;
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=66334
Your paranoid android.
=== w8 (32 bit report) ===
user32: msg.c:14689: Test failed: unexpected message 31f msg.c:14690: Test failed: bad wparam 1 msg.c:14696: Test failed: unicode WM_CHAR: 0: the msg sequence is not complete: expected 0102 - actual 0000
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=66333
Your paranoid android.
=== w8adm (32 bit report) ===
user32: msg.c:17569: Test failed: wrong status 00080000
=== w1064v1809_ja (32 bit report) ===
user32: msg.c:16303: Test failed: 16: WaitForSingleObject failed msg.c:8769: Test failed: MsgWaitForMultipleObjects failed 102 msg: Timeout
=== w1064v1809_ja (testbot log) ===
The task timed out
=== w864 (64 bit report) ===
user32: msg.c:17569: Test failed: thread: call SendMessage
-
Marvin -
Rémi Bernon