JNI crashes in FontManager code

[Martin Buchholz]

Hi font maintainers,

This is a bug report, with fix,
provided by Hiroshi Yamauchi.
This is a crash detected by a google test
(which is unfortunately impractical to share)
which makes this at least a P2 bug.

Description:

	Fixing the bad JNI code in the font manager code. Two issues:
	
	  o The JNIEnv is unique to the thread. It cannot be saved by one thread and
	    reused by another. Use GetEnv instead.
	
	  o The 'font2D' jobject needs to be converted into a global reference because
	    its lifetime exceeds the lifetime of a native method call.
	
Evaluation:

Appropriately register/free everything with the garbage collector.

Fix:

# HG changeset patch
# User martin
# Date 1224202830 25200
# Node ID 3c9d6001d8a90698a3540a2a483717f26a98db78
# Parent  68730f05449cd4f39ce1cb82adc6c4e57f87554f
Crash in freetypeScaler.c due to insufficient GC protection
Summary: NewGlobalRef/DeleteGlobalRef as needed.
Reviewed-by:
Contributed-by: yamauchi at google.com

diff --git a/make/sun/font/mapfile-vers.openjdk
b/make/sun/font/mapfile-vers.openjdk
--- a/make/sun/font/mapfile-vers.openjdk
+++ b/make/sun/font/mapfile-vers.openjdk
@@ -29,6 +29,7 @@

 SUNWprivate_1.1 {
 	global:
+                JNI_OnLoad;
                 getSunFontIDs;
                 newLayoutTableCache;
                 freeLayoutTableCache;
diff --git a/src/share/native/sun/font/freetypeScaler.c
b/src/share/native/sun/font/freetypeScaler.c
--- a/src/share/native/sun/font/freetypeScaler.c
+++ b/src/share/native/sun/font/freetypeScaler.c
@@ -48,16 +48,6 @@
 #define  ROUND(x) ((int) (x+0.5))

 typedef struct {
-    /* Important note:
-         JNI forbids sharing same env between different threads.
-         We are safe, because pointer is overwritten every time we get into
-         JNI call (see setupFTContext).
-
-         Pointer is used by font data reading callbacks
-         such as ReadTTFontFileFunc.
-
-         NB: We may consider switching to JNI_GetEnv. */
-    JNIEnv* env;
     FT_Library library;
     FT_Face face;
     jobject font2D;
@@ -90,6 +80,13 @@
 void z_error(char *s) {}
 #endif

+static JavaVM* jvm = NULL;
+
+JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) {
+    jvm = vm;
+    return JNI_VERSION_1_2;
+}
+
 /**************** Error handling utilities *****************/

 static jmethodID invalidateScalerMID;
@@ -107,6 +104,10 @@

     FT_Done_Face(scalerInfo->face);
     FT_Done_FreeType(scalerInfo->library);
+
+    if (scalerInfo->font2D != NULL) {
+        (*env)->DeleteGlobalRef(env, scalerInfo->font2D);
+    }

     if (scalerInfo->directBuffer != NULL) {
         (*env)->DeleteGlobalRef(env, scalerInfo->directBuffer);
@@ -131,10 +132,9 @@

 #define FILEDATACACHESIZE 1024

-/* NB: is it ever called? */
 static void CloseTTFontFileFunc(FT_Stream stream) {
+    JNIEnv* env = (JNIEnv*) JNU_GetEnv(jvm, JNI_VERSION_1_2);
     FTScalerInfo *scalerInfo = (FTScalerInfo *) stream->pathname.pointer;
-    JNIEnv* env = scalerInfo->env;
     jclass tmpClass = (*env)->FindClass(env, "sun/font/TrueTypeFont");
     jfieldID platNameField =
          (*env)->GetFieldID(env, tmpClass, "platName", "Ljava/lang/String;");
@@ -150,8 +150,8 @@
                                         unsigned char* destBuffer,
                                         unsigned long numBytes)
 {
+    JNIEnv* env = (JNIEnv*) JNU_GetEnv(jvm, JNI_VERSION_1_2);
     FTScalerInfo *scalerInfo = (FTScalerInfo *) stream->pathname.pointer;
-    JNIEnv* env = scalerInfo->env;
     jobject bBuffer;
     int bread = 0;

@@ -245,8 +245,7 @@
     if (scalerInfo == NULL)
         return 0;

-    scalerInfo->env = env;
-    scalerInfo->font2D = font2D;
+    scalerInfo->font2D = (*env)->NewGlobalRef(env, font2D);
     scalerInfo->fontDataOffset = 0;
     scalerInfo->fontDataLength = 0;
     scalerInfo->fileSize = filesize;
@@ -263,6 +262,7 @@
     */
     error = FT_Init_FreeType(&scalerInfo->library);
     if (error) {
+        (*env)->DeleteGlobalRef(env, scalerInfo->font2D);
         free(scalerInfo);
         return 0;
     }
@@ -331,6 +331,7 @@
         }
         if (scalerInfo->fontData != NULL)
             free(scalerInfo->fontData);
+        (*env)->DeleteGlobalRef(env, scalerInfo->font2D);
         free(scalerInfo);
         return 0;
     }
@@ -391,8 +392,10 @@
                           FTScalerContext *context) {
     int errCode = 0;

-    scalerInfo->env = env;
-    scalerInfo->font2D = font2D;
+    if (scalerInfo->font2D != NULL) {
+        (*env)->DeleteGlobalRef(env, scalerInfo->font2D);
+    }
+    scalerInfo->font2D = (*env)->NewGlobalRef(env, font2D);

     FT_Set_Transform(scalerInfo->face, &context->transform, NULL);