From: DRC <information@libjpeg-turbo.org>
Date: Fri, 12 Apr 2019 13:47:28 +0000 (-0500)
Subject: Android: Fix "using JNI after critical get" errors
X-Git-Tag: 2.0.3~21
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=33011754e616286d68b36bee0d48543ea64808b5;p=libjpeg-turbo

Android: Fix "using JNI after critical get" errors

5ea77d8b771bf7b0be7acc7fb70c255f8c9c8f77 was insufficient to fix all of
these.  In particular, we need to always release the primitive arrays
before throwing an exception, because throwing an exception qualifies as
"using JNI."

Refer to #300
---

diff --git a/turbojpeg-jni.c b/turbojpeg-jni.c
index 13f18f7..070dd90 100644
--- a/turbojpeg-jni.c
+++ b/turbojpeg-jni.c
@@ -98,6 +98,12 @@
   } \
 }
 
+#define SAFE_RELEASE(javaArray, cArray) { \
+  if (javaArray && cArray) \
+    (*env)->ReleasePrimitiveArrayCritical(env, javaArray, (void *)cArray, 0); \
+  cArray = NULL; \
+}
+
 int ProcessSystemProperties(JNIEnv *env)
 {
   jclass cls;
@@ -242,12 +248,15 @@ static jint TJCompressor_compress
 
   if (tjCompress2(handle, &srcBuf[y * actualPitch + x * tjPixelSize[pf]],
                   width, pitch, height, pf, &jpegBuf, &jpegSize, jpegSubsamp,
-                  jpegQual, flags | TJFLAG_NOREALLOC) == -1)
+                  jpegQual, flags | TJFLAG_NOREALLOC) == -1) {
+    SAFE_RELEASE(dst, jpegBuf);
+    SAFE_RELEASE(src, srcBuf);
     THROW_TJ();
+  }
 
 bailout:
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, jpegBuf, 0);
-  if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0);
+  SAFE_RELEASE(dst, jpegBuf);
+  SAFE_RELEASE(src, srcBuf);
   return (jint)jpegSize;
 }
 
@@ -343,23 +352,40 @@ JNIEXPORT jint JNICALL Java_org_libjpegturbo_turbojpeg_TJCompressor_compressFrom
 
   if (ProcessSystemProperties(env) < 0) goto bailout;
 
+#define RELEASE_ARRAYS_COMPRESSFROMYUV() { \
+  SAFE_RELEASE(dst, jpegBuf); \
+  for (i = 0; i < nc; i++) \
+    SAFE_RELEASE(jSrcPlanes[i], srcPlanes[i]); \
+  SAFE_RELEASE(jSrcStrides, srcStrides); \
+  SAFE_RELEASE(jSrcOffsets, srcOffsets); \
+}
+
   BAILIF0(srcOffsets = (*env)->GetPrimitiveArrayCritical(env, jSrcOffsets, 0));
   BAILIF0(srcStrides = (*env)->GetPrimitiveArrayCritical(env, jSrcStrides, 0));
   for (i = 0; i < nc; i++) {
     int planeSize = tjPlaneSizeYUV(i, width, srcStrides[i], height, subsamp);
     int pw = tjPlaneWidth(i, width, subsamp);
 
-    if (planeSize < 0 || pw < 0)
+    if (planeSize < 0 || pw < 0) {
+      RELEASE_ARRAYS_COMPRESSFROMYUV();
       THROW_ARG(tjGetErrorStr());
+    }
 
-    if (srcOffsets[i] < 0)
+    if (srcOffsets[i] < 0) {
+      RELEASE_ARRAYS_COMPRESSFROMYUV();
       THROW_ARG("Invalid argument in compressFromYUV()");
-    if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0)
+    }
+    if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) {
+      RELEASE_ARRAYS_COMPRESSFROMYUV();
       THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary");
+    }
 
     BAILIF0(jSrcPlanes[i] = (*env)->GetObjectArrayElement(env, srcobjs, i));
-    if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < srcOffsets[i] + planeSize)
+    if ((*env)->GetArrayLength(env, jSrcPlanes[i]) <
+        srcOffsets[i] + planeSize) {
+      RELEASE_ARRAYS_COMPRESSFROMYUV();
       THROW_ARG("Source plane is not large enough");
+    }
 
     BAILIF0(srcPlanes[i] =
             (*env)->GetPrimitiveArrayCritical(env, jSrcPlanes[i], 0));
@@ -369,20 +395,13 @@ JNIEXPORT jint JNICALL Java_org_libjpegturbo_turbojpeg_TJCompressor_compressFrom
 
   if (tjCompressFromYUVPlanes(handle, srcPlanes, width, srcStrides, height,
                               subsamp, &jpegBuf, &jpegSize, jpegQual,
-                              flags | TJFLAG_NOREALLOC) == -1)
+                              flags | TJFLAG_NOREALLOC) == -1) {
+    RELEASE_ARRAYS_COMPRESSFROMYUV();
     THROW_TJ();
+  }
 
 bailout:
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, jpegBuf, 0);
-  for (i = 0; i < nc; i++) {
-    if (srcPlanes[i] && jSrcPlanes[i])
-      (*env)->ReleasePrimitiveArrayCritical(env, jSrcPlanes[i],
-                                            (unsigned char *)srcPlanes[i], 0);
-  }
-  if (srcStrides)
-    (*env)->ReleasePrimitiveArrayCritical(env, jSrcStrides, srcStrides, 0);
-  if (srcOffsets)
-    (*env)->ReleasePrimitiveArrayCritical(env, jSrcOffsets, srcOffsets, 0);
+  RELEASE_ARRAYS_COMPRESSFROMYUV();
   return (jint)jpegSize;
 }
 
@@ -420,23 +439,40 @@ static void TJCompressor_encodeYUV
   if ((*env)->GetArrayLength(env, src) * srcElementSize < arraySize)
     THROW_ARG("Source buffer is not large enough");
 
+#define RELEASE_ARRAYS_ENCODEYUV() { \
+  SAFE_RELEASE(src, srcBuf); \
+  for (i = 0; i < nc; i++) \
+    SAFE_RELEASE(jDstPlanes[i], dstPlanes[i]); \
+  SAFE_RELEASE(jDstStrides, dstStrides); \
+  SAFE_RELEASE(jDstOffsets, dstOffsets); \
+}
+
   BAILIF0(dstOffsets = (*env)->GetPrimitiveArrayCritical(env, jDstOffsets, 0));
   BAILIF0(dstStrides = (*env)->GetPrimitiveArrayCritical(env, jDstStrides, 0));
   for (i = 0; i < nc; i++) {
     int planeSize = tjPlaneSizeYUV(i, width, dstStrides[i], height, subsamp);
     int pw = tjPlaneWidth(i, width, subsamp);
 
-    if (planeSize < 0 || pw < 0)
+    if (planeSize < 0 || pw < 0) {
+      RELEASE_ARRAYS_ENCODEYUV();
       THROW_ARG(tjGetErrorStr());
+    }
 
-    if (dstOffsets[i] < 0)
+    if (dstOffsets[i] < 0) {
+      RELEASE_ARRAYS_ENCODEYUV();
       THROW_ARG("Invalid argument in encodeYUV()");
-    if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0)
+    }
+    if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) {
+      RELEASE_ARRAYS_ENCODEYUV();
       THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary");
+    }
 
     BAILIF0(jDstPlanes[i] = (*env)->GetObjectArrayElement(env, dstobjs, i));
-    if ((*env)->GetArrayLength(env, jDstPlanes[i]) < dstOffsets[i] + planeSize)
+    if ((*env)->GetArrayLength(env, jDstPlanes[i]) <
+        dstOffsets[i] + planeSize) {
+      RELEASE_ARRAYS_ENCODEYUV();
       THROW_ARG("Destination plane is not large enough");
+    }
 
     BAILIF0(dstPlanes[i] =
             (*env)->GetPrimitiveArrayCritical(env, jDstPlanes[i], 0));
@@ -446,20 +482,13 @@ static void TJCompressor_encodeYUV
 
   if (tjEncodeYUVPlanes(handle, &srcBuf[y * actualPitch + x * tjPixelSize[pf]],
                         width, pitch, height, pf, dstPlanes, dstStrides,
-                        subsamp, flags) == -1)
+                        subsamp, flags) == -1) {
+    RELEASE_ARRAYS_ENCODEYUV();
     THROW_TJ();
+  }
 
 bailout:
-  if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0);
-  for (i = 0; i < nc; i++) {
-    if (dstPlanes[i] && jDstPlanes[i])
-      (*env)->ReleasePrimitiveArrayCritical(env, jDstPlanes[i], dstPlanes[i],
-                                            0);
-  }
-  if (dstStrides)
-    (*env)->ReleasePrimitiveArrayCritical(env, jDstStrides, dstStrides, 0);
-  if (dstOffsets)
-    (*env)->ReleasePrimitiveArrayCritical(env, jDstOffsets, dstOffsets, 0);
+  RELEASE_ARRAYS_ENCODEYUV();
 }
 
 /* TurboJPEG 1.4.x: TJCompressor::encodeYUV() byte source */
@@ -518,12 +547,15 @@ JNIEXPORT void JNICALL TJCompressor_encodeYUV_12
   BAILIF0(dstBuf = (*env)->GetPrimitiveArrayCritical(env, dst, 0));
 
   if (tjEncodeYUV2(handle, srcBuf, width, pitch, height, pf, dstBuf, subsamp,
-                   flags) == -1)
+                   flags) == -1) {
+    SAFE_RELEASE(dst, dstBuf);
+    SAFE_RELEASE(src, srcBuf);
     THROW_TJ();
+  }
 
 bailout:
-  if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0);
-  if (srcBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, srcBuf, 0);
+  SAFE_RELEASE(dst, dstBuf);
+  SAFE_RELEASE(src, srcBuf);
 }
 
 /* TurboJPEG 1.2.x: TJCompressor::encodeYUV() byte source */
@@ -634,11 +666,12 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
   BAILIF0(jpegBuf = (*env)->GetPrimitiveArrayCritical(env, src, 0));
 
   if (tjDecompressHeader3(handle, jpegBuf, (unsigned long)jpegSize, &width,
-                          &height, &jpegSubsamp, &jpegColorspace) == -1)
+                          &height, &jpegSubsamp, &jpegColorspace) == -1) {
+    SAFE_RELEASE(src, jpegBuf);
     THROW_TJ();
+  }
 
-  (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0);
-  jpegBuf = NULL;
+  SAFE_RELEASE(src, jpegBuf);
 
   BAILIF0(_fid = (*env)->GetFieldID(env, _cls, "jpegSubsamp", "I"));
   (*env)->SetIntField(env, obj, _fid, jpegSubsamp);
@@ -652,7 +685,7 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
   (*env)->SetIntField(env, obj, _fid, height);
 
 bailout:
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0);
+  SAFE_RELEASE(src, jpegBuf);
 }
 
 static void TJDecompressor_decompress
@@ -683,12 +716,15 @@ static void TJDecompressor_decompress
 
   if (tjDecompress2(handle, jpegBuf, (unsigned long)jpegSize,
                     &dstBuf[y * actualPitch + x * tjPixelSize[pf]], width,
-                    pitch, height, pf, flags) == -1)
+                    pitch, height, pf, flags) == -1) {
+    SAFE_RELEASE(dst, dstBuf);
+    SAFE_RELEASE(src, jpegBuf);
     THROW_TJ();
+  }
 
 bailout:
-  if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0);
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0);
+  SAFE_RELEASE(dst, dstBuf);
+  SAFE_RELEASE(src, jpegBuf);
 }
 
 /* TurboJPEG 1.3.x: TJDecompressor::decompress() byte destination */
@@ -786,6 +822,14 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
   if (i >= nsf)
     THROW_ARG("Could not scale down to desired image dimensions");
 
+#define RELEASE_ARRAYS_DECOMPRESSTOYUV() { \
+  SAFE_RELEASE(src, jpegBuf); \
+  for (i = 0; i < nc; i++) \
+    SAFE_RELEASE(jDstPlanes[i], dstPlanes[i]); \
+  SAFE_RELEASE(jDstStrides, dstStrides); \
+  SAFE_RELEASE(jDstOffsets, dstOffsets); \
+}
+
   BAILIF0(dstOffsets = (*env)->GetPrimitiveArrayCritical(env, jDstOffsets, 0));
   BAILIF0(dstStrides = (*env)->GetPrimitiveArrayCritical(env, jDstStrides, 0));
   for (i = 0; i < nc; i++) {
@@ -793,17 +837,26 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
                                    jpegSubsamp);
     int pw = tjPlaneWidth(i, scaledWidth, jpegSubsamp);
 
-    if (planeSize < 0 || pw < 0)
+    if (planeSize < 0 || pw < 0) {
+      RELEASE_ARRAYS_DECOMPRESSTOYUV();
       THROW_ARG(tjGetErrorStr());
+    }
 
-    if (dstOffsets[i] < 0)
+    if (dstOffsets[i] < 0) {
+      RELEASE_ARRAYS_DECOMPRESSTOYUV();
       THROW_ARG("Invalid argument in decompressToYUV()");
-    if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0)
+    }
+    if (dstStrides[i] < 0 && dstOffsets[i] - planeSize + pw < 0) {
+      RELEASE_ARRAYS_DECOMPRESSTOYUV();
       THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary");
+    }
 
     BAILIF0(jDstPlanes[i] = (*env)->GetObjectArrayElement(env, dstobjs, i));
-    if ((*env)->GetArrayLength(env, jDstPlanes[i]) < dstOffsets[i] + planeSize)
+    if ((*env)->GetArrayLength(env, jDstPlanes[i]) <
+        dstOffsets[i] + planeSize) {
+      RELEASE_ARRAYS_DECOMPRESSTOYUV();
       THROW_ARG("Destination plane is not large enough");
+    }
 
     BAILIF0(dstPlanes[i] =
             (*env)->GetPrimitiveArrayCritical(env, jDstPlanes[i], 0));
@@ -813,20 +866,13 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
 
   if (tjDecompressToYUVPlanes(handle, jpegBuf, (unsigned long)jpegSize,
                               dstPlanes, desiredWidth, dstStrides,
-                              desiredHeight, flags) == -1)
+                              desiredHeight, flags) == -1) {
+    RELEASE_ARRAYS_DECOMPRESSTOYUV();
     THROW_TJ();
+  }
 
 bailout:
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0);
-  for (i = 0; i < nc; i++) {
-    if (dstPlanes[i] && jDstPlanes[i])
-      (*env)->ReleasePrimitiveArrayCritical(env, jDstPlanes[i], dstPlanes[i],
-                                            0);
-  }
-  if (dstStrides)
-    (*env)->ReleasePrimitiveArrayCritical(env, jDstStrides, dstStrides, 0);
-  if (dstOffsets)
-    (*env)->ReleasePrimitiveArrayCritical(env, jDstOffsets, dstOffsets, 0);
+  RELEASE_ARRAYS_DECOMPRESSTOYUV();
 }
 
 /* TurboJPEG 1.2.x: TJDecompressor::decompressToYUV() */
@@ -856,12 +902,15 @@ JNIEXPORT void JNICALL Java_org_libjpegturbo_turbojpeg_TJDecompressor_decompress
   BAILIF0(dstBuf = (*env)->GetPrimitiveArrayCritical(env, dst, 0));
 
   if (tjDecompressToYUV(handle, jpegBuf, (unsigned long)jpegSize, dstBuf,
-                        flags) == -1)
+                        flags) == -1) {
+    SAFE_RELEASE(dst, dstBuf);
+    SAFE_RELEASE(src, jpegBuf);
     THROW_TJ();
+  }
 
 bailout:
-  if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0);
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, src, jpegBuf, 0);
+  SAFE_RELEASE(dst, dstBuf);
+  SAFE_RELEASE(src, jpegBuf);
 }
 
 static void TJDecompressor_decodeYUV
@@ -898,23 +947,40 @@ static void TJDecompressor_decodeYUV
   if ((*env)->GetArrayLength(env, dst) * dstElementSize < arraySize)
     THROW_ARG("Destination buffer is not large enough");
 
+#define RELEASE_ARRAYS_DECODEYUV() { \
+  SAFE_RELEASE(dst, dstBuf); \
+  for (i = 0; i < nc; i++) \
+    SAFE_RELEASE(jSrcPlanes[i], srcPlanes[i]); \
+  SAFE_RELEASE(jSrcStrides, srcStrides); \
+  SAFE_RELEASE(jSrcOffsets, srcOffsets); \
+}
+
   BAILIF0(srcOffsets = (*env)->GetPrimitiveArrayCritical(env, jSrcOffsets, 0));
   BAILIF0(srcStrides = (*env)->GetPrimitiveArrayCritical(env, jSrcStrides, 0));
   for (i = 0; i < nc; i++) {
     int planeSize = tjPlaneSizeYUV(i, width, srcStrides[i], height, subsamp);
     int pw = tjPlaneWidth(i, width, subsamp);
 
-    if (planeSize < 0 || pw < 0)
+    if (planeSize < 0 || pw < 0) {
+      RELEASE_ARRAYS_DECODEYUV();
       THROW_ARG(tjGetErrorStr());
+    }
 
-    if (srcOffsets[i] < 0)
+    if (srcOffsets[i] < 0) {
+      RELEASE_ARRAYS_DECODEYUV();
       THROW_ARG("Invalid argument in decodeYUV()");
-    if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0)
+    }
+    if (srcStrides[i] < 0 && srcOffsets[i] - planeSize + pw < 0) {
+      RELEASE_ARRAYS_DECODEYUV();
       THROW_ARG("Negative plane stride would cause memory to be accessed below plane boundary");
+    }
 
     BAILIF0(jSrcPlanes[i] = (*env)->GetObjectArrayElement(env, srcobjs, i));
-    if ((*env)->GetArrayLength(env, jSrcPlanes[i]) < srcOffsets[i] + planeSize)
+    if ((*env)->GetArrayLength(env, jSrcPlanes[i]) <
+        srcOffsets[i] + planeSize) {
+      RELEASE_ARRAYS_DECODEYUV();
       THROW_ARG("Source plane is not large enough");
+    }
 
     BAILIF0(srcPlanes[i] =
             (*env)->GetPrimitiveArrayCritical(env, jSrcPlanes[i], 0));
@@ -924,20 +990,13 @@ static void TJDecompressor_decodeYUV
 
   if (tjDecodeYUVPlanes(handle, srcPlanes, srcStrides, subsamp,
                         &dstBuf[y * actualPitch + x * tjPixelSize[pf]], width,
-                        pitch, height, pf, flags) == -1)
+                        pitch, height, pf, flags) == -1) {
+    RELEASE_ARRAYS_DECODEYUV();
     THROW_TJ();
+  }
 
 bailout:
-  if (dstBuf) (*env)->ReleasePrimitiveArrayCritical(env, dst, dstBuf, 0);
-  for (i = 0; i < nc; i++) {
-    if (srcPlanes[i] && jSrcPlanes[i])
-      (*env)->ReleasePrimitiveArrayCritical(env, jSrcPlanes[i],
-                                            (unsigned char *)srcPlanes[i], 0);
-  }
-  if (srcStrides)
-    (*env)->ReleasePrimitiveArrayCritical(env, jSrcStrides, srcStrides, 0);
-  if (srcOffsets)
-    (*env)->ReleasePrimitiveArrayCritical(env, jSrcOffsets, srcOffsets, 0);
+  RELEASE_ARRAYS_DECODEYUV();
 }
 
 /* TurboJPEG 1.4.x: TJDecompressor::decodeYUV() byte destination */
@@ -1149,15 +1208,16 @@ JNIEXPORT jintArray JNICALL Java_org_libjpegturbo_turbojpeg_TJTransformer_transf
             (*env)->GetPrimitiveArrayCritical(env, jdstBufs[i], 0));
 
   if (tjTransform(handle, jpegBuf, jpegSize, n, dstBufs, dstSizes, t,
-                  flags | TJFLAG_NOREALLOC) == -1)
+                  flags | TJFLAG_NOREALLOC) == -1) {
+    for (i = 0; i < n; i++)
+      SAFE_RELEASE(jdstBufs[i], dstBufs[i]);
+    SAFE_RELEASE(jsrcBuf, jpegBuf);
     THROW_TJ();
-
-  for (i = 0; i < n; i++) {
-    (*env)->ReleasePrimitiveArrayCritical(env, jdstBufs[i], dstBufs[i], 0);
-    dstBufs[i] = NULL;
   }
-  (*env)->ReleasePrimitiveArrayCritical(env, jsrcBuf, jpegBuf, 0);
-  jpegBuf = NULL;
+
+  for (i = 0; i < n; i++)
+    SAFE_RELEASE(jdstBufs[i], dstBufs[i]);
+  SAFE_RELEASE(jsrcBuf, jpegBuf);
 
   jdstSizes = (*env)->NewIntArray(env, n);
   BAILIF0(dstSizesi = (*env)->GetIntArrayElements(env, jdstSizes, 0));
@@ -1172,7 +1232,7 @@ bailout:
     }
     free(dstBufs);
   }
-  if (jpegBuf) (*env)->ReleasePrimitiveArrayCritical(env, jsrcBuf, jpegBuf, 0);
+  SAFE_RELEASE(jsrcBuf, jpegBuf);
   if (jdstBufs) free(jdstBufs);
   if (dstSizes) free(dstSizes);
   if (t) free(t);