On Fri, 17 Jan 2025 18:22:57 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:
>> **Issue:**
>> AwtWindow::SetIconData leaks the old icon handles in hOldIcon and hOldIconSm
>> if CreateIconFromRaster raises an exception. Additionally, an exception is
>> checked only after the first call to CreateIconFromRaster.
>>
>> **Solution:**
>> I have added the exception handling code to take care that the handles are
>> properly destroyed and not leaked.
>>
>> **Testing:**
>> I have tested the code to make sure there are no regressions caused by this.
>
> Rajat Mahajan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> check using not equal to NULL to match the rest of the code in the function
Although this leak is extremely unlikely, leaks of GDI resources can bring even
a modern windows system to its knees in a matter of seconds ..
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 2115:
> 2113: HICON hNewIcon = NULL;
> 2114: HICON hNewIconSm = NULL;
> 2115:
I presume we know for sure there's no exception pending when we enter ?
Looking at the only caller, it seems probable.
src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 2147:
> 2145: HICON hOldIcon = NULL;
> 2146: HICON hOldIconSm = NULL;
> 2147: if ((m_hIcon != NULL) && !m_iconInherited) {
I am not sure why the check for NULL is needed .. but it is OK.
-------------
Marked as reviewed by prr (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22932#pullrequestreview-2560101616
PR Review Comment: https://git.openjdk.org/jdk/pull/22932#discussion_r1920838476
PR Review Comment: https://git.openjdk.org/jdk/pull/22932#discussion_r1920839867
