8007918: Better image writing

Reviewed-by: mschoene, prr, jgodinez

jdk/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java

@@ -183,8 +183,7 @@ public class JPEGImageWriter extends ImageWriter {
                     return null;
                 }
             });
-        initWriterIDs(ImageOutputStream.class,
-                      JPEGQTable.class,
+        initWriterIDs(JPEGQTable.class,
                       JPEGHuffmanTable.class);
     }
 
@@ -200,11 +199,13 @@ public class JPEGImageWriter extends ImageWriter {
     public void setOutput(Object output) {
         setThreadLock();
         try {
+            cbLock.check();
+
             super.setOutput(output); // validates output
             resetInternalState();
             ios = (ImageOutputStream) output; // so this will always work
             // Set the native destination
-            setDest(structPointer, ios);
+            setDest(structPointer);
         } finally {
             clearThreadLock();
         }
@@ -359,6 +360,8 @@ public class JPEGImageWriter extends ImageWriter {
                       ImageWriteParam param) throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             writeOnThread(streamMetadata, image, param);
         } finally {
             clearThreadLock();
@@ -1082,13 +1085,18 @@ public class JPEGImageWriter extends ImageWriter {
                              haveMetadata,
                              restartInterval);
 
-        if (aborted) {
-            processWriteAborted();
-        } else {
-            processImageComplete();
-        }
+        cbLock.lock();
+        try {
+            if (aborted) {
+                processWriteAborted();
+            } else {
+                processImageComplete();
+            }
 
-        ios.flush();
+            ios.flush();
+        } finally {
+            cbLock.unlock();
+        }
         currentImage++;  // After a successful write
     }
 
@@ -1096,6 +1104,8 @@ public class JPEGImageWriter extends ImageWriter {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             prepareWriteSequenceOnThread(streamMetadata);
         } finally {
             clearThreadLock();
@@ -1175,6 +1185,8 @@ public class JPEGImageWriter extends ImageWriter {
         throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             if (sequencePrepared == false) {
                 throw new IllegalStateException("sequencePrepared not called!");
             }
@@ -1188,6 +1200,8 @@ public class JPEGImageWriter extends ImageWriter {
     public void endWriteSequence() throws IOException {
         setThreadLock();
         try {
+            cbLock.check();
+
             if (sequencePrepared == false) {
                 throw new IllegalStateException("sequencePrepared not called!");
             }
@@ -1200,6 +1214,10 @@ public class JPEGImageWriter extends ImageWriter {
     public synchronized void abort() {
         setThreadLock();
         try {
+            /**
+             * NB: we do not check the call back lock here, we allow to abort
+             * the reader any time.
+             */
             super.abort();
             abortWrite(structPointer);
         } finally {
@@ -1223,6 +1241,8 @@ public class JPEGImageWriter extends ImageWriter {
     public void reset() {
         setThreadLock();
         try {
+            cbLock.check();
+
             super.reset();
         } finally {
             clearThreadLock();
@@ -1232,6 +1252,8 @@ public class JPEGImageWriter extends ImageWriter {
     public void dispose() {
         setThreadLock();
         try {
+            cbLock.check();
+
             if (structPointer != 0) {
                 disposerRecord.dispose();
                 structPointer = 0;
@@ -1251,13 +1273,18 @@ public class JPEGImageWriter extends ImageWriter {
      * sending warnings to listeners.
      */
     void warningOccurred(int code) {
-        if ((code < 0) || (code > MAX_WARNING)){
-            throw new InternalError("Invalid warning index");
+        cbLock.lock();
+        try {
+            if ((code < 0) || (code > MAX_WARNING)){
+                throw new InternalError("Invalid warning index");
+            }
+            processWarningOccurred
+                (currentImage,
+                 "com.sun.imageio.plugins.jpeg.JPEGImageWriterResources",
+                Integer.toString(code));
+        } finally {
+            cbLock.unlock();
         }
-        processWarningOccurred
-            (currentImage,
-             "com.sun.imageio.plugins.jpeg.JPEGImageWriterResources",
-             Integer.toString(code));
     }
 
     /**
@@ -1274,21 +1301,41 @@ public class JPEGImageWriter extends ImageWriter {
      * library warnings from being printed to stderr.
      */
     void warningWithMessage(String msg) {
-        processWarningOccurred(currentImage, msg);
+        cbLock.lock();
+        try {
+            processWarningOccurred(currentImage, msg);
+        } 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();
+        }
     }
 
     ///////// End of Package-access API
@@ -1615,16 +1662,14 @@ public class JPEGImageWriter extends ImageWriter {
     ////////////// Native methods and callbacks
 
     /** Sets up static native structures. */
-    private static native void initWriterIDs(Class iosClass,
-                                             Class qTableClass,
+    private static native void initWriterIDs(Class qTableClass,
                                              Class huffClass);
 
     /** Sets up per-writer native structure and returns a pointer to it. */
     private native long initJPEGImageWriter();
 
     /** Sets up native structures for output stream */
-    private native void setDest(long structPointer,
-                                ImageOutputStream ios);
+    private native void setDest(long structPointer);
 
     /**
      * Returns <code>true</code> if the write was aborted.
@@ -1749,7 +1794,12 @@ public class JPEGImageWriter extends ImageWriter {
         }
         raster.setRect(sourceLine);
         if ((y > 7) && (y%8 == 0)) {  // Every 8 scanlines
-            processImageProgress((float) y / (float) sourceHeight * 100.0F);
+            cbLock.lock();
+            try {
+                processImageProgress((float) y / (float) sourceHeight * 100.0F);
+            } finally {
+                cbLock.unlock();
+            }
         }
     }
 
@@ -1777,6 +1827,25 @@ public class JPEGImageWriter extends ImageWriter {
         }
     }
 
+    /**
+     * This method is called from native code in order to write encoder
+     * output to the destination.
+     *
+     * We block any attempt to change the writer state during this
+     * method, in order to prevent a corruption of the native encoder
+     * state.
+     */
+    private void writeOutputData(byte[] data, int offset, int len)
+            throws IOException
+    {
+        cbLock.lock();
+        try {
+            ios.write(data, offset, len);
+        } finally {
+            cbLock.unlock();
+        }
+    }
+
     private Thread theThread = null;
     private int theLockCount = 0;
 
@@ -1811,4 +1880,34 @@ public class JPEGImageWriter extends ImageWriter {
             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 writer 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

@@ -66,7 +66,7 @@ static jmethodID JPEGImageReader_acceptPixelsID;
 static jmethodID JPEGImageReader_pushBackID;
 static jmethodID JPEGImageReader_passStartedID;
 static jmethodID JPEGImageReader_passCompleteID;
-static jmethodID ImageOutputStream_writeID;
+static jmethodID JPEGImageWriter_writeOutputDataID;
 static jmethodID JPEGImageWriter_warningOccurredID;
 static jmethodID JPEGImageWriter_warningWithMessageID;
 static jmethodID JPEGImageWriter_writeMetadataID;
@@ -2290,7 +2290,7 @@ imageio_empty_output_buffer (j_compress_ptr cinfo)
 
     (*env)->CallVoidMethod(env,
                            sb->stream,
-                           ImageOutputStream_writeID,
+                           JPEGImageWriter_writeOutputDataID,
                            sb->hstreamBuffer,
                            0,
                            sb->bufferLength);
@@ -2327,7 +2327,7 @@ imageio_term_destination (j_compress_ptr cinfo)
 
         (*env)->CallVoidMethod(env,
                                sb->stream,
-                               ImageOutputStream_writeID,
+                               JPEGImageWriter_writeOutputDataID,
                                sb->hstreamBuffer,
                                0,
                                datacount);
@@ -2365,13 +2365,12 @@ JNIEXPORT void JNICALL
 Java_com_sun_imageio_plugins_jpeg_JPEGImageWriter_initWriterIDs
     (JNIEnv *env,
      jclass cls,
-     jclass IOSClass,
      jclass qTableClass,
      jclass huffClass) {
 
-    ImageOutputStream_writeID = (*env)->GetMethodID(env,
-                                                    IOSClass,
-                                                    "write",
+    JPEGImageWriter_writeOutputDataID = (*env)->GetMethodID(env,
+                                                    cls,
+                                                    "writeOutputData",
                                                     "([BII)V");
 
     JPEGImageWriter_warningOccurredID = (*env)->GetMethodID(env,
@@ -2495,8 +2494,7 @@ JNIEXPORT void JNICALL
 Java_com_sun_imageio_plugins_jpeg_JPEGImageWriter_setDest
     (JNIEnv *env,
      jobject this,
-     jlong ptr,
-     jobject destination) {
+     jlong ptr) {
 
     imageIODataPtr data = (imageIODataPtr)jlong_to_ptr(ptr);
     j_compress_ptr cinfo;
@@ -2510,7 +2508,7 @@ Java_com_sun_imageio_plugins_jpeg_JPEGImageWriter_setDest
 
     cinfo = (j_compress_ptr) data->jpegObj;
 
-    imageio_set_stream(env, data->jpegObj, data, destination);
+    imageio_set_stream(env, data->jpegObj, data, this);
 
 
     // Don't call the init method, as that depends on pinned arrays