6640532: Graphics.getFontMetrics() throws NullPointerException

Summary: NIO usage needs to be robust against Thread.interrupt()
Reviewed-by: tdv

src/share/classes/sun/font/FontManager.java

@@ -93,7 +93,6 @@ public final class FontManager {
      */
     private static final int CHANNELPOOLSIZE = 20;
     private static int lastPoolIndex = 0;
-    private static int poolSize = 0;
     private static FileFont fontFileCache[] = new FileFont[CHANNELPOOLSIZE];
 
     /* Need to implement a simple linked list scheme for fast
@@ -283,29 +282,32 @@ public final class FontManager {
     private static native void initIDs();
 
     public static void addToPool(FileFont font) {
-        boolean added = false;
+
+        FileFont fontFileToClose = null;
+        int freeSlot = -1;
+
         synchronized (fontFileCache) {
-            /* use poolSize to quickly detect if there's any free slots.
-             * This is a performance tweak based on the assumption that
-             * if this is executed at all often, its because there are many
-             * fonts being used and the pool will be full, and we will save
-             * a fruitless iteration
+            /* Avoid duplicate entries in the pool, and don't close() it,
+             * since this method is called only from within open().
+             * Seeing a duplicate is most likely to happen if the thread
+             * was interrupted during a read, forcing perhaps repeated
+             * close and open calls and it eventually it ends up pointing
+             * at the same slot.
              */
-            if (poolSize < CHANNELPOOLSIZE) {
-                for (int i=0; i<CHANNELPOOLSIZE; i++) {
-                    if (fontFileCache[i] == null) {
-                        fontFileCache[i] = font;
-                        poolSize++;
-                        added = true;
-                        break;
-                    }
+            for (int i=0;i<CHANNELPOOLSIZE;i++) {
+                if (fontFileCache[i] == font) {
+                    return;
+                }
+                if (fontFileCache[i] == null && freeSlot < 0) {
+                    freeSlot = i;
                 }
-                assert added;
+            }
+            if (freeSlot >= 0) {
+                fontFileCache[freeSlot] = font;
+                return;
             } else {
-                // is it possible for this to be the same font?
-                assert fontFileCache[lastPoolIndex] != font;
-                /* replace with new font,  poolSize is unchanged. */
-                fontFileCache[lastPoolIndex].close();
+                /* replace with new font. */
+                fontFileToClose = fontFileCache[lastPoolIndex];
                 fontFileCache[lastPoolIndex] = font;
                 /* lastPoolIndex is updated so that the least recently opened
                  * file will be closed next.
@@ -313,6 +315,19 @@ public final class FontManager {
                 lastPoolIndex = (lastPoolIndex+1) % CHANNELPOOLSIZE;
             }
         }
+        /* Need to close the font file outside of the synchronized block,
+         * since its possible some other thread is in an open() call on
+         * this font file, and could be holding its lock and the pool lock.
+         * Releasing the pool lock allows that thread to continue, so it can
+         * then release the lock on this font, allowing the close() call
+         * below to proceed.
+         * Also, calling close() is safe because any other thread using
+         * the font we are closing() synchronizes all reading, so we
+         * will not close the file while its in use.
+         */
+        if (fontFileToClose != null) {
+            fontFileToClose.close();
+        }
     }
 
     /*
@@ -334,7 +349,6 @@ public final class FontManager {
             for (int i=0; i<CHANNELPOOLSIZE; i++) {
                 if (fontFileCache[i] == font) {
                     fontFileCache[i] = null;
-                    poolSize--;
                 }
             }
         }

test/java/awt/font/Threads/FontThread.java

+/*
+ * Copyright 2007 Sun Microsystems, Inc.  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 Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+
+/* @test
+ * @summary verify thread interruption doesn't affect font file reading.
+ * @bug 6640532
+ */
+
+import java.awt.*;
+
+public class FontThread extends Thread {
+
+    String fontName = "Dialog";
+    static FontThread thread1;
+    static FontThread thread2;
+    static FontThread thread3;
+
+    public static void main(String args[]) throws Exception {
+        thread1 = new FontThread("SansSerif");
+        thread2 = new FontThread("Serif");
+        thread3 = new FontThread("Monospaced");
+        thread1.dometrics(60); // load classes first
+        thread1.start();
+        thread2.start();
+        thread3.start();
+        InterruptThread ithread = new InterruptThread();
+        ithread.setDaemon(true);
+        ithread.start();
+        thread1.join();
+        thread2.join();
+        thread3.join();
+    }
+
+    FontThread(String font) {
+        super();
+        this.fontName = font;
+    }
+
+    public void run() {
+        System.out.println("started "+fontName); System.out.flush();
+        dometrics(4000);
+        System.out.println("done "+fontName); System.out.flush();
+    }
+
+    private void dometrics(int max) {
+        Font f = new Font(fontName, Font.PLAIN, 12);
+        FontMetrics fm = Toolkit.getDefaultToolkit().getFontMetrics(f);
+        for (char i=0;i<max;i++) {
+            if (f.canDisplay(i)) fm.charWidth(i);
+        }
+    }
+
+    static class InterruptThread extends Thread {
+        public void run() {
+            while (true) {
+                try {
+                    Thread.sleep(1);
+                } catch (InterruptedException e) {
+                }
+                thread1.interrupt();
+                thread2.interrupt();
+                thread3.interrupt();
+            }
+        }
+    }
+}
+
+