4200096: OffScreenImageSource.removeConsumer NullPointerException

Reviewed-by: prr, serb

Author: mickleness

src/java.desktop/share/classes/sun/awt/image/OffScreenImageSource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2023, 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
@@ -201,9 +201,12 @@ private void produce() {
                 }
             }
         } catch (NullPointerException e) {
-            e.printStackTrace();
+            // If theConsumer is null and we throw a NPE when interacting with it:
+            // That's OK. That is an expected use case that can happen when an
+            // ImageConsumer detaches itself from this ImageProducer mid-production.
 
             if (theConsumer != null) {
+                e.printStackTrace();
                 theConsumer.imageComplete(ImageConsumer.IMAGEERROR);
             }
         }

test/jdk/sun/awt/image/OffScreenImageSource/bug4200096/AddNullConsumerTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2023, 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.image.BufferedImage;
+
+/**
+ * @test
+ * @bug 4200096
+ * @summary OffScreenImageSource.addConsumer(null) shouldn't throw (or print) a NullPointerException
+ * @author Jeremy Wood
+ */
+
+/**
+ * This makes sure if OffScreenImageSource#addConsumer(null) is called: we
+ * treat that as a no-op and return immediately.
+ * <p>
+ * This test exists primarily to make sure the resolution to 4200096 does not
+ * significantly change legacy behavior. Whether or not a NPE is printed to
+ * System.err is not a hotly contested question, but at one point one of the
+ * proposed (and rejected) resolutions to 4200096 had the potential to
+ * throw a NPE when addConsumer(null) was called. That would be a
+ * significant change that we want to avoid.
+ * </p>
+ */
+ public class AddNullConsumerTest {
+    public static void main(String[] args) throws Exception {
+        try (AutoCloseable setup = bug4200096.setupTest(false)) {
+            BufferedImage bufferedImage = new BufferedImage(1, 1, BufferedImage.TYPE_INT_ARGB);
+            bufferedImage.getSource().addConsumer(null);
+        }
+    }
+}

test/jdk/sun/awt/image/OffScreenImageSource/bug4200096/ImageSizeTest.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2023, 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.Image;
+import java.awt.image.BufferedImage;
+import java.awt.image.ImageObserver;
+
+/**
+ * @test
+ * @bug 4200096
+ * @summary this real-world example detaches an ImageConsumer from an OSIS immediately after it learns the dimensions.
+ * @author Jeremy Wood
+ */
+
+/**
+ * This adds an ImageObserver that is only interested in identifying the dimensions of an ImageProducer.
+ * <p>
+ * Once it has the dimensions: {@link java.awt.image.ImageObserver#imageUpdate(Image, int, int, int, int, int)}
+ * returns false. This triggers the caller to remove the ImageObserver for us. And removing an ImageObserver
+ * while an OffScreenImageSource is mid-production triggers the NPE that is JDK-4200096.
+ * <p>
+ * The expected behavior is for this method to complete without OffScreenImageSource throwing/catching a NPE.
+ * <p>
+ * What's interesting about this test is: we never even explicitly call {@link java.awt.image.ImageProducer#addConsumer(ImageConsumer)}
+ * or {@link java.awt.image.ImageProducer#removeConsumer(ImageConsumer)} (ImageConsumer)}.
+ * </p>
+ */
+public class ImageSizeTest {
+    public static void main(String[] args) throws Exception {
+        try (AutoCloseable setup = bug4200096.setupTest(false)) {
+            Image img = createAbstractImage();
+            ImageObserver observer = new ImageObserver() {
+                Integer imageWidth, imageHeight;
+                @Override
+                public boolean imageUpdate(Image img, int infoflags, int x, int y, int width, int height) {
+                    if ( (infoflags | ImageObserver.WIDTH) > 0) {
+                        imageWidth = width;
+                    }
+                    if ( (infoflags | ImageObserver.HEIGHT) > 0) {
+                        imageHeight = height;
+                    }
+
+                    if (imageWidth != null || imageHeight != null)
+                        return false;
+                    return true;
+
+                }
+            };
+            img.getWidth(observer);
+        }
+    }
+
+    /**
+     * This creates a ToolkitImage, so it is not a BufferedImage.
+     * <p>
+     * This specific implementation happens to rely on scaling an existing
+     * BufferedImage, because that seemed like an easy way to avoid bundling a
+     * JPG/PNG with this unit test. But this return value still happens to be
+     * a ToolkitImage, which is what a JPG/PNG would also be (when loaded
+     * via the Toolkit class and not ImageIO).
+     * </p>
+     */
+    private static Image createAbstractImage() {
+        BufferedImage bufferedImage = new BufferedImage(1, 1, BufferedImage.TYPE_INT_ARGB);
+        Image img = bufferedImage.getScaledInstance(2, 2, Image.SCALE_SMOOTH);
+        return img;
+    }
+}

test/jdk/sun/awt/image/OffScreenImageSource/bug4200096/LegitimateNullPointerTest.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2023, 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.image.BufferedImage;
+import java.awt.image.ColorModel;
+import java.awt.image.ImageConsumer;
+import java.util.Hashtable;
+
+/**
+ * @test
+ * @bug 4200096
+ * @summary This makes sure NPE's that come from the ImageConsumer are still printed to System.err
+ * @author Jeremy Wood
+ */
+
+/**
+ * This test makes sure that if an ImageConsumer throws a NullPointerException: that NPE
+ * should be printed to System.err.
+ * <p>
+ * Most of the discussion around JDK-4200096 focuses on how we handle NPE's that come from
+ * OffScreenImageSource itself, but this test checks how we handle NPE's that come from
+ * the ImageConsumer that's listening to an OffScreenImageSource.
+ * </p>
+ * <p>
+ * This test is enforcing a legacy behavior.
+ * </p>
+ */
+ public class LegitimateNullPointerTest {
+    public static void main(String[] args) throws Exception {
+        try (AutoCloseable setup = bug4200096.setupTest(true)) {
+            BufferedImage bufferedImage = new BufferedImage(10, 10, BufferedImage.TYPE_INT_ARGB);
+            bufferedImage.getSource().addConsumer(new ImageConsumer() {
+
+                @Override
+                public void setDimensions(int width, int height) {
+                    throw new NullPointerException();
+                }
+
+                @Override
+                public void setProperties(Hashtable<?, ?> props) {
+                    // intentionally empty
+                }
+
+                @Override
+                public void setColorModel(ColorModel model) {
+                    // intentionally empty
+                }
+
+                @Override
+                public void setHints(int hintflags) {
+                    // intentionally empty
+                }
+
+                @Override
+                public void setPixels(int x, int y, int w, int h, ColorModel model, byte[] pixels, int off, int scansize) {
+                    // intentionally empty
+                }
+
+                @Override
+                public void setPixels(int x, int y, int w, int h, ColorModel model, int[] pixels, int off, int scansize) {
+                    // intentionally empty
+                }
+
+                @Override
+                public void imageComplete(int status) {
+                    // intentionally empty
+                }
+            });
+        }
+    }
+}