8007667: Better image reading

Reviewed-by: prr, jgodinez, mschoene

src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageReader.java

@@ -243,12 +243,17 @@ public class JPEGImageReader extends ImageReader {
      * sending warnings to listeners.
      */
     protected void warningOccurred(int code) {
+        cbLock.lock();
+        try {
             if ((code < 0) || (code > MAX_WARNING)){
                 throw new InternalError("Invalid warning index");
             }
             processWarningOccurred
                 ("com.sun.imageio.plugins.jpeg.JPEGImageReaderResources",
                  Integer.toString(code));
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     /**
@@ -265,7 +270,12 @@ public class JPEGImageReader extends ImageReader {
      * library warnings from being printed to stderr.
      */
     protected void warningWithMessage(String msg) {
+        cbLock.lock();
+        try {
             processWarningOccurred(msg);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     public void setInput(Object input,
@@ -274,18 +284,55 @@ public class JPEGImageReader extends ImageReader {
     {
         setThreadLock();
         try {
+            cbLock.check();
+
             super.setInput(input, seekForwardOnly, ignoreMetadata);
             this.ignoreMetadata = ignoreMetadata;
             resetInternalState();
             iis = (ImageInputStream) input; // Always works
-            setSource(structPointer, iis);
+            setSource(structPointer);
         } finally {
             clearThreadLock();
         }
     }
 
-    private native void setSource(long structPointer,
-                                  ImageInputStream source);
+    /**
+     * This method is called from native code in order to fill
+     * native input buffer.
+     *
+     * We block any attempt to change the reading state during this
+     * method, in order to prevent a corruption of the native decoder
+     * state.
+     *
+     * @return number of bytes read from the stream.
+     */
+    private int readInputData(byte[] buf, int off, int len) throws IOException {
+        cbLock.lock();
+        try {
+            return iis.read(buf, off, len);
+        } finally {
+            cbLock.unlock();
+        }
+    }
+
+    /**
+     * This method is called from the native code in order to
+     * skip requested number of bytes in the input stream.
+     *
+     * @param n
+     * @return
+     * @throws IOException
+     */
+    private long skipInputBytes(long n) throws IOException {
+        cbLock.lock();
+        try {
+            return iis.skipBytes(n);
+        } finally {
+            cbLock.unlock();
+        }
+    }
+
+    private native void setSource(long structPointer);
 
     private void checkTablesOnly() throws IOException {
         if (debug) {
@@ -337,6 +384,8 @@ public class JPEGImageReader extends ImageReader {
     public int getNumImages(boolean allowSearch) throws IOException {
         setThreadLock();
         try { // locked thread
+            cbLock.check();
+
             return getNumImagesOnThread(allowSearch);
         } finally {
             clearThreadLock();
@@ -536,8 +585,13 @@ public class JPEGImageReader extends ImageReader {
         if (debug) {
             System.out.println("pushing back " + num + " bytes");
         }
+        cbLock.lock();
+        try {
             iis.seek(iis.getStreamPosition()-num);
             // The buffer is clear after this, so no need to set haveSeeked.
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     /**
@@ -644,7 +698,12 @@ public class JPEGImageReader extends ImageReader {
                  * Ignore this profile.
                  */
                 iccCS = null;
+                cbLock.lock();
+                try {
                     warningOccurred(WARNING_IGNORE_INVALID_ICC);
+                } finally {
+                    cbLock.unlock();
+                }
             }
         }
     }
@@ -653,6 +712,7 @@ public class JPEGImageReader extends ImageReader {
         setThreadLock();
         try {
             if (currentImage != imageIndex) {
+                cbLock.check();
                 readHeader(imageIndex, true);
             }
             return width;
@@ -665,6 +725,7 @@ public class JPEGImageReader extends ImageReader {
         setThreadLock();
         try {
             if (currentImage != imageIndex) {
+                cbLock.check();
                 readHeader(imageIndex, true);
             }
             return height;
@@ -693,6 +754,8 @@ public class JPEGImageReader extends ImageReader {
         setThreadLock();
         try {
             if (currentImage != imageIndex) {
+                cbLock.check();
+
                 readHeader(imageIndex, true);
             }
 
@@ -716,6 +779,7 @@ public class JPEGImageReader extends ImageReader {
     private Iterator getImageTypesOnThread(int imageIndex)
         throws IOException {
         if (currentImage != imageIndex) {
+            cbLock.check();
             readHeader(imageIndex, true);
         }
 
@@ -931,6 +995,7 @@ public class JPEGImageReader extends ImageReader {
         setThreadLock();
         try {
             if (!tablesOnlyChecked) {
+                cbLock.check();
                 checkTablesOnly();
             }
             return streamMetadata;
@@ -951,6 +1016,8 @@ public class JPEGImageReader extends ImageReader {
                 return imageMetadata;
             }
 
+            cbLock.check();
+
             gotoImage(imageIndex);
 
             imageMetadata = new JPEGMetadata(false, false, iis, this);
@@ -967,6 +1034,7 @@ public class JPEGImageReader extends ImageReader {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
             try {
                 readInternal(imageIndex, param, false);
             } catch (RuntimeException e) {
@@ -1196,6 +1264,8 @@ public class JPEGImageReader extends ImageReader {
         }
         target.setRect(destROI.x, destROI.y + y, raster);
 
+        cbLock.lock();
+        try {
             processImageUpdate(image,
                                destROI.x, destROI.y+y,
                                raster.getWidth(), 1,
@@ -1249,6 +1319,9 @@ public class JPEGImageReader extends ImageReader {
                     processImageProgress(percentOfPass * 100.0F);
                 }
             }
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     private void initProgressData() {
@@ -1260,6 +1333,8 @@ public class JPEGImageReader extends ImageReader {
     }
 
     private void passStarted (int pass) {
+        cbLock.lock();
+        try {
             this.pass = pass;
             previousPassPercentage = percentToDate;
             processPassStarted(image,
@@ -1269,24 +1344,47 @@ public class JPEGImageReader extends ImageReader {
                                0, 0,
                                1,1,
                                destinationBands);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     private void passComplete () {
+        cbLock.lock();
+        try {
             processPassComplete(image);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     void thumbnailStarted(int thumbnailIndex) {
+        cbLock.lock();
+        try {
             processThumbnailStarted(currentImage, thumbnailIndex);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     // Provide access to protected superclass method
     void thumbnailProgress(float percentageDone) {
+        cbLock.lock();
+        try {
             processThumbnailProgress(percentageDone);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     // Provide access to protected superclass method
     void thumbnailComplete() {
+        cbLock.lock();
+        try {
             processThumbnailComplete();
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     /**
@@ -1310,6 +1408,11 @@ public class JPEGImageReader extends ImageReader {
     public void abort() {
         setThreadLock();
         try {
+            /**
+             * NB: we do not check the call back lock here,
+             * we allow to abort the reader any time.
+             */
+
             super.abort();
             abortRead(structPointer);
         } finally {
@@ -1332,6 +1435,7 @@ public class JPEGImageReader extends ImageReader {
         setThreadLock();
         Raster retval = null;
         try {
+            cbLock.check();
             /*
              * This could be further optimized by not resetting the dest.
              * offset and creating a translated raster in readInternal()
@@ -1371,6 +1475,8 @@ public class JPEGImageReader extends ImageReader {
     public int getNumThumbnails(int imageIndex) throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             getImageMetadata(imageIndex);  // checks iis state for us
             // Now check the jfif segments
             JFIFMarkerSegment jfif =
@@ -1391,6 +1497,8 @@ public class JPEGImageReader extends ImageReader {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             if ((thumbnailIndex < 0)
                 || (thumbnailIndex >= getNumThumbnails(imageIndex))) {
                 throw new IndexOutOfBoundsException("No such thumbnail");
@@ -1409,6 +1517,8 @@ public class JPEGImageReader extends ImageReader {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             if ((thumbnailIndex < 0)
                 || (thumbnailIndex >= getNumThumbnails(imageIndex))) {
                 throw new IndexOutOfBoundsException("No such thumbnail");
@@ -1428,6 +1538,8 @@ public class JPEGImageReader extends ImageReader {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             if ((thumbnailIndex < 0)
                 || (thumbnailIndex >= getNumThumbnails(imageIndex))) {
                 throw new IndexOutOfBoundsException("No such thumbnail");
@@ -1468,6 +1580,7 @@ public class JPEGImageReader extends ImageReader {
     public void reset() {
         setThreadLock();
         try {
+            cbLock.check();
             super.reset();
         } finally {
             clearThreadLock();
@@ -1479,6 +1592,8 @@ public class JPEGImageReader extends ImageReader {
     public void dispose() {
         setThreadLock();
         try {
+            cbLock.check();
+
             if (structPointer != 0) {
                 disposerRecord.dispose();
                 structPointer = 0;
@@ -1540,6 +1655,36 @@ public class JPEGImageReader extends ImageReader {
             theThread = null;
         }
     }
+
+    private CallBackLock cbLock = new CallBackLock();
+
+    private static class CallBackLock {
+
+        private State lockState;
+
+        CallBackLock() {
+            lockState = State.Unlocked;
+        }
+
+        void check() {
+            if (lockState != State.Unlocked) {
+                throw new IllegalStateException("Access to the reader is not allowed");
+            }
+        }
+
+        private void lock() {
+            lockState = State.Locked;
+        }
+
+        private void unlock() {
+            lockState = State.Unlocked;
+        }
+
+        private static enum State {
+            Unlocked,
+            Locked
+        }
+    }
 }
 
 /**

src/share/native/sun/awt/image/jpeg/imageioJPEG.c

@@ -57,8 +57,8 @@
 #define MAX(a,b)        ((a) > (b) ? (a) : (b))
 
 /* Cached Java method ids */
-static jmethodID ImageInputStream_readID;
-static jmethodID ImageInputStream_skipBytesID;
+static jmethodID JPEGImageReader_readInputDataID;
+static jmethodID JPEGImageReader_skipInputBytesID;
 static jmethodID JPEGImageReader_warningOccurredID;
 static jmethodID JPEGImageReader_warningWithMessageID;
 static jmethodID JPEGImageReader_setImageDataID;
@@ -923,7 +923,7 @@ imageio_fill_input_buffer(j_decompress_ptr cinfo)
     RELEASE_ARRAYS(env, data, src->next_input_byte);
     ret = (*env)->CallIntMethod(env,
                                 sb->stream,
-                                ImageInputStream_readID,
+                                JPEGImageReader_readInputDataID,
                                 sb->hstreamBuffer, 0,
                                 sb->bufferLength);
     if ((*env)->ExceptionOccurred(env)
@@ -1013,7 +1013,7 @@ imageio_fill_suspended_buffer(j_decompress_ptr cinfo)
     }
 
     ret = (*env)->CallIntMethod(env, sb->stream,
-                                ImageInputStream_readID,
+                                JPEGImageReader_readInputDataID,
                                 sb->hstreamBuffer,
                                 offset, buflen);
     if ((*env)->ExceptionOccurred(env)
@@ -1107,7 +1107,7 @@ imageio_skip_input_data(j_decompress_ptr cinfo, long num_bytes)
     RELEASE_ARRAYS(env, data, src->next_input_byte);
     ret = (*env)->CallLongMethod(env,
                                  sb->stream,
-                                 ImageInputStream_skipBytesID,
+                                 JPEGImageReader_skipInputBytesID,
                                  (jlong) num_bytes);
     if ((*env)->ExceptionOccurred(env)
         || !GET_ARRAYS(env, data, &(src->next_input_byte))) {
@@ -1382,13 +1382,13 @@ Java_com_sun_imageio_plugins_jpeg_JPEGImageReader_initReaderIDs
      jclass qTableClass,
      jclass huffClass) {
 
-    ImageInputStream_readID = (*env)->GetMethodID(env,
-                                                  ImageInputStreamClass,
-                                                  "read",
+    JPEGImageReader_readInputDataID = (*env)->GetMethodID(env,
+                                                  cls,
+                                                  "readInputData",
                                                   "([BII)I");
-    ImageInputStream_skipBytesID = (*env)->GetMethodID(env,
-                                                       ImageInputStreamClass,
-                                                       "skipBytes",
+    JPEGImageReader_skipInputBytesID = (*env)->GetMethodID(env,
+                                                       cls,
+                                                       "skipInputBytes",
                                                        "(J)J");
     JPEGImageReader_warningOccurredID = (*env)->GetMethodID(env,
                                                             cls,
@@ -1531,8 +1531,7 @@ JNIEXPORT void JNICALL
 Java_com_sun_imageio_plugins_jpeg_JPEGImageReader_setSource
     (JNIEnv *env,
      jobject this,
-     jlong ptr,
-     jobject source) {
+     jlong ptr) {
 
     imageIODataPtr data = (imageIODataPtr)jlong_to_ptr(ptr);
     j_common_ptr cinfo;
@@ -1546,7 +1545,7 @@ Java_com_sun_imageio_plugins_jpeg_JPEGImageReader_setSource
 
     cinfo = data->jpegObj;
 
-    imageio_set_stream(env, cinfo, data, source);
+    imageio_set_stream(env, cinfo, data, this);
 
     imageio_init_source((j_decompress_ptr) cinfo);
 }