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