[PATCH] winex11.drv: Treat invalid icon as no icon in fetch_icon_data().

Paul Gofman pgofman at codeweavers.com
Fri Apr 22 10:53:56 CDT 2022


Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
---
     Fixes Warhammer: End Times launcher lockup on start (semi-random).

     The launcher creates layered window and shows it in the main thread (while it however not yet shown as
     neither UpdateLayeredWindow nor SetLayeredWindowAttributes is called). Also, the main thread passes some
     probably not initilized icon to WM_SETICON after it fails to load the icon from application exe (which seems
     to be absent there). Sometimes this value is 0 (and in that case there is no issues) but sometimes it is
     some semi-random value which is not a valid icon handle.
     Then, another thread is doing UpdateLayeredWindow() while the main thread is waiting on event to be set by the
     UpdateLayeredWindow thread after. In the course of that, X11DRV_WindowPosChanged() calls fetch_icon_data()
     which deadlocks in SendMessage(WM_GETICON) as the main thread which is supposed to process window message through
     the queue is waiting for an event and does not process messages.
     It actually already tried to fetch the icon during showing the window from the main thread, but if the obtained
     window handle is invalid the pixmap data is NULL and the current logic in winex11.drv will be retrying fetching
     the icon handle with WM_GETICON each time it gets to X11DRV_WindowPosChanged().
     I have made a separate program which reproduces the case in a short example:
     https://gist.github.com/gofman/befcc1dc8b73d751fac32783f626ea1b
     This case doesn't deadlock on windows but currently deadlock under Wine. Also, I noticed that (when that is
     a normal, not a layered window) Wine expectedly won't show the default icon for the window if invalid handle was
     provided while Windows seems to show it.

     https://gist.github.com/gofman/befcc1dc8b73d751fac32783f626ea1b
 dlls/winex11.drv/window.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c
index 48500284b68..4cf771a998f 100644
--- a/dlls/winex11.drv/window.c
+++ b/dlls/winex11.drv/window.c
@@ -598,6 +598,11 @@ failed:
 }
 
 
+static HICON get_icon_info( HICON icon, ICONINFO *ii )
+{
+    return icon && NtUserGetIconInfo( icon, ii, NULL, NULL, NULL, 0 ) ? icon : NULL;
+}
+
 /***********************************************************************
  *              fetch_icon_data
  */
@@ -612,21 +617,24 @@ static void fetch_icon_data( HWND hwnd, HICON icon_big, HICON icon_small )
 
     if (!icon_big)
     {
-        icon_big = (HICON)send_message( hwnd, WM_GETICON, ICON_BIG, 0 );
-        if (!icon_big) icon_big = (HICON)NtUserGetClassLongPtrW( hwnd, GCLP_HICON );
-        if (!icon_big) icon_big = LoadIconW( 0, (LPWSTR)IDI_WINLOGO );
+        icon_big = get_icon_info( (HICON)send_message( hwnd, WM_GETICON, ICON_BIG, 0 ), &ii );
+        if (!icon_big)
+            icon_big = get_icon_info( (HICON)NtUserGetClassLongPtrW( hwnd, GCLP_HICON ), &ii );
+        if (!icon_big)
+            icon_big = get_icon_info( LoadIconW( 0, (LPWSTR)IDI_WINLOGO ), &ii);
     }
     if (!icon_small)
     {
-        icon_small = (HICON)send_message( hwnd, WM_GETICON, ICON_SMALL, 0 );
-        if (!icon_small) icon_small = (HICON)NtUserGetClassLongPtrW( hwnd, GCLP_HICONSM );
+        icon_small = get_icon_info( (HICON)send_message( hwnd, WM_GETICON, ICON_SMALL, 0 ), &ii_small );
+        if (!icon_small)
+            icon_small = get_icon_info( (HICON)NtUserGetClassLongPtrW( hwnd, GCLP_HICONSM ), &ii_small );
     }
 
-    if (!NtUserGetIconInfo( icon_big, &ii, NULL, NULL, NULL, 0 )) return;
+    if (!icon_big) return;
 
     hDC = NtGdiCreateCompatibleDC(0);
     bits = get_bitmap_argb( hDC, ii.hbmColor, ii.hbmMask, &size );
-    if (bits && NtUserGetIconInfo( icon_small, &ii_small, NULL, NULL, NULL, 0 ))
+    if (bits && icon_small)
     {
         unsigned int size_small;
         unsigned long *bits_small, *new;
-- 
2.35.1




More information about the wine-devel mailing list