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 ---
Supersedes: 205961-205963
v2: * Wait for thread handle to be set before waiting on it in the test.
* Don't check window thread state, TERMINATED threads will have detached all their windows already.
dlls/user32/tests/msg.c | 52 +++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 13e085dfacc..462053a951d 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8791,7 +8791,10 @@ static void test_paint_messages(void) struct wnd_event { HWND hwnd; + HWND child_hwnd; + HANDLE child; HANDLE grand_child; + HANDLE ready_event; HANDLE start_event; HANDLE stop_event; }; @@ -8822,42 +8825,48 @@ 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(); + + /* wait for wnd_event->child to be set */ + ret = WaitForSingleObject( wnd_event->ready_event, 1000 ); + ok( !ret, "WaitForSingleObject failed %x\n", ret ); 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); - } + /* wait for parent window thread to exit */ + ret = WaitForSingleObject( wnd_event->child, 1000 ); + ok( !ret, "WaitForSingleObject returned %x, error: %u\n", ret, GetLastError() ); + ok( IsWindow( hchild ), "Child window already destroyed\n" ); + flush_events(); + 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); + DWORD ret = MsgWaitForMultipleObjects(1, &wnd_event->start_event, FALSE, 1000, QS_SENDMESSAGE); if (ret != 1) break; while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) DispatchMessageA(&msg); } @@ -8997,9 +9006,11 @@ static void test_interthread_messages(void) flush_events(); flush_sequence(); log_all_parent_messages++; + wnd_event.ready_event = CreateEventA( NULL, TRUE, FALSE, NULL ); 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 ); + wnd_event.child = CreateThread( NULL, 0, create_child_thread, &wnd_event, 0, &tid ); + SetEvent( wnd_event.ready_event ); for (;;) { ret = MsgWaitForMultipleObjects(1, &wnd_event.start_event, FALSE, 1000, QS_SENDMESSAGE); @@ -9009,9 +9020,9 @@ static void test_interthread_messages(void) 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 ); + CloseHandle( wnd_event.child );
ret = WaitForSingleObject( wnd_event.grand_child, 5000 ); ok( !ret, "WaitForSingleObject failed %x\n", ret ); @@ -9019,8 +9030,9 @@ static void test_interthread_messages(void)
CloseHandle( wnd_event.start_event ); CloseHandle( wnd_event.stop_event ); + CloseHandle( wnd_event.ready_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.
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, and the message may never be received from these threads and the windows kept alive.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/user32/tests/msg.c | 4 ++-- server/window.c | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 462053a951d..6469bc48ba8 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8843,7 +8843,7 @@ static DWORD CALLBACK create_grand_child_thread( void *param ) ok( !ret, "WaitForSingleObject returned %x, error: %u\n", ret, GetLastError() ); ok( IsWindow( hchild ), "Child window already destroyed\n" ); flush_events(); - todo_wine ok( !IsWindow( hchild ), "Child window not destroyed\n" ); + ok( !IsWindow( hchild ), "Child window not destroyed\n" );
return 0; } @@ -9032,7 +9032,7 @@ static void test_interthread_messages(void) CloseHandle( wnd_event.stop_event ); CloseHandle( wnd_event.ready_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 ac12912db3a..026d54e6d32 100644 --- a/server/window.c +++ b/server/window.c @@ -1890,9 +1890,17 @@ 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) 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) 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=90706
Your paranoid android.
=== w864 (32 bit report) ===
user32: msg.c:8843: Test failed: WaitForSingleObject returned ffffffff, error: 6
=== w1064_tsign (32 bit report) ===
user32: msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 24: the msg 0x0085 was expected, but got msg 0x0047 instead msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 25: the msg sequence is not complete: expected 0014 - actual 0000
=== w864 (64 bit report) ===
user32: msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 24: the msg 0x0085 was expected, but got msg 0x0047 instead msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 25: the msg sequence is not complete: expected 0014 - actual 0000
=== w1064_tsign (64 bit report) ===
user32: msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 24: the msg 0x0085 was expected, but got msg 0x0047 instead msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 25: the msg sequence is not complete: expected 0014 - 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=90705
Your paranoid android.
=== w8adm (32 bit report) ===
user32: msg.c:8843: Test failed: WaitForSingleObject returned ffffffff, error: 6
=== w10pro64_ar (64 bit report) ===
user32: msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 24: the msg 0x0085 was expected, but got msg 0x0047 instead msg.c:5564: Test failed: RedrawWindow:show_popup_extreme_location: 25: the msg sequence is not complete: expected 0014 - actual 0000
-
Marvin -
Rémi Bernon