libpng is currently loaded on the first attempt to access a PNG icon or cursor. That creates a race condition that can deadlock the loader in certain circumstances if LoadIcon or LoadCursor is called while while the loader lock is held, e.g. from a DllMain.
Signed-off-by: Tim Clem tclem@codeweavers.com --- It's possible for one thread to win the INIT_ONCE to load libpng without owning the loader lock. If the thread with the loader lock happens to try to load libpng shortly thereafter, they deadlock.
All of Wine's built-in icon resources are PNGs, so we almost always end up loading libpng anyway. Doing it up-front in user32's DllMain ensures that it's done while the loader lock is held and prevents the issue.
dlls/user32/cursoricon.c | 21 +++------------------ dlls/user32/png.c | 4 ++-- dlls/user32/user_main.c | 6 +++++- dlls/user32/user_private.h | 3 +++ 4 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/dlls/user32/cursoricon.c b/dlls/user32/cursoricon.c index 7ad0a04a551..dc831db9287 100644 --- a/dlls/user32/cursoricon.c +++ b/dlls/user32/cursoricon.c @@ -111,21 +111,6 @@ static int get_display_bpp(void) return ret; }
-static INIT_ONCE init_once = INIT_ONCE_STATIC_INIT; - -static const struct png_funcs *png_funcs; - -static BOOL WINAPI load_libpng( INIT_ONCE *once, void *param, void **context ) -{ - __wine_init_unix_lib( user32_module, DLL_PROCESS_ATTACH, NULL, &png_funcs ); - return TRUE; -} - -static BOOL have_libpng(void) -{ - return InitOnceExecuteOnce( &init_once, load_libpng, NULL, NULL ) && png_funcs; -} - static HICON alloc_icon_handle( BOOL is_ani, UINT num_steps ) { struct cursoricon_object *obj; @@ -571,7 +556,7 @@ static BOOL CURSORICON_GetResIconEntry( LPCVOID dir, DWORD size, int n, *width = icon->bWidth; *height = icon->bHeight; *bits = resdir->idEntries[n].wBitCount; - if (!*width && !*height && have_libpng()) *width = *height = 256; + if (!*width && !*height && png_funcs) *width = *height = 256; return TRUE; }
@@ -704,7 +689,7 @@ static BOOL CURSORICON_GetFileEntry( LPCVOID dir, DWORD size, int n,
if (info->biSize == PNG_SIGN) { - if (have_libpng()) return png_funcs->get_png_info(info, size, width, height, bits); + if (png_funcs) return png_funcs->get_png_info(info, size, width, height, bits); *width = *height = *bits = 0; return TRUE; } @@ -858,7 +843,7 @@ static HICON create_icon_from_bmi( const BITMAPINFO *bmi, DWORD maxsize, HMODULE { BITMAPINFO *bmi_png;
- if (!have_libpng()) return 0; + if (!png_funcs) return 0; bmi_png = png_funcs->load_png( (const char *)bmi, &maxsize ); if (bmi_png) { diff --git a/dlls/user32/png.c b/dlls/user32/png.c index a994c6de5c2..bcd619fe52b 100644 --- a/dlls/user32/png.c +++ b/dlls/user32/png.c @@ -293,7 +293,7 @@ static BITMAPINFO * CDECL load_png(const char *png_data, DWORD *size) return info; }
-static const struct png_funcs png_funcs = +static const struct png_funcs funcs = { get_png_info, load_png @@ -335,7 +335,7 @@ NTSTATUS CDECL __wine_init_unix_lib( HMODULE module, DWORD reason, const void *p LOAD_FUNCPTR(png_set_read_fn); #undef LOAD_FUNCPTR
- *(const struct png_funcs **)ptr_out = &png_funcs; + *(const struct png_funcs **)ptr_out = &funcs; return STATUS_SUCCESS; }
diff --git a/dlls/user32/user_main.c b/dlls/user32/user_main.c index 303f19accfc..882a0c5237b 100644 --- a/dlls/user32/user_main.c +++ b/dlls/user32/user_main.c @@ -52,6 +52,8 @@ static HPALETTE hPrimaryPalette;
static DWORD exiting_thread_id;
+const struct png_funcs *png_funcs = NULL; + extern void WDML_NotifyThreadDetach(void);
#ifdef __MINGW32__ @@ -382,8 +384,10 @@ BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, LPVOID reserved ) case DLL_PROCESS_ATTACH: user32_module = inst; ret = process_attach(); - if(ret) + if(ret) { imm32_module = LoadLibraryW(L"imm32.dll"); + __wine_init_unix_lib( user32_module, DLL_PROCESS_ATTACH, NULL, &png_funcs ); + } break; case DLL_THREAD_DETACH: thread_detach(); diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index 7c4d9015976..0be02919bbe 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -380,6 +380,9 @@ struct png_funcs BITMAPINFO * (CDECL *load_png)(const char *png_data, DWORD *size); };
+/* May be NULL if libpng cannot be loaded. */ +extern const struct png_funcs *png_funcs DECLSPEC_HIDDEN; + /* Mingw's assert() imports MessageBoxA and gets confused by user32 exporting it */ #ifdef __MINGW32__ #undef assert
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=96421
Your paranoid android.
=== debiant2 (32 bit Chinese:China report) ===
user32: win.c:10383: Test failed: Expected foreground window 00150128, got 00E400F2 win.c:10385: Test failed: GetActiveWindow() = 00000000 win.c:10385: Test failed: GetFocus() = 00000000 win.c:10386: Test failed: Received WM_ACTIVATEAPP(1), did not expect it. win.c:10387: Test failed: Received WM_ACTIVATEAPP(0), did not expect it. win.c:10395: Test failed: Expected foreground window 00150128, got 00000000 win.c:10397: Test failed: GetActiveWindow() = 00000000 win.c:10397: Test failed: GetFocus() = 00000000 win.c:10405: Test failed: Received WM_ACTIVATEAPP(1), did not expect it.
-
Marvin -
Tim Clem