commit 657c1877d86a0d1353d2170026e2fcf46cf1eae0
author Yin-Chia Yeh <yinchiayeh@google.com> Tue Jul 18 18:09:57 2017 -0700
committer Yin-Chia Yeh <yinchiayeh@google.com> Wed Jul 19 12:05:38 2017 -0700
tree 79933dda3a4fd9ce3205298d8de67b92b56e7aec
parent 4f771e05dba8fc3f01ad2af4befe50f085e5f3fd

Camera: fix deadlock with flush

Test: CTS, run testReprocessAbort 50 times
Bug: 63701864
Change-Id: Id41b68da143c9292437f35f8960ce4f80b9d59f7

services/camera/libcameraservice/device3/Camera3Device.cpp

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 0807c0a..49423a0 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -851,12 +851,14 @@
 hardware::Return<void> Camera3Device::processCaptureResult(
         const hardware::hidl_vec<
                 hardware::camera::device::V3_2::CaptureResult>& results) {
-    {
-        Mutex::Autolock l(mLock);
-        if (mStatus == STATUS_ERROR) {
-            // Per API contract, HAL should act as closed after device error
-            ALOGW("%s: received capture result in error state!", __FUNCTION__);
-        }
+    // Ideally we should grab mLock, but that can lead to deadlock, and
+    // it's not super important to get up to date value of mStatus for this
+    // warning print, hence skipping the lock here
+    if (mStatus == STATUS_ERROR) {
+        // Per API contract, HAL should act as closed after device error
+        // But mStatus can be set to error by framework as well, so just log
+        // a warning here.
+        ALOGW("%s: received capture result in error state.", __FUNCTION__);
     }
 
     if (mProcessCaptureResultLock.tryLock() != OK) {
@@ -989,13 +991,16 @@
 
 hardware::Return<void> Camera3Device::notify(
         const hardware::hidl_vec<hardware::camera::device::V3_2::NotifyMsg>& msgs) {
-    {
-        Mutex::Autolock l(mLock);
-        if (mStatus == STATUS_ERROR) {
-            // Per API contract, HAL should act as closed after device error
-            ALOGW("%s: received notify message in error state!", __FUNCTION__);
-        }
+    // Ideally we should grab mLock, but that can lead to deadlock, and
+    // it's not super important to get up to date value of mStatus for this
+    // warning print, hence skipping the lock here
+    if (mStatus == STATUS_ERROR) {
+        // Per API contract, HAL should act as closed after device error
+        // But mStatus can be set to error by framework as well, so just log
+        // a warning here.
+        ALOGW("%s: received notify message in error state.", __FUNCTION__);
     }
+
     for (const auto& msg : msgs) {
         notify(msg);
     }