6925710: IndexColorModel.finalize can be made to double free

Reviewed-by: igor, prr, hawtin

src/share/classes/java/awt/image/IndexColorModel.java

@@ -129,6 +129,8 @@ public class IndexColorModel extends ColorModel {
     private boolean allgrayopaque;
     private BigInteger validBits;
 
+    private sun.awt.image.BufImgSurfaceData.ICMColorData colorData = null;
+
     private static int[] opaqueBits = {8, 8, 8};
     private static int[] alphaBits = {8, 8, 8, 8};
 
@@ -1511,7 +1513,6 @@ public class IndexColorModel extends ColorModel {
      * longer referenced.
      */
     public void finalize() {
-        sun.awt.image.BufImgSurfaceData.freeNativeICMData(this);
     }
 
     /**

src/share/classes/sun/awt/image/BufImgSurfaceData.java

@@ -49,7 +49,7 @@ public class BufImgSurfaceData extends SurfaceData {
     private BufferedImageGraphicsConfig graphicsConfig;
     RenderLoops solidloops;
 
-    private static native void initIDs(Class ICM);
+    private static native void initIDs(Class ICM, Class ICMColorData);
 
     private static final int DCM_RGBX_RED_MASK   = 0xff000000;
     private static final int DCM_RGBX_GREEN_MASK = 0x00ff0000;
@@ -67,7 +67,7 @@ public class BufImgSurfaceData extends SurfaceData {
     private static final int DCM_ARGBBM_BLUE_MASK  = 0x000000ff;
 
     static {
-        initIDs(IndexColorModel.class);
+        initIDs(IndexColorModel.class, ICMColorData.class);
     }
 
     public static SurfaceData createData(BufferedImage bufImg) {
@@ -403,7 +403,7 @@ public class BufImgSurfaceData extends SurfaceData {
         // their pixels are immediately retrievable anyway.
     }
 
-    public static native void freeNativeICMData(IndexColorModel icm);
+    private static native void freeNativeICMData(long pData);
 
     /**
      * Returns destination Image associated with this SurfaceData.
@@ -411,4 +411,19 @@ public class BufImgSurfaceData extends SurfaceData {
     public Object getDestination() {
         return bufImg;
     }
+
+    public static final class ICMColorData {
+        private long pData = 0L;
+
+        private ICMColorData(long pData) {
+            this.pData = pData;
+        }
+
+        public void finalize() {
+            if (pData != 0L) {
+                BufImgSurfaceData.freeNativeICMData(pData);
+                pData = 0L;
+            }
+        }
+    }
 }

src/share/native/sun/awt/image/BufImgSurfaceData.c

@@ -48,9 +48,12 @@ static ColorData *BufImg_SetupICM(JNIEnv *env, BufImgSDOps *bisdo);
 
 static jfieldID         rgbID;
 static jfieldID         mapSizeID;
-static jfieldID         CMpDataID;
+static jfieldID         colorDataID;
+static jfieldID         pDataID;
 static jfieldID         allGrayID;
 
+static jclass           clsICMCD;
+static jmethodID        initICMCDmID;
 /*
  * Class:     sun_awt_image_BufImgSurfaceData
  * Method:    initIDs
@@ -58,18 +61,23 @@ static jfieldID         allGrayID;
  */
 JNIEXPORT void JNICALL
 Java_sun_awt_image_BufImgSurfaceData_initIDs
-    (JNIEnv *env, jclass bisd, jclass icm)
+(JNIEnv *env, jclass bisd, jclass icm, jclass cd)
 {
     if (sizeof(BufImgRIPrivate) > SD_RASINFO_PRIVATE_SIZE) {
         JNU_ThrowInternalError(env, "Private RasInfo structure too large!");
         return;
     }
 
+    clsICMCD = (*env)->NewWeakGlobalRef(env, cd);
+    initICMCDmID = (*env)->GetMethodID(env, cd, "<init>", "(J)V");
+    pDataID = (*env)->GetFieldID(env, cd, "pData", "J");
+
     rgbID = (*env)->GetFieldID(env, icm, "rgb", "[I");
     allGrayID = (*env)->GetFieldID(env, icm, "allgrayopaque", "Z");
     mapSizeID = (*env)->GetFieldID(env, icm, "map_size", "I");
-    CMpDataID = (*env)->GetFieldID(env, icm, "pData", "J");
-    if (allGrayID == 0 || rgbID == 0 || mapSizeID == 0 || CMpDataID == 0) {
+    colorDataID = (*env)->GetFieldID(env, icm, "colorData",
+        "Lsun/awt/image/BufImgSurfaceData$ICMColorData;");
+    if (allGrayID == 0 || rgbID == 0 || mapSizeID == 0 || pDataID == 0|| colorDataID == 0 || initICMCDmID == 0) {
         JNU_ThrowInternalError(env, "Could not get field IDs");
     }
 }
@@ -81,18 +89,9 @@ Java_sun_awt_image_BufImgSurfaceData_initIDs
  */
 JNIEXPORT void JNICALL
 Java_sun_awt_image_BufImgSurfaceData_freeNativeICMData
-    (JNIEnv *env, jclass sd, jobject icm)
+    (JNIEnv *env, jclass sd, jlong pData)
 {
-    jlong pData;
-    ColorData *cdata;
-
-    if (JNU_IsNull(env, icm)) {
-        JNU_ThrowNullPointerException(env, "IndexColorModel cannot be null");
-        return;
-    }
-
-    pData = (*env)->GetLongField (env, icm, CMpDataID);
-    cdata = (ColorData *)pData;
+    ColorData *cdata = (ColorData*)jlong_to_ptr(pData);
     freeICMColorData(cdata);
 }
 
@@ -259,32 +258,48 @@ static void BufImg_Release(JNIEnv *env,
 static ColorData *BufImg_SetupICM(JNIEnv *env,
                                   BufImgSDOps *bisdo)
 {
-    ColorData *cData;
+    ColorData *cData = NULL;
+    jobject colorData;
 
     if (JNU_IsNull(env, bisdo->icm)) {
         return (ColorData *) NULL;
     }
 
-    cData = (ColorData *) JNU_GetLongFieldAsPtr(env, bisdo->icm, CMpDataID);
+    colorData = (*env)->GetObjectField(env, bisdo->icm, colorDataID);
 
-    if (cData == NULL) {
-        cData = (ColorData*)calloc(1, sizeof(ColorData));
+    if (JNU_IsNull(env, colorData)) {
+        if (JNU_IsNull(env, clsICMCD)) {
+            // we are unable to create a wrapper object
+            return (ColorData*)NULL;
+        }
+    } else {
+        cData = (ColorData*)JNU_GetLongFieldAsPtr(env, colorData, pDataID);
+    }
+
+    if (cData != NULL) {
+        return cData;
+    }
+
+    cData = (ColorData*)calloc(1, sizeof(ColorData));
 
-        if (cData != NULL) {
-            jboolean allGray
-                = (*env)->GetBooleanField(env, bisdo->icm, allGrayID);
-            int *pRgb = (int *)
-                ((*env)->GetPrimitiveArrayCritical(env, bisdo->lutarray, NULL));
-            cData->img_clr_tbl = initCubemap(pRgb, bisdo->lutsize, 32);
-            if (allGray == JNI_TRUE) {
-                initInverseGrayLut(pRgb, bisdo->lutsize, cData);
-            }
-            (*env)->ReleasePrimitiveArrayCritical(env, bisdo->lutarray, pRgb,
-                                                  JNI_ABORT);
+    if (cData != NULL) {
+        jboolean allGray
+            = (*env)->GetBooleanField(env, bisdo->icm, allGrayID);
+        int *pRgb = (int *)
+            ((*env)->GetPrimitiveArrayCritical(env, bisdo->lutarray, NULL));
+        cData->img_clr_tbl = initCubemap(pRgb, bisdo->lutsize, 32);
+        if (allGray == JNI_TRUE) {
+            initInverseGrayLut(pRgb, bisdo->lutsize, cData);
+        }
+        (*env)->ReleasePrimitiveArrayCritical(env, bisdo->lutarray, pRgb,
+                                              JNI_ABORT);
 
-            initDitherTables(cData);
+        initDitherTables(cData);
 
-            JNU_SetLongFieldFromPtr(env, bisdo->icm, CMpDataID, cData);
+        if (JNU_IsNull(env, colorData)) {
+            jlong pData = ptr_to_jlong(cData);
+            colorData = (*env)->NewObjectA(env, clsICMCD, initICMCDmID, (jvalue *)&pData);
+            (*env)->SetObjectField(env, bisdo->icm, colorDataID, colorData);
         }
     }