8138764: In some cases the usage of TreeLock can be replaced by other synchronization
authorserb
Wed, 21 Oct 2015 18:32:56 +0300
changeset 33510 1895624f2983
parent 33509 fad099f1505e
child 33511 35ac6c6f6b6a
8138764: In some cases the usage of TreeLock can be replaced by other synchronization Reviewed-by: alexp, alexsch
jdk/src/java.desktop/share/classes/java/awt/Component.java
jdk/src/java.desktop/share/classes/java/awt/Window.java
jdk/src/java.desktop/share/classes/sun/swing/CachedPainter.java
jdk/test/java/awt/Component/TreeLockDeadlock/TreeLockDeadlock.java
--- a/jdk/src/java.desktop/share/classes/java/awt/Component.java	Tue Oct 20 22:46:29 2015 +0300
+++ b/jdk/src/java.desktop/share/classes/java/awt/Component.java	Wed Oct 21 18:32:56 2015 +0300
@@ -312,7 +312,7 @@
      * @see GraphicsConfiguration
      * @see #getGraphicsConfiguration
      */
-    private transient GraphicsConfiguration graphicsConfig = null;
+    private transient volatile GraphicsConfiguration graphicsConfig;
 
     /**
      * A reference to a <code>BufferStrategy</code> object
@@ -1143,9 +1143,7 @@
      * @since 1.3
      */
     public GraphicsConfiguration getGraphicsConfiguration() {
-        synchronized(getTreeLock()) {
-            return getGraphicsConfiguration_NoClientCode();
-        }
+        return getGraphicsConfiguration_NoClientCode();
     }
 
     final GraphicsConfiguration getGraphicsConfiguration_NoClientCode() {
--- a/jdk/src/java.desktop/share/classes/java/awt/Window.java	Tue Oct 20 22:46:29 2015 +0300
+++ b/jdk/src/java.desktop/share/classes/java/awt/Window.java	Wed Oct 21 18:32:56 2015 +0300
@@ -347,7 +347,7 @@
      * @see #getOpacity()
      * @since 1.7
      */
-    private float opacity = 1.0f;
+    private volatile float opacity = 1.0f;
 
     /**
      * The shape assigned to this window. This field is set to {@code null} if
@@ -1040,9 +1040,7 @@
             closeSplashScreen();
             Dialog.checkShouldBeBlocked(this);
             super.show();
-            synchronized (getTreeLock()) {
-                this.locationByPlatform = false;
-            }
+            locationByPlatform = false;
             for (int i = 0; i < ownedWindowList.size(); i++) {
                 Window child = ownedWindowList.elementAt(i).get();
                 if ((child != null) && child.showWithParent) {
@@ -1115,9 +1113,7 @@
             modalBlocker.unblockWindow(this);
         }
         super.hide();
-        synchronized (getTreeLock()) {
-            this.locationByPlatform = false;
-        }
+        locationByPlatform = false;
     }
 
     final void clearMostRecentFocusOwnerOnHide() {
@@ -3411,7 +3407,7 @@
         return super.canContainFocusOwner(focusOwnerCandidate) && isFocusableWindow();
     }
 
-    private boolean locationByPlatform = locationByPlatformProp;
+    private volatile boolean locationByPlatform = locationByPlatformProp;
 
 
     /**
@@ -3482,9 +3478,7 @@
      * @since 1.5
      */
     public boolean isLocationByPlatform() {
-        synchronized (getTreeLock()) {
-            return locationByPlatform;
-        }
+        return locationByPlatform;
     }
 
     /**
@@ -3573,9 +3567,7 @@
      * @since 1.7
      */
     public float getOpacity() {
-        synchronized (getTreeLock()) {
-            return opacity;
-        }
+        return opacity;
     }
 
     /**
--- a/jdk/src/java.desktop/share/classes/sun/swing/CachedPainter.java	Tue Oct 20 22:46:29 2015 +0300
+++ b/jdk/src/java.desktop/share/classes/sun/swing/CachedPainter.java	Wed Oct 21 18:32:56 2015 +0300
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2006, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2015, 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
@@ -53,9 +53,7 @@
  */
 public abstract class CachedPainter {
     // CacheMap maps from class to ImageCache.
-    private static final Map<Object,ImageCache> cacheMap =
-                   new HashMap<Object,ImageCache>();
-
+    private static final Map<Object,ImageCache> cacheMap = new HashMap<>();
 
     private static ImageCache getCache(Object key) {
         synchronized(CachedPainter.class) {
@@ -96,20 +94,8 @@
         if (w <= 0 || h <= 0) {
             return;
         }
-        if (c != null) {
-            synchronized(c.getTreeLock()) {
-                synchronized(CachedPainter.class) {
-                    // If c is non-null, synchronize on the tree lock.
-                    // This is necessary because asking for the
-                    // GraphicsConfiguration will grab a tree lock.
-                    paint0(c, g, x, y, w, h, args);
-                }
-            }
-        }
-        else {
-            synchronized(CachedPainter.class) {
-                paint0(c, g, x, y, w, h, args);
-            }
+        synchronized (CachedPainter.class) {
+            paint0(c, g, x, y, w, h, args);
         }
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/awt/Component/TreeLockDeadlock/TreeLockDeadlock.java	Wed Oct 21 18:32:56 2015 +0300
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2015, 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.Frame;
+import java.awt.GraphicsConfiguration;
+import java.awt.Window;
+
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.NANOSECONDS;
+
+/**
+ * @test
+ * @bug 8138764
+ */
+public final class TreeLockDeadlock extends Frame {
+
+    @Override
+    public synchronized GraphicsConfiguration getGraphicsConfiguration() {
+        return super.getGraphicsConfiguration();
+    }
+
+    @Override
+    public synchronized void reshape(int x, int y, int width, int height) {
+        super.reshape(x, y, width, height);
+    }
+
+    @Override
+    public synchronized float getOpacity() {
+        return super.getOpacity();
+    }
+
+    public static void main(final String[] args) throws Exception {
+        final Window window = new TreeLockDeadlock();
+        window.setSize(300, 300);
+        test(window);
+    }
+
+    private static void test(final Window window) throws Exception {
+        final long start = System.nanoTime();
+        final long end = start + NANOSECONDS.convert(1, MINUTES);
+
+        final Runnable r1 = () -> {
+            while (System.nanoTime() < end) {
+                window.setBounds(window.getBounds());
+            }
+        };
+        final Runnable r2 = () -> {
+            while (System.nanoTime() < end) {
+                window.getGraphicsConfiguration();
+                window.getOpacity();
+            }
+        };
+
+        final Thread t1 = new Thread(r1);
+        final Thread t2 = new Thread(r1);
+        final Thread t3 = new Thread(r2);
+        final Thread t4 = new Thread(r2);
+
+        t1.start();
+        t2.start();
+        t3.start();
+        t4.start();
+        t1.join();
+        t2.join();
+        t3.join();
+        t4.join();
+    }
+}