[PATCH] user32: During destroy, don't send WM_ERASEBKGND to parent window if WS_CHILD is not set - wine-devel

15 Jul 2021

Prior to this change, Wine would hang during the DestroyWindow(child)
call because it is waiting for the parent window thread to process a
WM_ERASEBKGND message.
Signed-off-by: Andrew Eikum aeikum@codeweavers.com
---
 dlls/user32/tests/msg.c | 130 ++++++++++++++++++++++++++++++++++++++++
 dlls/user32/winpos.c    |   6 +-
 2 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index 9867e319d58..a3d30dd005b 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -11374,6 +11374,20 @@ static const struct message destroy_window_with_children[] = {
     { 0 }
 };
+static const struct message destroy_child_window_with_ws_child[] = {
+    { HCBT_DESTROYWND, hook },
+    { WM_PARENTNOTIFY, sent },
+    { WM_ERASEBKGND, sent },
+    { 0 }
+};
+
+static const struct message destroy_child_window_no_ws_child[] = {
+    { HCBT_DESTROYWND, hook },
+    { WM_PAINT, sent },
+    { WM_ERASEBKGND, sent|beginpaint },
+    { 0 }
+};
+
 static void test_DestroyWindow(void)
 {
     BOOL ret;
@@ -11478,6 +11492,121 @@ static void test_DestroyWindow(void)
     ok(!test, "wrong capture window %p\n", test);
 }
+struct destroy_child_window_test_data {
+    HANDLE evt;
+    HWND parent;
+    BOOL stop_processing;
+    BOOL quit;
+};
+
+static DWORD WINAPI destroy_child_window_parent_threadproc(void *user)
+{
+    struct destroy_child_window_test_data *data = user;
+
+    data->parent = CreateWindowA("TestWindowClass", "test parent",
+            WS_VISIBLE | WS_OVERLAPPEDWINDOW, 100, 200, 300, 400,
+            NULL, NULL, NULL, NULL);
+
+    SetEvent(data->evt);
+
+    while(!data->stop_processing)
+        flush_events();
+
+    SetEvent(data->evt);
+
+    while(data->stop_processing)
+        Sleep(10);
+
+    flush_events();
+
+    SetEvent(data->evt);
+
+    while(!data->quit)
+        Sleep(10);
+
+    DestroyWindow(data->parent);
+
+    flush_events();
+
+    return 0;
+}
+
+static void test_destroy_child_window(void)
+{
+    struct destroy_child_window_test_data data = {0};
+    HWND child;
+    HANDLE parent_thread;
+
+    /* create parent on this thread to demonstrate WM_ERASEBKGND message */
+    data.parent = CreateWindowA("TestWindowClass", "test parent",
+            WS_VISIBLE | WS_OVERLAPPEDWINDOW, 100, 200, 300, 400,
+            NULL, NULL, NULL, NULL);
+    ok(data.parent != NULL, "parent window create failed\n");
+
+    child = CreateWindowA("SimpleWindowClass", "test child",
+            WS_OVERLAPPEDWINDOW | WS_CLIPSIBLINGS | WS_CLIPCHILDREN,
+            100, 200, 50, 50, NULL, NULL, NULL, NULL);
+    ok(child != NULL, "child window create failed\n");
+
+    SetWindowLongPtrW(child, GWL_STYLE, GetWindowLongPtrW(child, GWL_STYLE) | WS_CHILD);
+    SetParent(child, data.parent);
+
+    ShowWindow(child, SW_SHOW);
+    ShowWindow(data.parent, SW_SHOW);
+
+    flush_events();
+    flush_sequence();
+
+    DestroyWindow(child);
+
+    ok_sequence(destroy_child_window_with_ws_child, "destroy child with WS_CHILD", FALSE);
+
+    DestroyWindow(data.parent);
+
+    /* create parent on separate thread to demonstrate avoiding deadlock on WM_ERASEBKGND message */
+    data.evt = CreateEventW(NULL, FALSE, FALSE, NULL);
+
+    parent_thread = CreateThread(NULL, 0, destroy_child_window_parent_threadproc, &data, 0, NULL);
+
+    WaitForSingleObject(data.evt, INFINITE);
+    ok(data.parent != NULL, "parent window create failed\n");
+
+    child = CreateWindowA("SimpleWindowClass", "test child",
+            WS_OVERLAPPEDWINDOW | WS_CLIPSIBLINGS | WS_CLIPCHILDREN,
+            100, 200, 50, 50, NULL, NULL, NULL, NULL);
+    ok(child != NULL, "child window create failed\n");
+
+    SetWindowLongPtrW(child, GWL_STYLE, GetWindowLongPtrW(child, GWL_STYLE) | WS_CHILD);
+    SetParent(child, data.parent);
+
+    ShowWindow(child, SW_SHOW);
+    ShowWindow(data.parent, SW_SHOW);
+
+    /* removing WS_CHILD avoids deadlock on WM_PARENTNOTIFY */
+    SetWindowLongPtrW(child, GWL_STYLE, GetWindowLongPtrW(child, GWL_STYLE) & ~WS_CHILD);
+
+    data.stop_processing = TRUE;
+    WaitForSingleObject(data.evt, INFINITE);
+
+    flush_events();
+    flush_sequence();
+
+    DestroyWindow(child);
+
+    data.stop_processing = FALSE;
+    WaitForSingleObject(data.evt, INFINITE);
+
+    ok_sequence(destroy_child_window_no_ws_child, "destroy child without WS_CHILD", FALSE);
+
+    data.quit = TRUE;
+    WaitForSingleObject(parent_thread, INFINITE);
+
+    flush_events();
+    flush_sequence();
+
+    CloseHandle(data.evt);
+    CloseHandle(parent_thread);
+}
static const struct message WmDispatchPaint[] = {
     { WM_NCPAINT, sent },
@@ -18333,6 +18462,7 @@ START_TEST(msg)
         test_recursive_hook();
     }
     test_DestroyWindow();
+    test_destroy_child_window();
     test_DispatchMessage();
     test_SendMessageTimeout();
     test_edit_messages();
diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c
index 6e96a4b5964..d047d714e79 100644
--- a/dlls/user32/winpos.c
+++ b/dlls/user32/winpos.c
@@ -2367,8 +2367,10 @@ BOOL USER_SetWindowPos( WINDOWPOS * winpos, int parent_x, int parent_y )
           (winpos->flags & SWP_AGG_STATUSFLAGS) != SWP_AGG_NOGEOMETRYCHANGE))
         {
             HWND parent = GetAncestor( winpos->hwnd, GA_PARENT );
-            if (!parent || parent == GetDesktopWindow()) parent = winpos->hwnd;
-            erase_now( parent, 0 );
+            if (!parent || parent == GetDesktopWindow())
+                erase_now( winpos->hwnd, 0 );
+            else if((GetWindowLongW(winpos->hwnd, GWL_STYLE) & WS_CHILD) == WS_CHILD)
+                erase_now( parent, 0 );
         }
/* Give newly shown windows a chance to redraw */
-- 
2.32.0



    