[OpenJDK 2D-Dev] [12] Review Request: 8211992 GraphicsConfiguration.getDevice().getDisplayMode() causes JVM crash on Mac

[Phil Race]

Looks reasonable. A test is difficult, but I presume you did some testing.
Can you report the scenarios you tested and on which OS version ?
eg,

Did you try remove/add/remove, or add/remove/add ?
Did you try wake from sleep ?
Did you try removing a monitor whilst it was sleeping and then waking ?

All whilst a Java app was running of course - on the monitor being removed.

-phil.

On 10/15/18 6:11 PM, Sergey Bylokhov wrote:
> Hello.
> Please review the fix for jdk 12.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8211992
> Webrev: http://cr.openjdk.java.net/~serb/8211992/webrev.00
>
> Short description:
>   The root cause is how we use CoreGraphics display ID. This 
> identifier can become non-valid at any time therefore methods, which 
> is using this id should be ready to it. And this bug found a few 
> places which does not take care about the rule above.
>
> Long description:
>   In the CGraphicsDevice class we maintain the ID for the native 
> screen. This ID usually changed when the user adds/removes monitors to 
> the system. Since using invalid id can cause NULL result in the native 
> code we tries to workaround it by two steps:
>
>   Step (1) If the monitor is removed from the system, then we reassign 
> its displayid to the primary screen. So if the user will holds the 
> reference to this GDevice, it will use the MainDisplay. But note that 
> if the user will remove the main screen again then the old screen, 
> which were removed previously, will not be updated(but will use an 
> invalid displayid).
>
>   Step (2) Some of the native methods in CGraphicsDevice class are 
> ready to NULL and provide some meaningful defaults, like 72 dpi. But 
> some others methods are not ready for that, like 
> nativeGetDisplayMode/etc.
>
> Fix description:
>  - I have minimized/dropped the usage of displayid outside of 
> CGraphicsDevice class, in CRobot it was unused, and the code from 
> CGraphicsConfig.m was moved to CGraphicsDevice.m.
>  - In CGraphicsEnvironment class we now maintain the list of old 
> devices, which is updated for all _displayReconfiguration events. This 
> is the same implementation as on windows in Win32GraphicsEnvironment.java
>  - I have added some default values to all native methods also. Just 
> to cover the rare case when we call the method and displayid became 
> invalid at the same moment.
>