https://bugs.winehq.org/show_bug.cgi?id=52207
Bug ID: 52207 Summary: HICON leak in CopyImage Product: Wine Version: 6.21 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: user32 Assignee: wine-bugs@winehq.org Reporter: jswinebz@kanargh.org.uk Distribution: ---
Created attachment 71264 --> https://bugs.winehq.org/attachment.cgi?id=71264 Don't leak HICONs from CopyImage
At some point between 6.7 and 6.21, user32.CopyImage was reworked.
In the IMAGE_ICON, LR_COPYFROMRESOURCE case it used to just call CURSORICON_Load and return its return value.
Now it calls CURSORICON_Load, makes *another* copy of that (in case it wants to resize it, but it still creates another copy if it's already the right size), then returns the copy. The original intermediate HICON is leaked. This also leaks its two underlying GDI HBITMAPs. Since there are two HBITMAPs for each HICON, we run out of GDI handles first.
(I run The Bat! as my email client, which animates its systray icon when there is unread mail. The leaking HICONs cause it to crash after just under 2 hours, which is roughly what 5fps at two handles per frame takes to hit 65536.)
The attached patch fixes that by fudging LR_COPYDELETEORG into the flags for that case, causing the "original" (actually our intermediate) icon to be deleted on exit...
...except there's a second bug there. As it stands it tries to use gdi32.DeleteObject to delete the HICON. But DeleteObject can only delete GDI handles (for example the two embedded HBITMAPs within the HICON), but the HICON itself is a user object and the delete fails.
So the DeleteObject call must also be changed to DestroyIcon to make that work.
With both of those changes in, the GDI handle count is now stable.
https://bugs.winehq.org/show_bug.cgi?id=52207
jswinebz@kanargh.org.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch
https://bugs.winehq.org/show_bug.cgi?id=52207
Gijs Vermeulen gijsvrm@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|HICON leak in CopyImage |HICON leak in CopyImage | |causes TheBat! to crash | |after a while CC| |z.figura12@gmail.com Regression SHA1| |beb70a79e1d92ddb10a552dcc6c | |06a4139365566 Version|6.21 |6.10 Keywords| |regression
--- Comment #1 from Gijs Vermeulen gijsvrm@gmail.com --- Thanks for the detailed bug report. Patches aren't picked up from bugzilla, so if you would like to formally submit your patches, have a look at https://wiki.winehq.org/Submitting_Patches.
This seems to be a regression caused by https://source.winehq.org/git/wine.git/commit/beb70a79e1d92ddb10a552dcc6c06a4139365566, CC'ing Zebediah.
https://bugs.winehq.org/show_bug.cgi?id=52207
--- Comment #2 from Zebediah Figura z.figura12@gmail.com --- If you prefer I can also submit the patches on your behalf, although in that case can you please provide your name?
https://bugs.winehq.org/show_bug.cgi?id=52207
--- Comment #3 from jswinebz@kanargh.org.uk --- If you would, please. John Sullivan.
https://bugs.winehq.org/show_bug.cgi?id=52207
--- Comment #4 from jswinebz@kanargh.org.uk --- (btw, I'm assuming that passing in LR_COPYDELETEORG and LR_COPYFROMRESOURCE to the original call is nonsensical - according to docs LR_CFR should only be used on LR_SHARED icons, and those should *not* be passed to DestroyIcon at all so we should never see that. If anything, an attempt to do that should be caught and either generate its own kind of error or be bypassed and do nothing...)
https://bugs.winehq.org/show_bug.cgi?id=52207
--- Comment #5 from Zebediah Figura z.figura12@gmail.com --- (In reply to jswinebz from comment #4)
(btw, I'm assuming that passing in LR_COPYDELETEORG and LR_COPYFROMRESOURCE to the original call is nonsensical - according to docs LR_CFR should only be used on LR_SHARED icons, and those should *not* be passed to DestroyIcon at all so we should never see that. If anything, an attempt to do that should be caught and either generate its own kind of error or be bypassed and do nothing...)
It's a bit of a moot point, because LR_COPYDELETEORG only deletes the original on success, whereas we also need to delete ours on failure. So that part will have to be changed anyway.
https://bugs.winehq.org/show_bug.cgi?id=52207
Zebediah Figura z.figura12@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |97bf2e7e1edcee9600badf4b708 | |bfe2463d2c28a Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED
--- Comment #6 from Zebediah Figura z.figura12@gmail.com --- Should be fixed upstream by https://source.winehq.org/git/wine.git/commitdiff/4a200821672dfe719dc5b20df853e719678496b7 and https://source.winehq.org/git/wine.git/commitdiff/97bf2e7e1edcee9600badf4b708bfe2463d2c28a.
https://bugs.winehq.org/show_bug.cgi?id=52207
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #7 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 7.0-rc2.
-
WineHQ Bugzilla