[PATCH 1/2] user32/tests: Add a delay to consistently trigger a race condition.
This test is failing from time to time. Adding a delay here triggers the underlying race condition -see next patch- and makes the test to always fail. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- Notes: The failure was hard to reproduce. The most reliable way without the sleep added here is to start multiple runs on the testbot, changing locale for each run. The locale change seems to have an impact on the timing and triggers the race condition. dlls/user32/tests/msg.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 193a20fd958..0b756050e47 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8588,6 +8588,9 @@ static DWORD CALLBACK create_grand_child_thread( void *param ) flush_sequence(); SetEvent( wnd_event->start_event ); + /* be sure to wait for the child thread to be destroyed */ + Sleep(1000); + for (;;) { MsgWaitForMultipleObjects(0, NULL, FALSE, 1000, QS_ALLINPUT); @@ -8771,13 +8774,14 @@ static void test_interthread_messages(void) CloseHandle( hThread ); ret = WaitForSingleObject( wnd_event.grand_child, 5000 ); + todo_wine ok( !ret, "WaitForSingleObject failed %x\n", ret ); 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 ); -- 2.24.0.rc2
On thread destroy, a WM_WINE_DESTROYWINDOW is sent for the children windows owned by 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 owning threads -which clears their message queues. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/tests/msg.c | 3 +-- server/window.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 0b756050e47..e61c0472213 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8774,14 +8774,13 @@ static void test_interthread_messages(void) CloseHandle( hThread ); ret = WaitForSingleObject( wnd_event.grand_child, 5000 ); - todo_wine ok( !ret, "WaitForSingleObject failed %x\n", ret ); 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", 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..3985ea4eb20 100644 --- a/server/window.c +++ b/server/window.c @@ -1914,7 +1914,7 @@ void destroy_window( struct window *win ) post_message( win->parent->handle, WM_PARENTNOTIFY, WM_DESTROY, win->handle ); } - detach_window_thread( win ); + if (win->thread == current) detach_window_thread( win ); if (win->win_region) free_region( win->win_region ); if (win->update_region) free_region( win->update_region ); if (win->class) release_class( win->class ); -- 2.24.0.rc2
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=59522 Your paranoid android. === w1064v1809 (32 bit report) === user32: msg.c:10350: Test failed: did not get expected count for minimum timeout (53 != ~100).
Rémi Bernon <rbernon(a)codeweavers.com> writes:
This test is failing from time to time. Adding a delay here triggers the underlying race condition -see next patch- and makes the test to always fail.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
Notes: The failure was hard to reproduce. The most reliable way without the sleep added here is to start multiple runs on the testbot, changing locale for each run. The locale change seems to have an impact on the timing and triggers the race condition.
dlls/user32/tests/msg.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 193a20fd958..0b756050e47 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -8588,6 +8588,9 @@ static DWORD CALLBACK create_grand_child_thread( void *param ) flush_sequence(); SetEvent( wnd_event->start_event );
+ /* be sure to wait for the child thread to be destroyed */ + Sleep(1000);
Please don't add sleeps, the tests are already slow enough. That sort of thing should be handled by using a proper synchronization object. -- Alexandre Julliard julliard(a)winehq.org
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=59521 Your paranoid android. === wvistau64 (32 bit report) === user32: msg.c:16280: Test failed: 0: WaitForSingleObject failed === w8adm (32 bit report) === user32: msg.c:12933: Test failed: WmMouseHoverSeq: 3: the msg 0x0118 was expected, but got msg 0x001a instead msg.c:12933: Test failed: WmMouseHoverSeq: 4: the msg 0x02a1 was expected, but got msg 0x001a instead msg.c:12933: Test failed: WmMouseHoverSeq: 5: the msg sequence is not complete: expected 0000 - actual 0118 === w1064v1809 (32 bit report) === user32: msg.c:10281: Test failed: did not get expected count for minimum timeout (54 != ~100). msg.c:10351: Test failed: did not get expected count for minimum timeout (53 != ~100). === w1064v1809_ar (32 bit report) === user32: msg.c:17771: Test failed: Restore minimized window: 41: the msg sequence is not complete: expected 0000 - actual 0007 === w1064v1809_he (32 bit report) === user32: msg.c:11878: Test failed: wrong qstatus 00490048 msg.c:11960: Test failed: got 0 and 0000 instead of TRUE and WM_PAINT msg.c:11963: Test failed: WmPaint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:11966: Test failed: wrong qstatus 00210000 msg.c:11977: Test failed: wrong qstatus 00210000 msg.c:11985: Test failed: wrong qstatus 00610040 msg.c:11991: Test failed: wrong qstatus 00690008 msg.c:12002: Test failed: wrong qstatus 00210000 msg.c:12010: Test failed: WmEmptySeq: 0: the msg sequence is not complete: expected 0000 - actual 0024 msg.c:12013: Test failed: wrong qstatus 00210000 msg.c:12019: Test failed: wrong qstatus 00290008 msg.c:12027: Test failed: wrong qstatus 00690040 msg.c:12038: Test failed: wrong qstatus 00290000 msg.c:12052: Test failed: wrong qstatus 00290000 msg.c:12066: Test failed: wrong qstatus 00280000 msg.c:12077: Test failed: wrong qstatus 00280000 msg.c:12088: Test failed: wrong qstatus 00200000 msg.c:12093: Test failed: PeekMessageA should have returned FALSE instead of msg 000f msg.c:12099: Test failed: wrong qstatus 00200000 msg.c:12108: Test failed: wrong qstatus 00280008 msg.c:12114: Test failed: wrong qstatus 00280008 msg.c:12124: Test failed: wrong qstatus 00280000 msg.c:12137: Test failed: wrong qstatus 00280000 msg.c:12143: Test failed: PeekMessageA should have returned FALSE instead of msg 000f msg.c:12149: Test failed: wrong qstatus 00200000 msg.c:12156: Test failed: wrong qstatus 00210001 msg.c:12160: Test failed: wrong qstatus 00290008 msg.c:12169: Test failed: wrong qstatus 00210000 msg.c:12180: Test failed: wrong qstatus 00200000 msg.c:12185: Test failed: wrong qstatus 00210001 msg.c:12189: Test failed: wrong qstatus 00290008 msg.c:12199: Test failed: wrong qstatus 00280000 msg.c:12209: Test failed: wrong qstatus 00200000 msg.c:12214: Test failed: wrong qstatus 00210001 msg.c:12218: Test failed: wrong qstatus 00290008 msg.c:12224: Test failed: wrong qstatus 00690040 msg.c:12235: Test failed: wrong qstatus 00280000 msg.c:12245: Test failed: wrong qstatus 00200000 === w1064v1809_ja (32 bit report) === user32: msg.c:16280: Test failed: 4: WaitForSingleObject failed
participants (3)
-
Alexandre Julliard -
Marvin -
Rémi Bernon