[OpenJDK 2D-Dev] [14] Review Request: 8229810 NullPointerException getting bounds of GraphicsConfiguration

[Jayathirth Rao]

Hi Sergey,

I think we should drive device update through GraphicsEnvironment.displayChanged() and DisplayChangeListener instead of force updating bounds in GraphicsDevice.displayChanged in the constructor.
We can synchronise the GraphicsEnvironment.displayChanged() by calling it under CGraphicsEnvironment.initDevices() sync lock, this will force call DisplayChangeListener when devices are updated.

--- a/src/java.desktop/macosx/classes/sun/awt/CGraphicsEnvironment.java Thu Oct 03 00:38:17 2019 +0300
+++ b/src/java.desktop/macosx/classes/sun/awt/CGraphicsEnvironment.java Thu Oct 03 13:16:12 2019 +0530
@@ -183,8 +183,8 @@
                 devices.put(id, old.containsKey(id) ? old.get(id)
                                                     : new CGraphicsDevice(id));
             }
+            displayChanged();
         }
-        displayChanged();
     }

I made this change and tested with reproduction steps and I dont see NPE.

Thanks,
Jay

> On 03-Oct-2019, at 11:09 AM, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
> 
> Any volunteers to review=)?
> 
> On 8/19/19 12:03 pm, Sergey Bylokhov wrote:
>> Hello.
>> Please review the fix for JDK 14.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8229810
>> Fix: http://cr.openjdk.java.net/~serb/8229810/webrev.00
>> The root cause is a lack of synchronization in CGraphicsEnvironment,
>> we create the GraphicsDevice under a special lock which is used to
>> access the list of devices:
>> http://hg.openjdk.java.net/jdk/client/file/39f133168348/src/java.desktop/macosx/classes/sun/awt/CGraphicsEnvironment.java#l184
>> but initialize the device w/o this lock.
>> http://hg.openjdk.java.net/jdk/client/file/39f133168348/src/java.desktop/macosx/classes/sun/awt/CGraphicsEnvironment.java#l187
>> So it is possible that we create the device, add it to the list of devices,
>> and before we initialize it the user could access the not fully initialized object.
>> The bug exists from the jdk7, but it was hidden because all fields in CGraphicsDevice
>> were primitives, so the app just used invalid data. In JDK-8211992 the object field "bounds"
>> was added and now we can get NPE if this field was not initialized.
>> As a fix, I suggest to always initialize the graphics device in the constructor.
>> Note that we cannot move "displayChanged" which will notify devices and all listeners
>> in awt/swing in the CGraphicsEnvironment under the lock, it will cause various
>> deadlocks in the a different listeners.
> 
> 
> -- 
> Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20191003/2c30141a/attachment.html>