Merge "Make ImageWriter thread safe" into tm-qpr-dev

media/java/android/media/ImageWriter.java

@@ -100,6 +100,8 @@ public class ImageWriter implements AutoCloseable {
     private final Object mListenerLock = new Object();
     private OnImageReleasedListener mListener;
     private ListenerHandler mListenerHandler;
+    private final Object mCloseLock = new Object();
+    private boolean mIsWriterValid = false;
     private long mNativeContext;
 
     private int mWidth;
@@ -305,6 +307,8 @@ public class ImageWriter implements AutoCloseable {
             ImageUtils.getEstimatedNativeAllocBytes(mWidth, mHeight,
                 useLegacyImageFormat ? imageFormat : hardwareBufferFormat, /*buffer count*/ 1);
         VMRuntime.getRuntime().registerNativeAllocation(mEstimatedNativeAllocBytes);
+
+        mIsWriterValid = true;
     }
 
     private ImageWriter(Surface surface, int maxImages, boolean useSurfaceImageFormatInfo,
@@ -448,14 +452,17 @@ public class ImageWriter implements AutoCloseable {
      * @see Image#close
      */
     public Image dequeueInputImage() {
-        if (mDequeuedImages.size() >= mMaxImages) {
+        synchronized (mCloseLock) {
-            throw new IllegalStateException("Already dequeued max number of Images " + mMaxImages);
+            if (mDequeuedImages.size() >= mMaxImages) {
+                throw new IllegalStateException(
+                        "Already dequeued max number of Images " + mMaxImages);
+            }
+            WriterSurfaceImage newImage = new WriterSurfaceImage(this);
+            nativeDequeueInputImage(mNativeContext, newImage);
+            mDequeuedImages.add(newImage);
+            newImage.mIsImageValid = true;
+            return newImage;
         }
-        WriterSurfaceImage newImage = new WriterSurfaceImage(this);
-        nativeDequeueInputImage(mNativeContext, newImage);
-        mDequeuedImages.add(newImage);
-        newImage.mIsImageValid = true;
-        return newImage;
     }
 
     /**
@@ -513,49 +520,53 @@ public class ImageWriter implements AutoCloseable {
         if (image == null) {
             throw new IllegalArgumentException("image shouldn't be null");
         }
-        boolean ownedByMe = isImageOwnedByMe(image);
-        if (ownedByMe && !(((WriterSurfaceImage) image).mIsImageValid)) {
-            throw new IllegalStateException("Image from ImageWriter is invalid");
-        }
 
-        // For images from other components that have non-null owner, need to detach first,
+        synchronized (mCloseLock) {
-        // then attach. Images without owners must already be attachable.
+            boolean ownedByMe = isImageOwnedByMe(image);
-        if (!ownedByMe) {
+            if (ownedByMe && !(((WriterSurfaceImage) image).mIsImageValid)) {
-            if ((image.getOwner() instanceof ImageReader)) {
+                throw new IllegalStateException("Image from ImageWriter is invalid");
-                ImageReader prevOwner = (ImageReader) image.getOwner();
-
-                prevOwner.detachImage(image);
-            } else if (image.getOwner() != null) {
-                throw new IllegalArgumentException("Only images from ImageReader can be queued to"
-                        + " ImageWriter, other image source is not supported yet!");
             }
 
-            attachAndQueueInputImage(image);
+            // For images from other components that have non-null owner, need to detach first,
-            // This clears the native reference held by the original owner.
+            // then attach. Images without owners must already be attachable.
-            // When this Image is detached later by this ImageWriter, the
+            if (!ownedByMe) {
-            // native memory won't be leaked.
+                if ((image.getOwner() instanceof ImageReader)) {
-            image.close();
+                    ImageReader prevOwner = (ImageReader) image.getOwner();
-            return;
-        }
 
-        Rect crop = image.getCropRect();
+                    prevOwner.detachImage(image);
-        nativeQueueInputImage(mNativeContext, image, image.getTimestamp(), image.getDataSpace(),
+                } else if (image.getOwner() != null) {
-                crop.left, crop.top, crop.right, crop.bottom, image.getTransform(),
+                    throw new IllegalArgumentException(
-                image.getScalingMode());
+                            "Only images from ImageReader can be queued to"
+                                    + " ImageWriter, other image source is not supported yet!");
+                }
 
-        /**
+                attachAndQueueInputImage(image);
-         * Only remove and cleanup the Images that are owned by this
+                // This clears the native reference held by the original owner.
-         * ImageWriter. Images detached from other owners are only temporarily
+                // When this Image is detached later by this ImageWriter, the
-         * owned by this ImageWriter and will be detached immediately after they
+                // native memory won't be leaked.
-         * are released by downstream consumers, so there is no need to keep
+                image.close();
-         * track of them in mDequeuedImages.
+                return;
-         */
+            }
-        if (ownedByMe) {
+
-            mDequeuedImages.remove(image);
+            Rect crop = image.getCropRect();
-            // Do not call close here, as close is essentially cancel image.
+            nativeQueueInputImage(mNativeContext, image, image.getTimestamp(), image.getDataSpace(),
-            WriterSurfaceImage wi = (WriterSurfaceImage) image;
+                    crop.left, crop.top, crop.right, crop.bottom, image.getTransform(),
-            wi.clearSurfacePlanes();
+                    image.getScalingMode());
-            wi.mIsImageValid = false;
+
+            /**
+             * Only remove and cleanup the Images that are owned by this
+             * ImageWriter. Images detached from other owners are only temporarily
+             * owned by this ImageWriter and will be detached immediately after they
+             * are released by downstream consumers, so there is no need to keep
+             * track of them in mDequeuedImages.
+             */
+            if (ownedByMe) {
+                mDequeuedImages.remove(image);
+                // Do not call close here, as close is essentially cancel image.
+                WriterSurfaceImage wi = (WriterSurfaceImage) image;
+                wi.clearSurfacePlanes();
+                wi.mIsImageValid = false;
+            }
         }
     }
 
@@ -691,17 +702,23 @@ public class ImageWriter implements AutoCloseable {
      */
     @Override
     public void close() {
-        setOnImageReleasedListener(null, null);
+        synchronized (mCloseLock) {
-        for (Image image : mDequeuedImages) {
+            if (!mIsWriterValid) {
-            image.close();
+                return;
-        }
+            }
-        mDequeuedImages.clear();
+            setOnImageReleasedListener(null, null);
-        nativeClose(mNativeContext);
+            for (Image image : mDequeuedImages) {
-        mNativeContext = 0;
+                image.close();
+            }
+            mDequeuedImages.clear();
+            nativeClose(mNativeContext);
+            mNativeContext = 0;
 
-        if (mEstimatedNativeAllocBytes > 0) {
+            if (mEstimatedNativeAllocBytes > 0) {
-            VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);
+                VMRuntime.getRuntime().registerNativeFree(mEstimatedNativeAllocBytes);
-            mEstimatedNativeAllocBytes = 0;
+                mEstimatedNativeAllocBytes = 0;
+            }
+            mIsWriterValid = false;
         }
     }
 
@@ -790,10 +807,16 @@ public class ImageWriter implements AutoCloseable {
         @Override
         public void handleMessage(Message msg) {
             OnImageReleasedListener listener;
-            synchronized (mListenerLock) {
+            boolean isWriterValid;
+            synchronized (ImageWriter.this.mListenerLock) {
                 listener = mListener;
             }
-            if (listener != null) {
+            // Check to make sure we don't accidentally queue images after the writer is
+            // closed or closing
+            synchronized (ImageWriter.this.mCloseLock) {
+                isWriterValid = ImageWriter.this.mIsWriterValid;
+            }
+            if (listener != null && isWriterValid) {
                 listener.onImageReleased(ImageWriter.this);
             }
         }
@@ -813,10 +836,14 @@ public class ImageWriter implements AutoCloseable {
         }
 
         final Handler handler;
+        final boolean isWriterValid;
         synchronized (iw.mListenerLock) {
             handler = iw.mListenerHandler;
         }
-        if (handler != null) {
+        synchronized (iw.mCloseLock) {
+            isWriterValid = iw.mIsWriterValid;
+        }
+        if (handler != null && isWriterValid) {
             handler.sendEmptyMessage(0);
         }
     }
@@ -1050,6 +1077,9 @@ public class ImageWriter implements AutoCloseable {
         private int mTransform = 0; //Default no transform
         private int mScalingMode = 0; //Default frozen scaling mode
 
+        private final Object mCloseLock = new Object(); // lock to protect against multiple
+                                                        // simultaneous calls to close()
+
         public WriterSurfaceImage(ImageWriter writer) {
             mOwner = writer;
             mWidth = writer.mWidth;
@@ -1192,8 +1222,10 @@ public class ImageWriter implements AutoCloseable {
 
         @Override
         public void close() {
-            if (mIsImageValid) {
+            synchronized (mCloseLock) {
-                getOwner().abortImage(this);
+                if (mIsImageValid) {
+                    getOwner().abortImage(this);
+                }
             }
         }