From 726fbd9a64e0c74ed159610f5d587df27763dc69 Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Mon, 6 Jan 2025 09:26:03 -0800 Subject: [PATCH 1/6] 8282862: AwtWindow::SetIconData leaks old icon handles if an exception is detected --- .../windows/native/libawt/windows/awt_Window.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index f632da02162ad..6aaffd1791416 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -2121,9 +2121,19 @@ void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, hOldIconSm = m_hIconSm; } m_hIconSm = NULL; - m_hIcon = CreateIconFromRaster(env, iconRaster, w, h); - JNU_CHECK_EXCEPTION(env); - m_hIconSm = CreateIconFromRaster(env, smallIconRaster, smw, smh); + + try { + m_hIcon = CreateIconFromRaster(env, iconRaster, w, h); + JNU_CHECK_EXCEPTION(env); // Might throw here + m_hIconSm = CreateIconFromRaster(env, smallIconRaster, smw, smh); + JNU_CHECK_EXCEPTION(env); // Or here + } catch (...) { + // Clean up any allocated resources here + if (m_hIcon != NULL) { + DestroyIcon(m_hIcon); + } + throw; // Re-throw the exception + } m_iconInherited = (m_hIcon == NULL); if (m_iconInherited) { From bdba3a94b0de1f99eff8adf2d901a00497998a7a Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Mon, 6 Jan 2025 13:10:45 -0800 Subject: [PATCH 2/6] fix indentation and make sure we delete both handles. --- .../windows/native/libawt/windows/awt_Window.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index 6aaffd1791416..2d87ed5fe75fc 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -2127,12 +2127,15 @@ void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, JNU_CHECK_EXCEPTION(env); // Might throw here m_hIconSm = CreateIconFromRaster(env, smallIconRaster, smw, smh); JNU_CHECK_EXCEPTION(env); // Or here - } catch (...) { - // Clean up any allocated resources here - if (m_hIcon != NULL) { - DestroyIcon(m_hIcon); - } - throw; // Re-throw the exception + } catch (...) { + // Clean up any allocated resources here + if (m_hIcon != NULL) { + DestroyIcon(m_hIcon); + } + if (m_hIconSm != NULL) { + DestroyIcon(m_hIconSm); + } + throw; // Re-throw the exception } m_iconInherited = (m_hIcon == NULL); From 79a82de036ce8d24a124b03312eaa5889e6a6f26 Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Tue, 7 Jan 2025 11:33:58 -0800 Subject: [PATCH 3/6] Update copyright year --- src/java.desktop/windows/native/libawt/windows/awt_Window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index 2d87ed5fe75fc..706ece9564c40 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it From e22d8119689d1ea4ae6e76788782451ce71cdfa5 Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Wed, 15 Jan 2025 13:49:04 -0800 Subject: [PATCH 4/6] Update code according to the feedback in code review --- .../native/libawt/windows/awt_Window.cpp | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index 706ece9564c40..4b505298b82f4 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -2108,35 +2108,50 @@ HICON CreateIconFromRaster(JNIEnv* env, jintArray iconRaster, jint w, jint h) } void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, - jintArray smallIconRaster, jint smw, jint smh) -{ + jintArray smallIconRaster, jint smw, jint smh) { + HICON hNewIcon = NULL; + HICON hNewIconSm = NULL; + + try { + hNewIcon = CreateIconFromRaster(env, iconRaster, w, h); + if (env->ExceptionCheck()) { + if (hNewIcon) { + DestroyIcon(hNewIcon); + } + return; + } + + hNewIconSm = CreateIconFromRaster(env, smallIconRaster, smw, smh); + if (env->ExceptionCheck()) { + if (hNewIcon != NULL) { + DestroyIcon(hNewIcon); + } + if (hNewIconSm != NULL) { + DestroyIcon(hNewIconSm); + } + return; + } + } catch (...) { + if (hNewIcon != NULL) { + DestroyIcon(hNewIcon); + } + if (hNewIconSm != NULL) { + DestroyIcon(hNewIconSm); + } + return; + } + HICON hOldIcon = NULL; HICON hOldIconSm = NULL; - //Destroy previous icon if it isn't inherited if ((m_hIcon != NULL) && !m_iconInherited) { hOldIcon = m_hIcon; } - m_hIcon = NULL; if ((m_hIconSm != NULL) && !m_iconInherited) { hOldIconSm = m_hIconSm; } - m_hIconSm = NULL; - try { - m_hIcon = CreateIconFromRaster(env, iconRaster, w, h); - JNU_CHECK_EXCEPTION(env); // Might throw here - m_hIconSm = CreateIconFromRaster(env, smallIconRaster, smw, smh); - JNU_CHECK_EXCEPTION(env); // Or here - } catch (...) { - // Clean up any allocated resources here - if (m_hIcon != NULL) { - DestroyIcon(m_hIcon); - } - if (m_hIconSm != NULL) { - DestroyIcon(m_hIconSm); - } - throw; // Re-throw the exception - } + m_hIcon = hNewIcon; + m_hIconSm = hNewIconSm; m_iconInherited = (m_hIcon == NULL); if (m_iconInherited) { @@ -2149,8 +2164,11 @@ void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, m_iconInherited = FALSE; } } + DoUpdateIcon(); EnumThreadWindows(AwtToolkit::MainThread(), UpdateOwnedIconCallback, (LPARAM)this); + + // Destroy previous icons if they were not inherited if (hOldIcon != NULL) { DestroyIcon(hOldIcon); } From 505f804ba3a5047397e287c373a19c736ec1c10f Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Fri, 17 Jan 2025 09:50:02 -0800 Subject: [PATCH 5/6] leave opening bracre as is --- src/java.desktop/windows/native/libawt/windows/awt_Window.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index 4b505298b82f4..f33a7e782da3c 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -2108,7 +2108,8 @@ HICON CreateIconFromRaster(JNIEnv* env, jintArray iconRaster, jint w, jint h) } void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, - jintArray smallIconRaster, jint smw, jint smh) { + jintArray smallIconRaster, jint smw, jint smh) +{ HICON hNewIcon = NULL; HICON hNewIconSm = NULL; From 2f47a554e93a64869cc560fb89790b3475763557 Mon Sep 17 00:00:00 2001 From: Rajat Mahajan Date: Fri, 17 Jan 2025 10:18:33 -0800 Subject: [PATCH 6/6] check using not equal to NULL to match the rest of the code in the function --- src/java.desktop/windows/native/libawt/windows/awt_Window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp index f33a7e782da3c..d5bbdc51b0b46 100644 --- a/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp +++ b/src/java.desktop/windows/native/libawt/windows/awt_Window.cpp @@ -2116,7 +2116,7 @@ void AwtWindow::SetIconData(JNIEnv* env, jintArray iconRaster, jint w, jint h, try { hNewIcon = CreateIconFromRaster(env, iconRaster, w, h); if (env->ExceptionCheck()) { - if (hNewIcon) { + if (hNewIcon != NULL) { DestroyIcon(hNewIcon); } return;