6899078: potential deadlock and performance issue in freeing strike resources with D3D pipeline

Reviewed-by: tdv, igor

jdk/src/share/classes/sun/font/Font2D.java

@@ -320,21 +320,6 @@ public abstract class Font2D {
                     lastFontStrike = new SoftReference(strike);
                     StrikeCache.refStrike(strike);
                     return strike;
-                } else {
-                    /* We have found a cleared reference that has not yet
-                     * been removed by the disposer.
-                     * If we make this reference unreachable by removing it
-                     * from the map,or overwriting it with a new reference to
-                     * a new strike, then it is possible it may never be
-                     * enqueued for disposal. That is the implication of
-                     * the docs for java.lang.ref. So on finding a cleared
-                     * reference, we need to dispose the native resources,
-                     * if they haven't already been freed.
-                     * The reference object needs to have a reference to
-                     * the disposer instance for this to occur.
-                     */
-                  ((StrikeCache.DisposableStrike)strikeRef)
-                      .getDisposer().dispose();
                 }
             }
             /* When we create a new FontStrike instance, we *must*

jdk/src/share/classes/sun/font/FontDesignMetrics.java

@@ -171,7 +171,7 @@ public final class FontDesignMetrics extends FontMetrics {
      * out we can clear the keys from the table.
      */
     private static class KeyReference extends SoftReference
-        implements DisposerRecord {
+        implements DisposerRecord, Disposer.PollDisposable {
 
         static ReferenceQueue queue = Disposer.getQueue();
 

jdk/src/share/classes/sun/font/FontStrikeDisposer.java

@@ -25,6 +25,7 @@
 
 package sun.font;
 
+import sun.java2d.Disposer;
 import sun.java2d.DisposerRecord;
 
 /*
@@ -49,7 +50,8 @@ import sun.java2d.DisposerRecord;
  * entries would be removed much more promptly than we need.
  */
 
-class FontStrikeDisposer implements DisposerRecord {
+class FontStrikeDisposer
+    implements DisposerRecord, Disposer.PollDisposable {
 
     Font2D font2D;
     FontStrikeDesc desc;

jdk/src/share/classes/sun/font/StrikeCache.java

@@ -254,9 +254,20 @@ public final class StrikeCache {
         // because they may be accessed on that thread at the time of the
         // disposal (for example, when the accel. cache is invalidated)
 
-        // REMIND: this look a bit heavyweight, but should be ok
-        // because strike disposal is a relatively infrequent operation,
-        // more worrisome is the necessity of getting a GC here.
+        // Whilst this is a bit heavyweight, in most applications
+        // strike disposal is a relatively infrequent operation, so it
+        // doesn't matter. But in some tests that use vast numbers
+        // of strikes, the switching back and forth is measurable.
+        // So the "pollRemove" call is added to batch up the work.
+        // If we are polling we know we've already been called back
+        // and can directly dispose the record.
+        // Also worrisome is the necessity of getting a GC here.
+
+        if (Disposer.pollingQueue) {
+            doDispose(disposer);
+            return;
+        }
+
         RenderQueue rq = null;
         GraphicsEnvironment ge =
             GraphicsEnvironment.getLocalGraphicsEnvironment();
@@ -277,6 +288,7 @@ public final class StrikeCache {
                 rq.flushAndInvokeNow(new Runnable() {
                     public void run() {
                         doDispose(disposer);
+                        Disposer.pollRemove();
                     }
                 });
             } finally {

jdk/src/share/classes/sun/java2d/Disposer.java

@@ -29,6 +29,7 @@ import java.lang.ref.Reference;
 import java.lang.ref.ReferenceQueue;
 import java.lang.ref.PhantomReference;
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 import java.util.Hashtable;
 
 /**
@@ -146,6 +147,7 @@ public class Disposer implements Runnable {
                 rec.dispose();
                 obj = null;
                 rec = null;
+                clearDeferredRecords();
             } catch (Exception e) {
                 System.out.println("Exception while removing reference: " + e);
                 e.printStackTrace();
@@ -153,6 +155,85 @@ public class Disposer implements Runnable {
         }
     }
 
+    /*
+     * This is a marker interface that, if implemented, means it
+     * doesn't acquire any special locks, and is safe to
+     * be disposed in the poll loop on whatever thread
+     * which happens to be the Toolkit thread, is in use.
+     */
+    public static interface PollDisposable {
+    };
+
+    private static ArrayList<DisposerRecord> deferredRecords = null;
+
+    private static void clearDeferredRecords() {
+        if (deferredRecords == null || deferredRecords.isEmpty()) {
+            return;
+        }
+        for (int i=0;i<deferredRecords.size(); i++) {
+            try {
+                DisposerRecord rec = deferredRecords.get(i);
+                rec.dispose();
+            } catch (Exception e) {
+                System.out.println("Exception while disposing deferred rec.");
+                e.printStackTrace();
+            }
+        }
+        deferredRecords.clear();
+    }
+
+    /*
+     * Set to indicate the queue is presently being polled.
+     */
+    public static volatile boolean pollingQueue = false;
+
+    /*
+     * The pollRemove() method is called back from a dispose method
+     * that is running on the toolkit thread and wants to
+     * dispose any pending refs that are safe to be disposed
+     * on that thread.
+     */
+    public static void pollRemove() {
+
+        /* This should never be called recursively, so this check
+         * is just a safeguard against the unexpected.
+         */
+        if (pollingQueue) {
+            return;
+        }
+        Object obj;
+        pollingQueue = true;
+        int freed = 0;
+        int deferred = 0;
+        try {
+            while ((obj = queue.poll()) != null
+                   && freed < 10000 && deferred < 100) {
+                freed++;
+                ((Reference)obj).clear();
+                DisposerRecord rec = (DisposerRecord)records.remove(obj);
+                if (rec instanceof PollDisposable) {
+                    rec.dispose();
+                    obj = null;
+                    rec = null;
+                } else {
+                    if (rec == null) { // shouldn't happen, but just in case.
+                        continue;
+                    }
+                    deferred++;
+                    if (deferredRecords == null) {
+                      deferredRecords = new ArrayList<DisposerRecord>(5);
+                    }
+                    deferredRecords.add(rec);
+                }
+            }
+        } catch (Exception e) {
+            System.out.println("Exception while removing reference: " + e);
+            e.printStackTrace();
+        } finally {
+            pollingQueue = false;
+        }
+    }
+
     private static native void initIDs();
 
     /*