commit b29fce7b7e2220e6d0feb3597a35216c89c9f412
author Yin-Chia Yeh <yinchiayeh@google.com> Mon Nov 06 17:16:22 2017 -0800
committer Yin-Chia Yeh <yinchiayeh@google.com> Mon Nov 06 17:57:36 2017 -0800
tree db207e9d4af2b8519d67fafbef0854186e622165
parent 068ba821bca6f2d7a2a8335e85fcdb059ea85274

Camera NDK: fix bug in lock order

AImage::close() can be called by AImageReader while holding
AImageReader::mLock.

Test: AR test app + CTS stress
Bug: 68885255
Change-Id: I17037e3e30e0f53b35ca538a3f321693c539cbdc

media/ndk/NdkImage.cpp

diff --git a/media/ndk/NdkImage.cpp b/media/ndk/NdkImage.cpp
index 87b649a..c4ff537 100644
--- a/media/ndk/NdkImage.cpp
+++ b/media/ndk/NdkImage.cpp
@@ -53,7 +53,6 @@
 
 void
 AImage::close(int releaseFenceFd) {
-    lockReader();
     Mutex::Autolock _l(mLock);
     if (mIsClosed) {
         return;
@@ -71,7 +70,6 @@
     mBuffer = nullptr;
     mLockedBuffer = nullptr;
     mIsClosed = true;
-    unlockReader();
 }
 
 void
@@ -622,7 +620,9 @@
 void AImage_deleteAsync(AImage* image, int releaseFenceFd) {
     ALOGV("%s", __FUNCTION__);
     if (image != nullptr) {
+        image->lockReader();
         image->close(releaseFenceFd);
+        image->unlockReader();
         if (!image->isClosed()) {
             LOG_ALWAYS_FATAL("Image close failed!");
         }

media/ndk/NdkImageReader.cpp

diff --git a/media/ndk/NdkImageReader.cpp b/media/ndk/NdkImageReader.cpp
index e90783d..fd6ecb5 100644
--- a/media/ndk/NdkImageReader.cpp
+++ b/media/ndk/NdkImageReader.cpp
@@ -349,8 +349,8 @@
     for (auto it = mAcquiredImages.begin();
               it != mAcquiredImages.end(); it++) {
         AImage* image = *it;
+        Mutex::Autolock _l(image->mLock);
         releaseImageLocked(image, /*releaseFenceFd*/-1);
-        image->close();
     }
 
     // Delete Buffer Items
@@ -502,6 +502,8 @@
     mBufferItemConsumer->releaseBuffer(*buffer, bufferFence);
     returnBufferItemLocked(buffer);
     image->mBuffer = nullptr;
+    image->mLockedBuffer = nullptr;
+    image->mIsClosed = true;
 
     bool found = false;
     // cleanup acquired image list

media/ndk/NdkImageReaderPriv.h

diff --git a/media/ndk/NdkImageReaderPriv.h b/media/ndk/NdkImageReaderPriv.h
index 989b937..bed8a21 100644
--- a/media/ndk/NdkImageReaderPriv.h
+++ b/media/ndk/NdkImageReaderPriv.h
@@ -85,7 +85,7 @@
     // Called by AImageReader_acquireXXX to acquire a Buffer and setup AImage.
     media_status_t acquireImageLocked(/*out*/AImage** image, /*out*/int* fenceFd);
 
-    // Called by AImage to close image
+    // Called by AImage/~AImageReader to close image. Caller is responsible to grab AImage::mLock
     void releaseImageLocked(AImage* image, int releaseFenceFd);
 
     static int getBufferWidth(BufferItem* buffer);