[Bug 52207] New: HICON leak in CopyImage
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(a)winehq.org Reporter: jswinebz(a)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. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 jswinebz(a)kanargh.org.uk changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 Gijs Vermeulen <gijsvrm(a)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(a)gmail.com Regression SHA1| |beb70a79e1d92ddb10a552dcc6c | |06a4139365566 Version|6.21 |6.10 Keywords| |regression --- Comment #1 from Gijs Vermeulen <gijsvrm(a)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. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 --- Comment #2 from Zebediah Figura <z.figura12(a)gmail.com> --- If you prefer I can also submit the patches on your behalf, although in that case can you please provide your name? -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 --- Comment #3 from jswinebz(a)kanargh.org.uk --- If you would, please. John Sullivan. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 --- Comment #4 from jswinebz(a)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...) -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 --- Comment #5 from Zebediah Figura <z.figura12(a)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. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |97bf2e7e1edcee9600badf4b708 | |bfe2463d2c28a Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #6 from Zebediah Figura <z.figura12(a)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>. -- 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.
https://bugs.winehq.org/show_bug.cgi?id=52207 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #7 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 7.0-rc2. -- 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.
participants (1)
-
WineHQ Bugzilla