8220186: Improve use of font temporary files
authorprr
Tue, 23 Apr 2019 11:59:54 -0700
changeset 58613 eb09ad30eccb
parent 58612 32aff2b7585b
child 58614 29624901d8bc
8220186: Improve use of font temporary files Reviewed-by: serb, psadhukhan, mschoene, rhalade
src/java.desktop/share/classes/sun/font/FileFont.java
src/java.desktop/share/classes/sun/font/FontScaler.java
src/java.desktop/share/classes/sun/font/FreetypeFontScaler.java
src/java.desktop/share/native/libfontmanager/freetypeScaler.c
--- a/src/java.desktop/share/classes/sun/font/FileFont.java	Mon Apr 22 13:01:57 2019 +0800
+++ b/src/java.desktop/share/classes/sun/font/FileFont.java	Tue Apr 23 11:59:54 2019 -0700
@@ -172,7 +172,7 @@
             }
         }
         if (scaler != null) {
-            scaler.dispose();
+            scaler.disposeScaler();
         }
         scaler = FontScaler.getNullScaler();
     }
--- a/src/java.desktop/share/classes/sun/font/FontScaler.java	Mon Apr 22 13:01:57 2019 +0800
+++ b/src/java.desktop/share/classes/sun/font/FontScaler.java	Tue Apr 23 11:59:54 2019 -0700
@@ -173,6 +173,12 @@
              scaler context objects! */
     public void dispose() {}
 
+    /**
+     * Used when the native resources held by the scaler need
+     * to be released before the 2D disposer runs.
+     */
+    public void disposeScaler() {}
+
     /* At the moment these 3 methods are needed for Type1 fonts only.
      * For Truetype fonts we extract required info outside of scaler
      * on java layer.
--- a/src/java.desktop/share/classes/sun/font/FreetypeFontScaler.java	Mon Apr 22 13:01:57 2019 +0800
+++ b/src/java.desktop/share/classes/sun/font/FreetypeFontScaler.java	Tue Apr 23 11:59:54 2019 -0700
@@ -163,6 +163,9 @@
             .getNullScaler().getGlyphVectorOutline(0L, glyphs, numGlyphs, x, y);
     }
 
+    /* This method should not be called directly, in case
+     * it is being invoked from a thread with a native context.
+     */
     public synchronized void dispose() {
         if (nativeScaler != 0L) {
             disposeNativeScaler(font.get(), nativeScaler);
@@ -170,6 +173,21 @@
         }
     }
 
+    public synchronized void disposeScaler() {
+        if (nativeScaler != 0L) {
+           /*
+            * The current thread may be calling this method from the context
+            * of a JNI up-call. It will hold the native lock from the
+            * original down-call so can directly enter dispose and free
+            * the resources. So we need to schedule the disposal to happen
+            * only once we've returned from native. So by running the dispose
+            * on another thread which does nothing except that disposal we
+            * are sure that this is safe.
+            */
+            new Thread(null, () -> dispose(), "free scaler", 0, false).start();
+        }
+    }
+
     synchronized int getNumGlyphs() throws FontScalerException {
         if (nativeScaler != 0L) {
             return getNumGlyphsNative(nativeScaler);
@@ -206,7 +224,7 @@
         return getUnitsPerEMNative(nativeScaler);
     }
 
-    long createScalerContext(double[] matrix,
+    synchronized long createScalerContext(double[] matrix,
             int aa, int fm, float boldness, float italic,
             boolean disableHinting) {
         if (nativeScaler != 0L) {
@@ -236,7 +254,7 @@
     private native GeneralPath getGlyphVectorOutlineNative(Font2D font,
             long pScalerContext, long pScaler,
             int[] glyphs, int numGlyphs, float x, float y);
-    native Point2D.Float getGlyphPointNative(Font2D font,
+    private native Point2D.Float getGlyphPointNative(Font2D font,
             long pScalerContext, long pScaler, int glyphCode, int ptNumber);
 
     private native void disposeNativeScaler(Font2D font2D, long pScaler);
@@ -247,7 +265,7 @@
 
     private native long getUnitsPerEMNative(long pScaler);
 
-    native long createScalerContextNative(long pScaler, double[] matrix,
+    private native long createScalerContextNative(long pScaler, double[] matrix,
             int aa, int fm, float boldness, float italic);
 
     /* Freetype scaler context does not contain any pointers that
--- a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c	Mon Apr 22 13:01:57 2019 +0800
+++ b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c	Tue Apr 23 11:59:54 2019 -0700
@@ -154,7 +154,31 @@
     jobject bBuffer;
     int bread = 0;
 
-    if (numBytes == 0) return 0;
+    /* A call with numBytes == 0 is a seek. It should return 0 if the
+     * seek position is within the file and non-zero otherwise.
+     * For all other cases, ie numBytes !=0, return the number of bytes
+     * actually read. This applies to truncated reads and also failed reads.
+     */
+
+    if (numBytes == 0) {
+        if (offset >= scalerInfo->fileSize) {
+            return -1;
+        } else {
+            return 0;
+       }
+    }
+
+    if (offset + numBytes < offset) {
+        return 0; // ft should not do this, but just in case.
+    }
+
+    if (offset >= scalerInfo->fileSize) {
+        return 0;
+    }
+
+    if (offset + numBytes > scalerInfo->fileSize) {
+        numBytes = scalerInfo->fileSize - offset;
+    }
 
     /* Large reads will bypass the cache and data copying */
     if (numBytes > FILEDATACACHESIZE) {
@@ -164,7 +188,11 @@
                                           scalerInfo->font2D,
                                           sunFontIDs.ttReadBlockMID,
                                           bBuffer, offset, numBytes);
-            return bread;
+            if (bread < 0) {
+                return 0;
+            } else {
+               return bread;
+            }
         } else {
             /* We probably hit bug 4845371. For reasons that
              * are currently unclear, the call stacks after the initial
@@ -179,9 +207,18 @@
             (*env)->CallObjectMethod(env, scalerInfo->font2D,
                                      sunFontIDs.ttReadBytesMID,
                                      offset, numBytes);
-            (*env)->GetByteArrayRegion(env, byteArray,
-                                       0, numBytes, (jbyte*)destBuffer);
-            return numBytes;
+            /* If there's an OutofMemoryError then byteArray will be null */
+            if (byteArray == NULL) {
+                return 0;
+            } else {
+                jsize len = (*env)->GetArrayLength(env, byteArray);
+                if (len < numBytes) {
+                    numBytes = len; // don't get more bytes than there are ..
+                }
+                (*env)->GetByteArrayRegion(env, byteArray,
+                                           0, numBytes, (jbyte*)destBuffer);
+                return numBytes;
+            }
         }
     } /* Do we have a cache hit? */
       else if (scalerInfo->fontDataOffset <= offset &&
@@ -203,6 +240,11 @@
                                       sunFontIDs.ttReadBlockMID,
                                       bBuffer, offset,
                                       scalerInfo->fontDataLength);
+        if (bread <= 0) {
+            return 0;
+        } else if (bread < numBytes) {
+           numBytes = bread;
+        }
         memcpy(destBuffer, scalerInfo->fontData, numBytes);
         return numBytes;
     }