8007667: Better image reading

Reviewed-by: prr, jgodinez, mschoene

jdk/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) {
-        if ((code < 0) || (code > MAX_WARNING)){
+        cbLock.lock();
-            throw new InternalError("Invalid warning index");
+        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();
         }
-        processWarningOccurred
-            ("com.sun.imageio.plugins.jpeg.JPEGImageReaderResources",
-             Integer.toString(code));
     }
 
     /**
@@ -265,7 +270,12 @@ public class JPEGImageReader extends ImageReader {
      * library warnings from being printed to stderr.
      */
     protected void warningWithMessage(String msg) {
-        processWarningOccurred(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");
         }
-        iis.seek(iis.getStreamPosition()-num);
+        cbLock.lock();
-        // The buffer is clear after this, so no need to set haveSeeked.
+        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;
-                warningOccurred(WARNING_IGNORE_INVALID_ICC);
+                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,58 +1264,63 @@ public class JPEGImageReader extends ImageReader {
         }
         target.setRect(destROI.x, destROI.y + y, raster);
 
-        processImageUpdate(image,
+        cbLock.lock();
-                           destROI.x, destROI.y+y,
+        try {
-                           raster.getWidth(), 1,
+            processImageUpdate(image,
-                           1, 1,
+                               destROI.x, destROI.y+y,
-                           destinationBands);
+                               raster.getWidth(), 1,
-        if ((y > 0) && (y%progInterval == 0)) {
+                               1, 1,
-            int height = target.getHeight()-1;
+                               destinationBands);
-            float percentOfPass = ((float)y)/height;
+            if ((y > 0) && (y%progInterval == 0)) {
-            if (progressive) {
+                int height = target.getHeight()-1;
-                if (knownPassCount != UNKNOWN) {
+                float percentOfPass = ((float)y)/height;
-                    processImageProgress((pass + percentOfPass)*100.0F
+                if (progressive) {
-                                         / knownPassCount);
+                    if (knownPassCount != UNKNOWN) {
-                } else if (maxProgressivePass != Integer.MAX_VALUE) {
+                        processImageProgress((pass + percentOfPass)*100.0F
-                    // Use the range of allowed progressive passes
+                                             / knownPassCount);
-                    processImageProgress((pass + percentOfPass)*100.0F
+                    } else if (maxProgressivePass != Integer.MAX_VALUE) {
-                        / (maxProgressivePass - minProgressivePass + 1));
+                        // Use the range of allowed progressive passes
-                } else {
+                        processImageProgress((pass + percentOfPass)*100.0F
-                    // Assume there are a minimum of MIN_ESTIMATED_PASSES
+                                             / (maxProgressivePass - minProgressivePass + 1));
-                    // and that there is always one more pass
+                    } else {
-                    // Compute the percentage as the percentage at the end
+                        // Assume there are a minimum of MIN_ESTIMATED_PASSES
-                    // of the previous pass, plus the percentage of this
+                        // and that there is always one more pass
-                    // pass scaled to be the percentage of the total remaining,
+                        // Compute the percentage as the percentage at the end
-                    // assuming a minimum of MIN_ESTIMATED_PASSES passes and
+                        // of the previous pass, plus the percentage of this
-                    // that there is always one more pass.  This is monotonic
+                        // pass scaled to be the percentage of the total remaining,
-                    // and asymptotic to 1.0, which is what we need.
+                        // assuming a minimum of MIN_ESTIMATED_PASSES passes and
-                    int remainingPasses = // including this one
+                        // that there is always one more pass.  This is monotonic
-                        Math.max(2, MIN_ESTIMATED_PASSES-pass);
+                        // and asymptotic to 1.0, which is what we need.
-                    int totalPasses = pass + remainingPasses-1;
+                        int remainingPasses = // including this one
-                    progInterval = Math.max(height/20*totalPasses,
+                            Math.max(2, MIN_ESTIMATED_PASSES-pass);
-                                            totalPasses);
+                        int totalPasses = pass + remainingPasses-1;
-                    if (y%progInterval == 0) {
+                        progInterval = Math.max(height/20*totalPasses,
-                        percentToDate = previousPassPercentage +
+                                                totalPasses);
-                            (1.0F - previousPassPercentage)
+                        if (y%progInterval == 0) {
-                            * (percentOfPass)/remainingPasses;
+                            percentToDate = previousPassPercentage +
-                        if (debug) {
+                                (1.0F - previousPassPercentage)
-                            System.out.print("pass= " + pass);
+                                * (percentOfPass)/remainingPasses;
-                            System.out.print(", y= " + y);
+                            if (debug) {
-                            System.out.print(", progInt= " + progInterval);
+                                System.out.print("pass= " + pass);
-                            System.out.print(", % of pass: " + percentOfPass);
+                                System.out.print(", y= " + y);
-                            System.out.print(", rem. passes: "
+                                System.out.print(", progInt= " + progInterval);
-                                             + remainingPasses);
+                                System.out.print(", % of pass: " + percentOfPass);
-                            System.out.print(", prev%: "
+                                System.out.print(", rem. passes: "
-                                             + previousPassPercentage);
+                                                 + remainingPasses);
-                            System.out.print(", %ToDate: " + percentToDate);
+                                System.out.print(", prev%: "
-                            System.out.print(" ");
+                                                 + previousPassPercentage);
+                                System.out.print(", %ToDate: " + percentToDate);
+                                System.out.print(" ");
+                            }
+                            processImageProgress(percentToDate*100.0F);
                         }
-                        processImageProgress(percentToDate*100.0F);
                     }
+                } else {
+                    processImageProgress(percentOfPass * 100.0F);
                 }
-            } else {
-                processImageProgress(percentOfPass * 100.0F);
             }
+        } finally {
+            cbLock.unlock();
         }
     }
 
@@ -1260,33 +1333,58 @@ public class JPEGImageReader extends ImageReader {
     }
 
     private void passStarted (int pass) {
-        this.pass = pass;
+        cbLock.lock();
-        previousPassPercentage = percentToDate;
+        try {
-        processPassStarted(image,
+            this.pass = pass;
-                           pass,
+            previousPassPercentage = percentToDate;
-                           minProgressivePass,
+            processPassStarted(image,
-                           maxProgressivePass,
+                               pass,
-                           0, 0,
+                               minProgressivePass,
-                           1,1,
+                               maxProgressivePass,
-                           destinationBands);
+                               0, 0,
+                               1,1,
+                               destinationBands);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     private void passComplete () {
-        processPassComplete(image);
+        cbLock.lock();
+        try {
+            processPassComplete(image);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     void thumbnailStarted(int thumbnailIndex) {
-        processThumbnailStarted(currentImage, thumbnailIndex);
+        cbLock.lock();
+        try {
+            processThumbnailStarted(currentImage, thumbnailIndex);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     // Provide access to protected superclass method
     void thumbnailProgress(float percentageDone) {
-        processThumbnailProgress(percentageDone);
+        cbLock.lock();
+        try {
+            processThumbnailProgress(percentageDone);
+        } finally {
+            cbLock.unlock();
+        }
     }
 
     // Provide access to protected superclass method
     void thumbnailComplete() {
-        processThumbnailComplete();
+        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
+        }
+    }
 }
 
 /**

jdk/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 JPEGImageReader_readInputDataID;
-static jmethodID ImageInputStream_skipBytesID;
+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,
+    JPEGImageReader_readInputDataID = (*env)->GetMethodID(env,
-                                                  ImageInputStreamClass,
+                                                  cls,
-                                                  "read",
+                                                  "readInputData",
                                                   "([BII)I");
-    ImageInputStream_skipBytesID = (*env)->GetMethodID(env,
+    JPEGImageReader_skipInputBytesID = (*env)->GetMethodID(env,
-                                                       ImageInputStreamClass,
+                                                       cls,
-                                                       "skipBytes",
+                                                       "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,
+     jlong ptr) {
-     jobject source) {
 
     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);
 }