From 0b489c75d357557dbf1e9b79917c77f0f3a406bc Mon Sep 17 00:00:00 2001
From: Heiko Klare <heiko.klare@vector.com>
Date: Thu, 11 Sep 2025 17:38:16 +0200
Subject: [PATCH] [Win32] Fix potential memory leak in GC#drawImage #2499

When calling GC#drawImage(), an operation is created (like for every
execution on GC) which stores the image to draw. Since the image may be
disposed afterwards, a dispose listener is registered to the image, such
that the image can be copied and maintained inside the operation in case
of a disposal until that operation itself is disposed.
The listener is currently not de-registered when disposing the GC and
its operations, thus the GC operations (and thus also the containing GC)
will still be referenced (as listeners from the image) until that image
is disposed. Since images may be long living (or never be disposed at
all), this can lead to a memory leak.

This change removes the listener from the image upon disposal of a GC
and its operations such that no memory can occur.

Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/2499
---
 .../win32/org/eclipse/swt/graphics/GC.java    | 10 +++++++++-
 .../win32/org/eclipse/swt/graphics/Image.java |  8 ++++++++
 .../Test_org_eclipse_swt_graphics_GC.java     | 19 +++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
index e2d8c831cc1..7449b612b5f 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java	
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java	
@@ -16,6 +16,7 @@
 
 import java.util.*;
 import java.util.List;
+import java.util.function.*;
 import java.util.stream.*;
 
 import org.eclipse.swt.*;
@@ -494,9 +495,11 @@ public void copyArea (Image image, int x, int y) {
 private abstract class ImageOperation extends Operation {
 	private Image image;
 
+	private final Consumer<Image> disposeCallback = this::setCopyOfImage;
+
 	ImageOperation(Image image) {
 		setImage(image);
-		image.addOnDisposeListener(this::setCopyOfImage);
+		image.addOnDisposeListener(disposeCallback);
 	}
 
 	private void setImage(Image image) {
@@ -515,6 +518,11 @@ protected Image getImage() {
 		return image;
 	}
 
+	@Override
+	void disposeAll() {
+		image.removeOnDisposeListener(disposeCallback);
+		super.disposeAll();
+	}
 }
 
 private class CopyAreaToImageOperation extends ImageOperation {
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
index 84147c25e0c..2e665685127 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java	
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java	
@@ -1020,10 +1020,18 @@ void addOnDisposeListener(Consumer<Image> onDisposeListener) {
 	onDisposeListeners.add(onDisposeListener);
 }
 
+void removeOnDisposeListener(Consumer<Image> onDisposeListener) {
+	if (onDisposeListeners == null) {
+		return;
+	}
+	onDisposeListeners.remove(onDisposeListener);
+}
+
 @Override
 public void dispose() {
 	if (onDisposeListeners != null) {
 		onDisposeListeners.forEach(listener -> listener.accept(this));
+		onDisposeListeners.clear();
 	}
 	super.dispose();
 }
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java
index 559438997a8..6604b61b9f5 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java	
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java	
@@ -29,6 +29,7 @@
 import static org.junit.jupiter.api.Assumptions.assumeFalse;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
+import java.lang.ref.WeakReference;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.swt.SWT;
@@ -804,6 +805,24 @@ public void test_bug1288_createGCFromImageFromNonDisplayThread() throws Interrup
 	assertNull(exceptionReference.get(), "Creating a GC from an Image without a device threw an exception");
 }
 
+// Tests that a GC instance is cleaned from memory after it was disposed.
+// Primarily supposed to test that the operations (currently only implemented for Windows) do not produce any leaks.
+@Test
+public void test_noMemoryLeakAfterDispose() {
+	GC testGC = new GC(display);
+	Image image = new Image(display, 1, 1);
+	testGC.setFont(display.getSystemFont());
+	testGC.setClipping(new Rectangle(0, 0, 2, 2));
+	testGC.drawImage(image, 0, 0);
+	testGC.drawText("Test", 0, 0);
+	testGC.drawLine(0, 0, 5, 5);
+	WeakReference<GC> testGCReference = new WeakReference<>(testGC);
+	testGC.dispose();
+	testGC = null;
+	System.gc();
+	assertNull(testGCReference.get());
+}
+
 /* custom */
 Display display;
 Shell shell;