8211992: GraphicsConfiguration.getDevice().getDisplayMode() causes JVM crash on Mac

Reviewed-by: prr

src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2018, 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
@@ -25,9 +25,12 @@
 
 package sun.awt;
 
-import java.awt.*;
+import java.awt.GraphicsConfiguration;
-import java.awt.geom.*;
+import java.awt.Rectangle;
-import java.awt.image.*;
+import java.awt.Transparency;
+import java.awt.geom.AffineTransform;
+import java.awt.image.BufferedImage;
+import java.awt.image.ColorModel;
 
 import sun.java2d.SurfaceData;
 import sun.java2d.opengl.CGLLayer;
@@ -49,12 +52,9 @@ public abstract class CGraphicsConfig extends GraphicsConfiguration
         throw new UnsupportedOperationException("not implemented");
     }
 
-    private static native Rectangle2D nativeGetBounds(int screen);
-
     @Override
-    public Rectangle getBounds() {
+    public final Rectangle getBounds() {
-        final Rectangle2D nativeBounds = nativeGetBounds(device.getCGDisplayID());
+        return device.getBounds();
-        return nativeBounds.getBounds(); // does integer rounding
     }
 
     @Override

src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, 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
@@ -30,8 +30,11 @@ import java.awt.DisplayMode;
 import java.awt.GraphicsConfiguration;
 import java.awt.GraphicsDevice;
 import java.awt.Insets;
+import java.awt.Rectangle;
 import java.awt.Window;
+import java.awt.geom.Rectangle2D;
 import java.util.Objects;
+
 import sun.java2d.SunGraphicsEnvironment;
 import sun.java2d.opengl.CGLGraphicsConfig;
 
@@ -45,6 +48,7 @@ public final class CGraphicsDevice extends GraphicsDevice
     private volatile int displayID;
     private volatile double xResolution;
     private volatile double yResolution;
+    private volatile Rectangle bounds;
     private volatile int scale;
 
     // Array of all GraphicsConfig instances for this device
@@ -61,20 +65,10 @@ public final class CGraphicsDevice extends GraphicsDevice
     public CGraphicsDevice(final int displayID) {
         this.displayID = displayID;
         configs = new GraphicsConfiguration[] {
-            CGLGraphicsConfig.getConfig(this, 0)
+            CGLGraphicsConfig.getConfig(this, displayID, 0)
         };
     }
 
-    /**
-     * Returns CGDirectDisplayID, which is the same id as @"NSScreenNumber" in
-     * NSScreen.
-     *
-     * @return CoreGraphics display id.
-     */
-    public int getCGDisplayID() {
-        return displayID;
-    }
-
     /**
      * Return a list of all configurations.
      */
@@ -118,6 +112,10 @@ public final class CGraphicsDevice extends GraphicsDevice
         return yResolution;
     }
 
+    Rectangle getBounds() {
+        return bounds.getBounds();
+    }
+
     public Insets getScreenInsets() {
         // the insets are queried synchronously and are not cached
         // since there are no Quartz or Cocoa means to receive notifications
@@ -140,6 +138,7 @@ public final class CGraphicsDevice extends GraphicsDevice
     public void displayChanged() {
         xResolution = nativeGetXResolution(displayID);
         yResolution = nativeGetYResolution(displayID);
+        bounds = nativeGetBounds(displayID).getBounds(); //does integer rounding
         initScaleFactor();
         //TODO configs/fullscreenWindow/modes?
     }
@@ -273,4 +272,6 @@ public final class CGraphicsDevice extends GraphicsDevice
     private static native double nativeGetYResolution(int displayID);
 
     private static native Insets nativeGetScreenInsets(int displayID);
+
+    private static native Rectangle2D nativeGetBounds(int displayID);
 }

src/java.desktop/macosx/classes/sun/awt/CGraphicsEnvironment.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2018, 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
@@ -25,10 +25,22 @@
 
 package sun.awt;
 
-import java.awt.*;
+import java.awt.AWTError;
-import java.util.*;
+import java.awt.Font;
+import java.awt.GraphicsConfiguration;
+import java.awt.GraphicsDevice;
+import java.awt.HeadlessException;
+import java.awt.Toolkit;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.ListIterator;
+import java.util.Map;
 
-import sun.java2d.*;
+import sun.java2d.MacosxSurfaceManagerFactory;
+import sun.java2d.SunGraphicsEnvironment;
+import sun.java2d.SurfaceManagerFactory;
 
 /**
  * This is an implementation of a GraphicsEnvironment object for the default
@@ -87,6 +99,9 @@ public final class CGraphicsEnvironment extends SunGraphicsEnvironment {
     /** Reference to the display reconfiguration callback context. */
     private final long displayReconfigContext;
 
+    // list of invalidated graphics devices (those which were removed)
+    private List<WeakReference<CGraphicsDevice>> oldDevices = new ArrayList<>();
+
     /**
      * Construct a new instance.
      */
@@ -116,11 +131,23 @@ public final class CGraphicsEnvironment extends SunGraphicsEnvironment {
         synchronized (this) {
             if (removed && devices.containsKey(displayId)) {
                 final CGraphicsDevice gd = devices.remove(displayId);
-                gd.invalidate(getMainDisplayID());
+                oldDevices.add(new WeakReference<>(gd));
-                gd.displayChanged();
             }
         }
         initDevices();
+
+        // Need to notify old devices, in case the user hold the reference to it
+        for (ListIterator<WeakReference<CGraphicsDevice>> it =
+             oldDevices.listIterator(); it.hasNext(); ) {
+            CGraphicsDevice gd = it.next().get();
+            if (gd != null) {
+                gd.invalidate(mainDisplayID);
+                gd.displayChanged();
+            } else {
+                // no more references to this device, remove it
+                it.remove();
+            }
+        }
     }
 
     @Override

src/java.desktop/macosx/classes/sun/java2d/opengl/CGLGraphicsConfig.java

@@ -54,12 +54,14 @@ import sun.java2d.opengl.OGLContext.OGLContextCaps;
 import sun.java2d.pipe.hw.AccelSurface;
 import sun.java2d.pipe.hw.AccelTypedVolatileImage;
 import sun.java2d.pipe.hw.ContextCapabilities;
-import static sun.java2d.opengl.OGLSurfaceData.*;
-import static sun.java2d.opengl.OGLContext.OGLContextCaps.*;
-
 import sun.lwawt.LWComponentPeer;
 import sun.lwawt.macosx.CPlatformView;
 
+import static sun.java2d.opengl.OGLContext.OGLContextCaps.CAPS_DOUBLEBUFFERED;
+import static sun.java2d.opengl.OGLContext.OGLContextCaps.CAPS_EXT_FBOBJECT;
+import static sun.java2d.opengl.OGLSurfaceData.FBOBJECT;
+import static sun.java2d.opengl.OGLSurfaceData.TEXTURE;
+
 public final class CGLGraphicsConfig extends CGraphicsConfig
     implements OGLGraphicsConfig
 {
@@ -125,7 +127,7 @@ public final class CGLGraphicsConfig extends CGraphicsConfig
     }
 
     public static CGLGraphicsConfig getConfig(CGraphicsDevice device,
-                                              int pixfmt)
+                                              int displayID, int pixfmt)
     {
         if (!cglAvailable) {
             return null;
@@ -141,9 +143,7 @@ public final class CGLGraphicsConfig extends CGraphicsConfig
             // surfaces/contexts, so we should first invalidate the current
             // Java-level context and flush the queue...
             OGLContext.invalidateCurrentContext();
-
+            cfginfo = getCGLConfigInfo(displayID, pixfmt, kOpenGLSwapInterval);
-            cfginfo = getCGLConfigInfo(device.getCGDisplayID(), pixfmt,
-                                       kOpenGLSwapInterval);
             if (cfginfo != 0L) {
                 textureSize = nativeGetMaxTextureSize();
                 // 7160609: GL still fails to create a square texture of this
@@ -259,8 +259,8 @@ public final class CGLGraphicsConfig extends CGraphicsConfig
 
     @Override
     public String toString() {
-        int displayID = getDevice().getCGDisplayID();
+        String display = getDevice().getIDstring();
-        return ("CGLGraphicsConfig[dev="+displayID+",pixfmt="+pixfmt+"]");
+        return ("CGLGraphicsConfig[" + display + ", pixfmt=" + pixfmt + "]");
     }
 
     @Override

src/java.desktop/macosx/classes/sun/lwawt/macosx/CRobot.java

@@ -25,12 +25,15 @@
 
 package sun.lwawt.macosx;
 
-import java.awt.*;
+import java.awt.Point;
-import java.awt.peer.*;
+import java.awt.Rectangle;
+import java.awt.Robot;
+import java.awt.peer.RobotPeer;
 
 import sun.awt.CGraphicsDevice;
 
-class CRobot implements RobotPeer {
+final class CRobot implements RobotPeer {
+
     private static final int MOUSE_LOCATION_UNKNOWN      = -1;
 
     private final CGraphicsDevice fDevice;
@@ -65,8 +68,7 @@ class CRobot implements RobotPeer {
         mouseLastX = x;
         mouseLastY = y;
 
-        mouseEvent(fDevice.getCGDisplayID(), mouseLastX, mouseLastY,
+        mouseEvent(mouseLastX, mouseLastY, mouseButtonsState, true, true);
-                   mouseButtonsState, true, true);
     }
 
     /**
@@ -79,8 +81,7 @@ class CRobot implements RobotPeer {
     public void mousePress(int buttons) {
         mouseButtonsState |= buttons;
         checkMousePos();
-        mouseEvent(fDevice.getCGDisplayID(), mouseLastX, mouseLastY,
+        mouseEvent(mouseLastX, mouseLastY, buttons, true, false);
-                   buttons, true, false);
     }
 
     /**
@@ -93,8 +94,7 @@ class CRobot implements RobotPeer {
     public void mouseRelease(int buttons) {
         mouseButtonsState &= ~buttons;
         checkMousePos();
-        mouseEvent(fDevice.getCGDisplayID(), mouseLastX, mouseLastY,
+        mouseEvent(mouseLastX, mouseLastY, buttons, false, false);
-                   buttons, false, false);
     }
 
     /**
@@ -193,8 +193,7 @@ class CRobot implements RobotPeer {
     }
 
     private native void initRobot();
-    private native void mouseEvent(int displayID, int lastX, int lastY,
+    private native void mouseEvent(int lastX, int lastY, int buttonsState,
-                                   int buttonsState,
                                    boolean isButtonsDownState,
                                    boolean isMouseMove);
     private native void keyEvent(int javaKeyCode, boolean keydown);

src/java.desktop/macosx/native/libawt_lwawt/awt/CGraphicsConfig.m

@@ -1,50 +0,0 @@
-/*
- * Copyright (c) 2011, 2012, 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.  Oracle designates this
- * particular file as subject to the "Classpath" exception as provided
- * by Oracle in the LICENSE file that accompanied this code.
- *
- * 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.
- */
-
-#include "LWCToolkit.h"
-#include "GeomUtilities.h"
-
-#include "sun_awt_CGraphicsConfig.h"
-#import <JavaNativeFoundation/JavaNativeFoundation.h>
-
-/*
- * Class:     sun_awt_CGraphicsConfig
- * Method:    nativeGetBounds
- * Signature: (I)Ljava/awt/Rectangle;
- */
-JNIEXPORT jobject JNICALL Java_sun_awt_CGraphicsConfig_nativeGetBounds
-(JNIEnv *env, jclass class, jint displayID)
-{
-    jobject jrect = NULL;
-
-JNF_COCOA_ENTER(env);
-
-    CGRect rect = CGDisplayBounds((CGDirectDisplayID)displayID);
-    jrect = CGToJavaRect(env, rect);
-
-JNF_COCOA_EXIT(env);
-
-    return jrect;
-}

src/java.desktop/macosx/native/libawt_lwawt/awt/CGraphicsDevice.m

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2018, 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
@@ -25,9 +25,17 @@
 
 #import "LWCToolkit.h"
 #import "ThreadUtilities.h"
+#include "GeomUtilities.h"
 
 #import <JavaNativeFoundation/JavaNativeFoundation.h>
 
+/**
+ * Some default values for invalid CoreGraphics display ID.
+ */
+#define DEFAULT_DEVICE_WIDTH 1024
+#define DEFAULT_DEVICE_HEIGHT 768
+#define DEFAULT_DEVICE_DPI 72
+
 /*
  * Convert the mode string to the more convinient bits per pixel value
  */
@@ -56,10 +64,12 @@ static BOOL isValidDisplayMode(CGDisplayModeRef mode){
 }
 
 static CFMutableArrayRef getAllValidDisplayModes(jint displayID){
+    // CGDisplayCopyAllDisplayModes can return NULL if displayID is invalid
     CFArrayRef allModes = CGDisplayCopyAllDisplayModes(displayID, NULL);
-
+    CFMutableArrayRef validModes = nil;
+    if (allModes) {
         CFIndex numModes = CFArrayGetCount(allModes);
-    CFMutableArrayRef validModes = CFArrayCreateMutable(kCFAllocatorDefault, numModes + 1, &kCFTypeArrayCallBacks);
+        validModes = CFArrayCreateMutable(kCFAllocatorDefault, numModes + 1, &kCFTypeArrayCallBacks);
 
         CFIndex n;
         for (n=0; n < numModes; n++) {
@@ -70,8 +80,9 @@ static CFMutableArrayRef getAllValidDisplayModes(jint displayID){
         }
         CFRelease(allModes);
 
+        // CGDisplayCopyDisplayMode can return NULL if displayID is invalid
         CGDisplayModeRef currentMode = CGDisplayCopyDisplayMode(displayID);
-
+        if (currentMode) {
             BOOL containsCurrentMode = NO;
             numModes = CFArrayGetCount(validModes);
             for (n=0; n < numModes; n++) {
@@ -80,11 +91,12 @@ static CFMutableArrayRef getAllValidDisplayModes(jint displayID){
                     break;
                 }
             }
-
             if (!containsCurrentMode) {
                 CFArrayAppendValue(validModes, currentMode);
             }
             CGDisplayModeRelease(currentMode);
+        }
+    }
 
     return validModes;
 }
@@ -95,7 +107,7 @@ static CFMutableArrayRef getAllValidDisplayModes(jint displayID){
  */
 static CGDisplayModeRef getBestModeForParameters(CFArrayRef allModes, int w, int h, int bpp, int refrate) {
     CGDisplayModeRef bestGuess = NULL;
-    CFIndex numModes = CFArrayGetCount(allModes), n;
+    CFIndex numModes = allModes ? CFArrayGetCount(allModes) : 0, n;
     int thisBpp = 0;
     for(n = 0; n < numModes; n++ ) {
         CGDisplayModeRef cRef = (CGDisplayModeRef) CFArrayGetValueAtIndex(allModes, n);
@@ -129,18 +141,21 @@ static CGDisplayModeRef getBestModeForParameters(CFArrayRef allModes, int w, int
 }
 
 /*
- * Create a new java.awt.DisplayMode instance based on provided CGDisplayModeRef
+ * Create a new java.awt.DisplayMode instance based on provided
+ * CGDisplayModeRef, if CGDisplayModeRef is NULL, then some stub is returned.
  */
-static jobject createJavaDisplayMode(CGDisplayModeRef mode, JNIEnv *env, jint displayID) {
+static jobject createJavaDisplayMode(CGDisplayModeRef mode, JNIEnv *env) {
     jobject ret = NULL;
-    jint h, w, bpp, refrate;
+    jint h = DEFAULT_DEVICE_HEIGHT, w = DEFAULT_DEVICE_WIDTH, bpp = 0, refrate = 0;
     JNF_COCOA_ENTER(env);
+    if (mode) {
         CFStringRef currentBPP = CGDisplayModeCopyPixelEncoding(mode);
         bpp = getBPPFromModeString(currentBPP);
         refrate = CGDisplayModeGetRefreshRate(mode);
         h = CGDisplayModeGetHeight(mode);
         w = CGDisplayModeGetWidth(mode);
         CFRelease(currentBPP);
+    }
     static JNF_CLASS_CACHE(jc_DisplayMode, "java/awt/DisplayMode");
     static JNF_CTOR_CACHE(jc_DisplayMode_ctor, jc_DisplayMode, "(IIII)V");
     ret = JNFNewObject(env, jc_DisplayMode_ctor, w, h, bpp, refrate);
@@ -163,7 +178,7 @@ Java_sun_awt_CGraphicsDevice_nativeGetXResolution
     CGRect rect = CGDisplayBounds(displayID);
     // 1 inch == 25.4 mm
     jfloat inches = size.width / 25.4f;
-    return inches > 0 ? rect.size.width / inches : 72;
+    return inches > 0 ? rect.size.width / inches : DEFAULT_DEVICE_DPI;
 }
 
 /*
@@ -180,7 +195,26 @@ Java_sun_awt_CGraphicsDevice_nativeGetYResolution
     CGRect rect = CGDisplayBounds(displayID);
     // 1 inch == 25.4 mm
     jfloat inches = size.height / 25.4f;
-    return inches > 0 ? rect.size.height / inches : 72;
+    return inches > 0 ? rect.size.height / inches : DEFAULT_DEVICE_DPI;
+}
+
+/*
+ * Class:     sun_awt_CGraphicsDevice
+ * Method:    nativeGetBounds
+ * Signature: (I)Ljava/awt/Rectangle;
+ */
+JNIEXPORT jobject JNICALL
+Java_sun_awt_CGraphicsDevice_nativeGetBounds
+(JNIEnv *env, jclass class, jint displayID)
+{
+    CGRect rect = CGDisplayBounds(displayID);
+    if (rect.size.width == 0) {
+        rect.size.width = DEFAULT_DEVICE_WIDTH;
+    }
+    if (rect.size.height == 0) {
+        rect.size.height = DEFAULT_DEVICE_HEIGHT;
+    }
+    return CGToJavaRect(env, rect);
 }
 
 /*
@@ -269,8 +303,9 @@ Java_sun_awt_CGraphicsDevice_nativeGetDisplayMode
 (JNIEnv *env, jclass class, jint displayID)
 {
     jobject ret = NULL;
+    // CGDisplayCopyDisplayMode can return NULL if displayID is invalid
     CGDisplayModeRef currentMode = CGDisplayCopyDisplayMode(displayID);
-    ret = createJavaDisplayMode(currentMode, env, displayID);
+    ret = createJavaDisplayMode(currentMode, env);
     CGDisplayModeRelease(currentMode);
     return ret;
 }
@@ -288,7 +323,7 @@ Java_sun_awt_CGraphicsDevice_nativeGetDisplayModes
     JNF_COCOA_ENTER(env);
     CFArrayRef allModes = getAllValidDisplayModes(displayID);
 
-    CFIndex numModes = CFArrayGetCount(allModes);
+    CFIndex numModes = allModes ? CFArrayGetCount(allModes): 0;
     static JNF_CLASS_CACHE(jc_DisplayMode, "java/awt/DisplayMode");
 
     jreturnArray = JNFNewObjectArray(env, &jc_DisplayMode, (jsize) numModes);
@@ -301,7 +336,7 @@ Java_sun_awt_CGraphicsDevice_nativeGetDisplayModes
     for (n=0; n < numModes; n++) {
         CGDisplayModeRef cRef = (CGDisplayModeRef) CFArrayGetValueAtIndex(allModes, n);
         if (cRef != NULL) {
-            jobject oneMode = createJavaDisplayMode(cRef, env, displayID);
+            jobject oneMode = createJavaDisplayMode(cRef, env);
             (*env)->SetObjectArrayElement(env, jreturnArray, n, oneMode);
             if ((*env)->ExceptionOccurred(env)) {
                 (*env)->ExceptionDescribe(env);
@@ -311,7 +346,9 @@ Java_sun_awt_CGraphicsDevice_nativeGetDisplayModes
             (*env)->DeleteLocalRef(env, oneMode);
         }
     }
+    if (allModes) {
         CFRelease(allModes);
+    }
     JNF_COCOA_EXIT(env);
 
     return jreturnArray;

src/java.desktop/macosx/native/libawt_lwawt/awt/CRobot.m

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, 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
@@ -139,8 +139,7 @@ Java_sun_lwawt_macosx_CRobot_initRobot
  */
 JNIEXPORT void JNICALL
 Java_sun_lwawt_macosx_CRobot_mouseEvent
-(JNIEnv *env, jobject peer,
+(JNIEnv *env, jobject peer, jint mouseLastX, jint mouseLastY, jint buttonsState,
- jint displayID, jint mouseLastX, jint mouseLastY, jint buttonsState,
  jboolean isButtonsDownState, jboolean isMouseMove)
 {
     JNF_COCOA_ENTER(env);