[Bug 53014] HICON leak in CopyImage causes TheBat! to crash after a while

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53014

--- Comment #5 from jswinebz at kanargh.org.uk ---
Ok, think I've nailed the deadlock.

When bits of user32's thread detach were moved into win32u, including the
destroy_thread_windows() function, that function gained a call to user_lock().

The Bat! can animate both its systray icon, and the taskbar icon. The latter
doesn't actually work any more for me, I believe due to gtk deciding that
people shouldn't have access to that sort of UI decoration any more, but the
Win32 side of things is still handled.

To animate the taskbar icon it ultimately sets the window class word containing
the icon handle, which ends up in user32:class.c:set_class_long, case
GCLP_HICON, itself under user_lock. This calls CopyImage, which can call either
GetModuleHandleW or LdrGetDllFullName to access the icon resource information,
both of which take the process lock (loader_section).

On the other side a thread is in DllMain having already taken the process lock,
which blocks the thread calling CopyImage, then tries to call
destroy_thread_windows(), deadlocking on user_lock held by the thread calling
CopyImage.

So there I'm stuck: someone clearly thought adding user_lock() to
destroy_thread_windows() was necessary, but taking additional locks within
DllMain is a highly dangerous act and always has been. That lock must be
removed again from destroy_thread_windows(), but that can't necessarily be done
simply without addressing whatever other thing it was there to protect.

(There might be nothing else required and the addition was mere paranoia, but
that requires bit more understanding of wine internals than I have.)

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.