[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 #1 from jswinebz at kanargh.org.uk ---
Ok, I notice this difference between my previous patched 6.21 and the current
7.5.

In user32/cursoricon.c CopyImage we used to have:

            if (!(icon = get_icon_ptr( hnd ))) return 0;

            if (icon->rsrc && (flags & LR_COPYFROMRESOURCE))
            {
                hnd = CURSORICON_Load( icon->module, icon->resname, desiredx,
desiredy, depth,
                                       !icon->is_icon, flags );
                release_user_handle_ptr( icon );
                if (!(icon = get_icon_ptr( hnd ))) return 0;
            }

Now in 7.5 we have:

            if (!GetIconInfoExW( hnd, &icon_info )) return 0;

            if (icon_info.szModName[0] && (flags & LR_COPYFROMRESOURCE) &&
                (module = GetModuleHandleW( icon_info.szModName )))
            {
                const WCHAR *res = icon_info.szResName[0] ? icon_info.szResName
                    : MAKEINTRESOURCEW( icon_info.wResID );
                resource_icon = CURSORICON_Load( module, res, desiredx,
desiredy, depth,
                                                 !icon_info.fIcon, flags );
                DeleteObject( icon_info.hbmColor );
                DeleteObject( icon_info.hbmMask );
                NtUserGetIconSize( resource_icon, 0, &width, &height );
                if (!GetIconInfoExW( resource_icon, &icon_info )) return 0;
            }

If I add additional TRACE output then I get:

             if (!GetIconInfoExW( hnd, &icon_info )) return 0;
+            TRACE("GetIconInfoExW -> mask=%p color=%p modname='%s'\n",
icon_info.hbmMask, icon_info.hbmColor, debugstr_w(icon_info.szModName));
+            if (icon_info.szModName[0] && (flags & LR_COPYFROMRESOURCE)) {
+              module = GetModuleHandleW( icon_info.szModName );
+              TRACE("module=%p\n", module);
+            }

0124:trace:cursor:CopyImage CopyImage: hnd=0000000000010178, type=1,
desiredx=16, desiredy=16, flags=4000
...
0124:trace:cursor:CopyImage GetIconInfoExW -> mask=000000003B090583
color=00000000A509043F modname='L"C:\\Prog"'
0124:trace:cursor:CopyImage module=0000000000000000

So immediately we can see that it *looks* like szModName is truncated[*], and
that GetModuleHandle is failing to find the HMODULE for that dll. (And we can
infer from the first four letters of the path that it is likely one of the
application's own dlls containing its resources...) As such we'll muddle our
way through the non-COPYFROMRESOURCE path. I haven't figured out the entire
consequences of that yet, but it clearly wasn't what was intended...

[*] (and for this call it always appears to be truncated to either 7-chars/an
8-char buffer or 20-chars/a 21-char buffer. There are the occasional calls for
icons from "C:\\windows\\system32\\user32.dll" which log the complete path
correctly. It could be that dbgstr_w is running out of temp-buffer and
truncating, but since we are then failing the GetModuleHandle call I assume
that the underlying data actually is the problem.)

Anyway, we carry on doing that for a while, and the systray-icon *does* display
and animate, but after half an hour we get to:

0124:trace:cursor:CopyImage GetIconInfoExW -> mask=000000002209024B
color=00000000230900CD modname='L"C:\\Prog"'
0124:err:sync:RtlpWaitForCriticalSection section 0000000170067640
"dlls/ntdll/loader.c: loader_section" wait timed out in thread 0124, blocked by
020c, retrying (60 sec)
0210:err:sync:RtlpWaitForCriticalSection section 0000000170067640
"dlls/ntdll/loader.c: loader_section" wait timed out in thread 0210, blocked by
020c, retrying (60 sec)

The first TRACE always happens, but we often don't even get the "wait timed
out" TRACEs after it. However we never get to the "module=" TRACE line so it
looks like we've deadlocked there somewhere within GetModuleHandle...

I'll further note that since my original patch, which appeared affective, was
triggered within the original if(COPYFROMRESOURCE) statement in 6.21, that
icon->rsrc *must* have been non-NULL and we *must* have been hitting the
CURSORICON_Load path in that case (without, in that version, any need to call
GetModuleHandle or look at szModName).

-- 
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.