[Win32] Avoid disposed image usage in GC operations

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java

@@ -491,13 +491,38 @@ public void copyArea (Image image, int x, int y) {
 	storeAndApplyOperationForExistingHandle(new CopyAreaToImageOperation(image, x, y));
 }
 
-private class CopyAreaToImageOperation extends Operation {
-	private final Image image;
+private abstract class ImageOperation extends Operation {
+	private Image image;
+
+	ImageOperation(Image image) {
+		setImage(image);
+		image.addOnDisposeListener(this::setCopyOfImage);
+	}
+
+	private void setImage(Image image) {
+		this.image = image;
+	}
+
+	private void setCopyOfImage(Image image) {
+		if (!GC.this.isDisposed()) {
+			Image copiedImage = new Image(image.device, image, SWT.IMAGE_COPY);
+			setImage(copiedImage);
+			registerForDisposal(copiedImage);
+		}
+	}
+
+	protected Image getImage() {
+		return image;
+	}
+
+}
+
+private class CopyAreaToImageOperation extends ImageOperation {
 	private final int x;
 	private final int y;
 
 	CopyAreaToImageOperation(Image image, int x, int y) {
-		this.image = image;
+		super(image);
 		this.x = x;
 		this.y = y;
 	}
@@ -507,7 +532,7 @@ void apply() {
 		int zoom = getZoom();
 		int scaledX = Win32DPIUtils.pointToPixel(drawable, this.x, zoom);
 		int scaledY = Win32DPIUtils.pointToPixel(drawable, this.y, zoom);
-		copyAreaInPixels(this.image, scaledX, scaledY);
+		copyAreaInPixels(getImage(), scaledX, scaledY);
 	}
 }
 
@@ -1013,18 +1038,17 @@ public void drawImage (Image image, int x, int y) {
 	storeAndApplyOperationForExistingHandle(new DrawImageOperation(image, new Point(x, y)));
 }
 
-private class DrawImageOperation extends Operation {
-	private final Image image;
+private class DrawImageOperation extends ImageOperation {
 	private final Point location;
 
 	DrawImageOperation(Image image, Point location) {
-		this.image = image;
+		super(image);
 		this.location = location;
 	}
 
 	@Override
 	void apply() {
-		drawImageInPixels(this.image, Win32DPIUtils.pointToPixel(drawable, this.location, getZoom()));
+		drawImageInPixels(getImage(), Win32DPIUtils.pointToPixel(drawable, this.location, getZoom()));
 	}
 
 	private void drawImageInPixels(Image image, Point location) {
@@ -1081,13 +1105,12 @@ void drawImage(Image srcImage, int srcX, int srcY, int srcWidth, int srcHeight,
 	storeAndApplyOperationForExistingHandle(new DrawImageToImageOperation(srcImage, new Rectangle(srcX, srcY, srcWidth, srcHeight), new Rectangle(destX, destY, destWidth, destHeight), simple));
 }
 
-private class DrawScalingImageToImageOperation extends Operation {
-	private final Image image;
+private class DrawScalingImageToImageOperation extends ImageOperation {
 	private final Rectangle source;
 	private final Rectangle destination;
 
 	DrawScalingImageToImageOperation(Image image, Rectangle source, Rectangle destination) {
-		this.image = image;
+		super(image);
 		this.source = source;
 		this.destination = destination;
 	}
@@ -1096,7 +1119,7 @@ private class DrawScalingImageToImageOperation extends Operation {
 	void apply() {
 		int gcZoom = getZoom();
 		int srcImageZoom = calculateZoomForImage(gcZoom, source.width, source.height, destination.width, destination.height);
-		drawImage(image, source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, gcZoom, srcImageZoom);
+		drawImage(getImage(), source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, gcZoom, srcImageZoom);
 	}
 
 	private Collection<Integer> getAllCurrentMonitorZooms() {
@@ -1154,22 +1177,21 @@ private void drawImage(Image image, int srcX, int srcY, int srcWidth, int srcHei
 	drawImage(image, src.x, src.y, src.width, src.height, dest.x, dest.y, dest.width, dest.height, false, scaledImageZoom);
 }
 
-private class DrawImageToImageOperation extends Operation {
-	private final Image image;
+private class DrawImageToImageOperation extends ImageOperation {
 	private final Rectangle source;
 	private final Rectangle destination;
 	private final boolean simple;
 
 	DrawImageToImageOperation(Image image, Rectangle source, Rectangle destination, boolean simple) {
-		this.image = image;
+		super(image);
 		this.source = source;
 		this.destination = destination;
 		this.simple = simple;
 	}
 
 	@Override
 	void apply() {
-		drawImage(image, source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, simple, getZoom());
+		drawImage(getImage(), source.x, source.y, source.width, source.height, destination.x, destination.y, destination.width, destination.height, simple, getZoom());
 	}
 }
 

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java

@@ -139,6 +139,8 @@ public final class Image extends Resource implements Drawable {
 
 	private Map<Integer, ImageHandle> zoomLevelToImageHandle = new HashMap<>();
 
+	private List<Consumer<Image>> onDisposeListeners;
+
 private Image (Device device, int type, long handle, int nativeZoom) {
 	super(device);
 	this.type = type;
@@ -997,6 +999,21 @@ public static void drawScaled(GC gc, Image original, int width, int height, floa
 	return null;
 }
 
+void addOnDisposeListener(Consumer<Image> onDisposeListener) {
+	if (onDisposeListeners == null) {
+		onDisposeListeners = new ArrayList<>();
+	}
+	onDisposeListeners.add(onDisposeListener);
+}
+
+@Override
+public void dispose() {
+	if (onDisposeListeners != null) {
+		onDisposeListeners.forEach(listener -> listener.accept(this));
+	}
+	super.dispose();
+}
+
 @Override
 void destroy () {
 	device.deregisterResourceWithZoomSupport(this);

tests/org.eclipse.swt.tests.win32/JUnit Tests/org/eclipse/swt/graphics/ImageWin32Tests.java

@@ -17,10 +17,13 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
+import org.eclipse.swt.SWT;
 import org.eclipse.swt.internal.DPIUtil;
 import org.eclipse.swt.widgets.Display;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 /**
  * Automated Tests for class org.eclipse.swt.graphics.Image
@@ -67,4 +70,108 @@ public void testImageShouldHaveDimesionAsPerZoomLevel() {
 		}
 	}
 
+	@ParameterizedTest
+	@ValueSource(booleans = { true, false } )
+	public void testRetrieveImageDataAtDifferentZooms(boolean concurrentlyDisposeDrawnImage) {
+		Color imageColor = display.getSystemColor(SWT.COLOR_RED);
+		Image image = new Image (display, 1, 1);
+		GC gc = new GC(image);
+
+		try {
+			Image temporaryImage = createImageOfSizeOne(imageColor);
+			gc.drawImage(temporaryImage, 0,  0);
+
+			if (concurrentlyDisposeDrawnImage) {
+				temporaryImage.dispose();
+			}
+			ImageData resultImageDataAtDifferentZoom = image.getImageData(200);
+
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageDataAtDifferentZoom, 0, 0));
+
+			if (!concurrentlyDisposeDrawnImage) {
+				temporaryImage.dispose();
+			}
+		} finally {
+			gc.dispose();
+			image.dispose();
+		}
+	}
+
+	private Image createImageOfSizeOne(Color imageColor) {
+		Image imageToDraw = new Image (display, 1, 1);
+		GC imageToDrawGc = new GC(imageToDraw);
+		imageToDrawGc.setBackground(imageColor);
+		imageToDrawGc.fillRectangle(0, 0, 1, 1);
+		imageToDrawGc.dispose();
+		return imageToDraw;
+	}
+
+	private RGB getRGBAtPixel(ImageData resultImageDataAtDifferentZoom, int x, int y) {
+		return resultImageDataAtDifferentZoom.palette.getRGB(resultImageDataAtDifferentZoom.getPixel(x, y));
+	}
+
+	@ParameterizedTest
+	@ValueSource(booleans = { true, false } )
+	public void testDrawImageAtDifferentZooms(boolean concurrentlyDisposeDrawnImage) {
+		Color imageColor = display.getSystemColor(SWT.COLOR_RED);
+		Image image = new Image (display, 1, 1);
+		GC gc = new GC(image);
+		Image temporaryImage = createImageOfSizeOne(imageColor);
+		try {
+			gc.drawImage(temporaryImage, 0,  0);
+		} finally {
+			if (concurrentlyDisposeDrawnImage) {
+				temporaryImage.dispose();
+			}
+		}
+
+		Image targetCanvas = new Image(display, 5, 5);
+		GC targetCanvasGc = new GC(targetCanvas);
+		try {
+			targetCanvasGc.getGCData().nativeZoom = 200;
+			targetCanvasGc.drawImage(image, 0, 0);
+			targetCanvasGc.getGCData().nativeZoom = 100;
+			targetCanvasGc.drawImage(image, 3, 0);
+			ImageData resultImageData = targetCanvas.getImageData(100);
+
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 0, 0));
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 1, 0));
+			assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 2, 0));
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 3, 0));
+			assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 4, 0));
+		} finally {
+			if (!concurrentlyDisposeDrawnImage) {
+				temporaryImage.dispose();
+			}
+			image.dispose();
+			targetCanvasGc.dispose();
+			gc.dispose();
+		}
+	}
+
+	@Test
+	public void testDisposeDrawnImageBeforeRequestingTargetForOtherZoom() {
+		Color imageColor = display.getSystemColor(SWT.COLOR_RED);
+		Image imageToDraw = new Image (display, 1, 1);
+		GC imageToDrawGc = new GC(imageToDraw);
+		imageToDrawGc.setBackground(imageColor);
+		imageToDrawGc.fillRectangle(0, 0, 1, 1);
+
+		Image targetCanvas = new Image(display, 5, 5);
+		GC targetCanvasGc = new GC(targetCanvas);
+		try {
+			targetCanvasGc.drawImage(imageToDraw, 0, 0);
+			imageToDrawGc.dispose();
+			imageToDraw.dispose();
+			ImageData resultImageData = targetCanvas.getImageData(200);
+
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 0, 0));
+			assertEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 1, 0));
+			assertNotEquals(imageColor.getRGB(), getRGBAtPixel(resultImageData, 2, 0));
+		} finally {
+			targetCanvasGc.dispose();
+			targetCanvas.dispose();
+		}
+	}
+
 }

tests/org.eclipse.swt.tests.win32/META-INF/MANIFEST.MF

@@ -9,4 +9,6 @@ Eclipse-BundleShape: dir
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Automatic-Module-Name: org.eclipse.swt.tests.win32
 Import-Package: org.junit.jupiter.api;version="[5.13.0,6.0.0)",
+ org.junit.jupiter.params;version="[5.13.0,6.0.0)",
+ org.junit.jupiter.params.provider;version="[5.13.0,6.0.0)",
  org.junit.platform.suite.api;version="[1.13.0,2.0.0)"