From 9a13ff1b5c66b7b82b15e96646992cfbb8c7e75c Mon Sep 17 00:00:00 2001 From: Nikita Gubarkov Date: Wed, 13 Aug 2025 17:36:07 +0000 Subject: [PATCH] 8364434: Inconsistent BufferedContext state after GC Reviewed-by: jdv, azvegint, avu (cherry picked from commit 899e13f40a70c98d1d393ba6c3973abcb36e1f00) --- .../sun/java2d/pipe/BufferedContext.java | 60 ++++++++------ .../java/awt/ColorClass/WeakColorTest.java | 83 +++++++++++++++++++ 2 files changed, 118 insertions(+), 25 deletions(-) create mode 100644 test/jdk/java/awt/ColorClass/WeakColorTest.java diff --git a/src/java.desktop/share/classes/sun/java2d/pipe/BufferedContext.java b/src/java.desktop/share/classes/sun/java2d/pipe/BufferedContext.java index e57b240bce5..83e63fc75d8 100644 --- a/src/java.desktop/share/classes/sun/java2d/pipe/BufferedContext.java +++ b/src/java.desktop/share/classes/sun/java2d/pipe/BufferedContext.java @@ -100,11 +100,11 @@ public abstract class BufferedContext { */ protected static BufferedContext currentContext; - private Reference validSrcDataRef = new WeakReference<>(null); - private Reference validDstDataRef = new WeakReference<>(null); - private Reference validClipRef = new WeakReference<>(null); - private Reference validCompRef = new WeakReference<>(null); - private Reference validPaintRef = new WeakReference<>(null); + private Reference validSrcDataRef = null; + private Reference validDstDataRef = null; + private Reference validClipRef = null; + private Reference validCompRef = null; + private Reference validPaintRef = null; // renamed from isValidatedPaintAColor as part of a work around for 6764257 private boolean isValidatedPaintJustAColor; private int validatedRGB; @@ -213,20 +213,18 @@ private void validate(AccelSurface srcData, AccelSurface dstData, updatePaint = true; isValidatedPaintJustAColor = true; } - } else if (validPaintRef.get() != paint) { + } else if (stateChanged(validPaintRef, paint)) { updatePaint = true; // this should be set when we are switching from paint to color // in which case this condition will be true isValidatedPaintJustAColor = false; } - final AccelSurface validatedSrcData = validSrcDataRef.get(); - final AccelSurface validatedDstData = validDstDataRef.get(); - if ((currentContext != this) || - (srcData != validatedSrcData) || - (dstData != validatedDstData)) + final boolean srcChanged = stateChanged(validSrcDataRef, srcData); + final boolean dstChanged = stateChanged(validDstDataRef, dstData); + if ((currentContext != this) || srcChanged || dstChanged) { - if (dstData != validatedDstData) { + if (dstChanged) { // the clip is dependent on the destination surface, so we // need to update it if we have a new destination surface updateClip = true; @@ -243,13 +241,13 @@ private void validate(AccelSurface srcData, AccelSurface dstData, setSurfaces(srcData, dstData); currentContext = this; - validSrcDataRef = new WeakReference<>(srcData); - validDstDataRef = new WeakReference<>(dstData); + validSrcDataRef = wrapState(srcData); + validDstDataRef = wrapState(dstData); } // validate clip - final Region validatedClip = validClipRef.get(); - if ((clip != validatedClip) || updateClip) { + final Region validatedClip = validClipRef == null ? null : validClipRef.get(); + if (stateChanged(validClipRef, clip) || updateClip) { if (clip != null) { if (updateClip || validatedClip == null || @@ -264,13 +262,13 @@ private void validate(AccelSurface srcData, AccelSurface dstData, } else { resetClip(); } - validClipRef = new WeakReference<>(clip); + validClipRef = wrapState(clip); } // validate composite (note that a change in the context flags // may require us to update the composite state, even if the // composite has not changed) - if ((comp != validCompRef.get()) || (flags != validatedFlags)) { + if (stateChanged(validCompRef, comp) || (flags != validatedFlags)) { if (comp != null) { setComposite(comp, flags); } else { @@ -279,7 +277,7 @@ private void validate(AccelSurface srcData, AccelSurface dstData, // the paint state is dependent on the composite state, so make // sure we update the color below updatePaint = true; - validCompRef = new WeakReference<>(comp); + validCompRef = wrapState(comp); validatedFlags = flags; } @@ -313,7 +311,7 @@ private void validate(AccelSurface srcData, AccelSurface dstData, } else { BufferedPaints.resetPaint(rq); } - validPaintRef = new WeakReference<>(paint); + validPaintRef = wrapState(paint); } // mark dstData dirty @@ -321,6 +319,18 @@ private void validate(AccelSurface srcData, AccelSurface dstData, dstData.markDirty(); } + private static boolean stateChanged(Reference ref, T obj) { + // null ref means "true" null object + if (ref == null) return obj != null; + T old = ref.get(); + // null ref value means the object was GC'ed, return true in that case + return old == null || old != obj; + } + + private static Reference wrapState(T obj) { + return obj == null ? null : new WeakReference<>(obj); + } + private void setSurfaces(AccelSurface srcData, AccelSurface dstData) { @@ -434,11 +444,11 @@ public final void invalidateContext() { resetComposite(); resetClip(); BufferedPaints.resetPaint(rq); - validSrcDataRef.clear(); - validDstDataRef.clear(); - validCompRef.clear(); - validClipRef.clear(); - validPaintRef.clear(); + validSrcDataRef = null; + validDstDataRef = null; + validCompRef = null; + validClipRef = null; + validPaintRef = null; isValidatedPaintJustAColor = false; xformInUse = false; } diff --git a/test/jdk/java/awt/ColorClass/WeakColorTest.java b/test/jdk/java/awt/ColorClass/WeakColorTest.java new file mode 100644 index 00000000000..d4adbc3f111 --- /dev/null +++ b/test/jdk/java/awt/ColorClass/WeakColorTest.java @@ -0,0 +1,83 @@ +/* + * Copyright (c) 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.awt.Color; +import java.awt.Graphics2D; +import java.awt.GraphicsEnvironment; +import java.awt.image.BufferedImage; +import java.awt.image.VolatileImage; +import java.lang.ref.WeakReference; + +import jtreg.SkippedException; + +/* + * @test + * @key headful + * @bug 8364434 + * @summary Check that garbage-collecting Color before accelerated painting is complete does not cause artifacts. + * @requires (os.family != "linux") + * @library /test/lib + * @run main/othervm -Xms16m -Xmx16m WeakColorTest + */ + +public class WeakColorTest { + public static void main(String[] args) throws Exception { + BufferedImage bi = new BufferedImage(100, 100, BufferedImage.TYPE_INT_RGB); // This image is full-black. + VolatileImage image = GraphicsEnvironment.getLocalGraphicsEnvironment() + .getDefaultScreenDevice().getDefaultConfiguration().createCompatibleVolatileImage(100, 100); + Graphics2D g = image.createGraphics(); + + // Create a new Color - we want it to be collected later. + g.setColor(new Color(255, 0, 0)); + WeakReference color = new WeakReference<>(g.getColor()); + + g.fillRect(0, 0, 100, 100); + + // Change color to prevent Graphics from keeping our Color alive. + g.setColor(Color.BLACK); + + // Force Color to be GC'ed. + final int MAX_ITERATIONS = 1000, ARRAY_SIZE = 1000000; + WeakReference array = null; + for (int i = 0;; i++) { + System.gc(); + if (color.get() == null) { + System.out.println("Color collected at: " + i); + break; + } else if (i >= MAX_ITERATIONS) { + throw new SkippedException("Color was not collected after " + MAX_ITERATIONS + " iterations"); + } + Object[] a = new Object[ARRAY_SIZE]; + a[0] = array; + array = new WeakReference<>(a); + } + + // Do a blit. If it succeeds, the resulting image will be full-black. + g.drawImage(bi, 0, 0, null); + g.dispose(); + + // We expect black. If it's red, then the blit must have failed. + int actualColor = image.getSnapshot().getRGB(50, 50); + if ((actualColor & 0xFFFFFF) != 0) throw new Error("Wrong color: 0x" + Integer.toHexString(actualColor)); + } +}