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

[Sergey Bylokhov]

On 30/10/2018 09:59, Phil Race wrote:
> 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 ?

I was not able to reproduce this bug on the multimonitor config,
but in a single monitor the bug is reproduced when I switch
the user while the program is executed.

For the test purpose and to prove that the updated methods will not crash,
I have hardcoded the wrong display id at the end of the constructor
of the GraphicsDevice, and run our tests.

> 
> 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.
>>
> 


-- 
Best regards, Sergey.