commit 1646c3ce9a06b9be85c77b5797e152bf6e46851c
author Eino-Ville Talvala <etalvala@google.com> Thu Jul 29 13:48:40 2021 -0700
committer Eino-Ville Talvala <etalvala@google.com> Thu Jul 29 21:41:20 2021 +0000
tree 558e11368f905892fac38313836db2312dbe8350
parent 2a7630de12f785bfb07905f5f1f41b36ccec83ef

Camera: Fix deadlock when querying for camera muting support

Camera3Device::RequestThread can call into
Camera3Device::supportsCameraMute() while
Camera3Device::waitUntilDrained() is waiting for requestThread to
drain the camera request queue, while holding the same lock that
supportsCameraMute needs to acquire.  This leads to a deadlock since
RequestThread can't make forward progress while waitUntilDrained is
waiting for it to complete.

Refactor code to pass in supportsCameraMute into RequestThread's constructor so
it can be referenced within RequestThread without querying Camera3Device.

Test: atest CtsCameraTestCases on coral
      manual verification with TestingCamera2 on cuttlefish
Bug: 194765122
Change-Id: Ie9713af6961d3e085d898f351b9b1f1b6d9fdfb0

services/camera/libcameraservice/device3/Camera3Device.cpp

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index d572d57..fd645c7 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -306,9 +306,25 @@
         sessionParamKeys.insertArrayAt(sessionKeysEntry.data.i32, 0, sessionKeysEntry.count);
     }
 
+    camera_metadata_entry_t availableTestPatternModes = mDeviceInfo.find(
+            ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES);
+    for (size_t i = 0; i < availableTestPatternModes.count; i++) {
+        if (availableTestPatternModes.data.i32[i] ==
+                ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR) {
+            mSupportCameraMute = true;
+            mSupportTestPatternSolidColor = true;
+            break;
+        } else if (availableTestPatternModes.data.i32[i] ==
+                ANDROID_SENSOR_TEST_PATTERN_MODE_BLACK) {
+            mSupportCameraMute = true;
+            mSupportTestPatternSolidColor = false;
+        }
+    }
+
     /** Start up request queue thread */
     mRequestThread = new RequestThread(
-            this, mStatusTracker, mInterface, sessionParamKeys, mUseHalBufManager);
+            this, mStatusTracker, mInterface, sessionParamKeys,
+            mUseHalBufManager, mSupportCameraMute);
     res = mRequestThread->run(String8::format("C3Dev-%s-ReqQueue", mId.string()).string());
     if (res != OK) {
         SET_ERR_L("Unable to start request queue thread: %s (%d)",
@@ -364,21 +380,6 @@
         mRotateAndCropMappers.emplace(mId.c_str(), &mDeviceInfo);
     }
 
-    camera_metadata_entry_t availableTestPatternModes = mDeviceInfo.find(
-            ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES);
-    for (size_t i = 0; i < availableTestPatternModes.count; i++) {
-        if (availableTestPatternModes.data.i32[i] ==
-                ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR) {
-            mSupportCameraMute = true;
-            mSupportTestPatternSolidColor = true;
-            break;
-        } else if (availableTestPatternModes.data.i32[i] ==
-                ANDROID_SENSOR_TEST_PATTERN_MODE_BLACK) {
-            mSupportCameraMute = true;
-            mSupportTestPatternSolidColor = false;
-        }
-    }
-
     mInjectionMethods = new Camera3DeviceInjectionMethods(this);
 
     return OK;
@@ -4155,7 +4156,8 @@
 Camera3Device::RequestThread::RequestThread(wp<Camera3Device> parent,
         sp<StatusTracker> statusTracker,
         sp<HalInterface> interface, const Vector<int32_t>& sessionParamKeys,
-        bool useHalBufManager) :
+        bool useHalBufManager,
+        bool supportCameraMute) :
         Thread(/*canCallJava*/false),
         mParent(parent),
         mStatusTracker(statusTracker),
@@ -4181,7 +4183,8 @@
         mRequestLatency(kRequestLatencyBinSize),
         mSessionParamKeys(sessionParamKeys),
         mLatestSessionParams(sessionParamKeys.size()),
-        mUseHalBufManager(useHalBufManager) {
+        mUseHalBufManager(useHalBufManager),
+        mSupportCameraMute(supportCameraMute){
     mStatusId = statusTracker->addComponent("RequestThread");
 }
 
@@ -5862,12 +5865,7 @@
         const sp<CaptureRequest> &request) {
     ATRACE_CALL();
 
-    {
-        sp<Camera3Device> parent = mParent.promote();
-        if (parent != nullptr) {
-            if (!parent->supportsCameraMute()) return false;
-        }
-    }
+    if (!mSupportCameraMute) return false;
 
     Mutex::Autolock l(mTriggerMutex);
 

services/camera/libcameraservice/device3/Camera3Device.h

diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index 733ed15..39714f0 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -813,7 +813,8 @@
                 sp<camera3::StatusTracker> statusTracker,
                 sp<HalInterface> interface,
                 const Vector<int32_t>& sessionParamKeys,
-                bool useHalBufManager);
+                bool useHalBufManager,
+                bool supportCameraMute);
         ~RequestThread();
 
         void     setNotificationListener(wp<NotificationListener> listener);
@@ -1086,6 +1087,7 @@
         std::map<int32_t, std::set<String8>> mGroupIdPhysicalCameraMap;
 
         const bool         mUseHalBufManager;
+        const bool         mSupportCameraMute;
     };
     sp<RequestThread> mRequestThread;