RFR: 8282862: AwtWindow::SetIconData leaks old icon handles if an exception is detected [v3]
Alexey Ivanov
aivanov at openjdk.org
Fri Jan 10 20:44:47 UTC 2025
On Tue, 7 Jan 2025 19:37:24 GMT, Rajat Mahajan <rmahajan at 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:
>
> Update copyright year
The suggested fix does not address the reported issue yet: both `hOldIcon` and `hOldIconSm` are still leaked if the function exists prematurely.
Based on my above comments, I suggest restructuring the code.
1. Create new icons using `CreateIconFromRaster`. Do not assign the handles to member fields `m_hIcon` and `m_hIconSm` just yet.
2. Wrap the calls to `CreateIconFromRaster` in step 1 into a try-catch block to prevent a C++ exception escaping from JNI code as well as ensure a Java exception isn't raised.
1. If any exception occurs, destroy the newly created icon handles (`(new_)hIcon` and `(new_)hIconSm`) and exit.
In this case both `m_hIcon` and `m_hIconSm` remain unmodified—the current icons remain unchanged.
2. If no exception occurs,
1. Assign the old values of `m_hIcon` and `m_hIconSm` to temporary variables (`hOldIcon` and `hOldIconSm`),
~~or call `DestroyIcon` right on the member fields `m_*`~~;
2. Assign the newly created icons to the member fields `m_hIcon` and `m_hIconSm` and execute the code below to update the inherited icons;
3. Call `DestroyIcon` on `hOldIcon` and `hOldIconSm`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/22932#issuecomment-2584089564
More information about the client-libs-dev
mailing list