[OpenJDK 2D-Dev] RFR: 8054991 : sun.font.GlyphList uses broken double-checked locking

[prasanta sadhukhan]

Hi Phil,


On 5/26/2016 3:48 AM, Phil Race wrote:
> bug : https://bugs.openjdk.java.net/browse/JDK-8054991
>
> There is a comment in there about a proposed patch but I can't find it.
I guess the proposed patch suggested was this:

diff -r 84ff8117acd2 src/share/classes/sun/font/GlyphList.java
--- a/src/share/classes/sun/font/GlyphList.java    Mon Aug 11 09:26:05 
2014 -0400
+++ b/src/share/classes/sun/font/GlyphList.java    Tue Aug 19 08:16:53 
2014 -0400
@@ -29,6 +29,7 @@
  import java.awt.font.GlyphVector;
  import java.awt.font.FontRenderContext;
  import sun.java2d.loops.FontInfo;
+import java.util.concurrent.atomic.AtomicBoolean;

  /*
   * This class represents a list of actual renderable glyphs.
@@ -151,8 +152,8 @@
       * occur and if it did, it would just lead to some extra garbage being
       * created.
       */
-    private static GlyphList reusableGL = new GlyphList();
-    private static boolean inUse;
+    private static final GlyphList reusableGL = new GlyphList();
+    private static final AtomicBoolean inUse = new AtomicBoolean(false);


      void ensureCapacity(int len) {
@@ -184,25 +185,10 @@
  //     }

      public static GlyphList getInstance() {
-        /* The following heuristic is that if the reusable instance is
-         * in use, it probably still will be in a micro-second, so avoid
-         * synchronising on the class and just allocate a new instance.
-         * The cost is one extra boolean test for the normal case, and some
-         * small number of cases where we allocate an extra object when
-         * in fact the reusable one would be freed very soon.
-         */
-        if (inUse) {
+        if (inUse.compareAndSet(false, true))
+            return reusableGL;
+        else
              return new GlyphList();
-        } else {
-            synchronized(GlyphList.class) {
-                if (inUse) {
-                    return new GlyphList();
-                } else {
-                    inUse = true;
-                    return reusableGL;
-                }
-            }
-        }
      }

      /* In some cases the caller may be able to estimate the size of
@@ -423,7 +409,7 @@
              }
              usePositions = false;
              strikelist = null; // remove reference to the strike list
-            inUse = false;
+            inUse.set(false);
          }
      }

Regards
Prasanta
> However I think this will be sufficient :-
>
> diff --git a/src/java.desktop/share/classes/sun/font/GlyphList.java 
> b/src/java.desktop/share/classes/sun/font/GlyphList.java
> --- a/src/java.desktop/share/classes/sun/font/GlyphList.java
> +++ b/src/java.desktop/share/classes/sun/font/GlyphList.java
> @@ -152,7 +152,7 @@
>       * created.
>       */
>      private static GlyphList reusableGL = new GlyphList();
> -    private static boolean inUse;
> +    private static volatile boolean inUse;
>
> -phil.