commit 33e8ef86ecfa90f9d41ab700c6f9f5e2a19c97a1
author Jayant Chowdhary <jchowdhary@google.com> Fri Sep 27 09:20:42 2019 -0700
committer Jayant Chowdhary <jchowdhary@google.com> Mon Oct 14 10:53:27 2019 -0700
tree fea40195e0d0e02860d709b7802f926e9f5cff70
parent 4bebef8d593f62a5ee80cab04167f4a3ba3722d8

cameraserver: Avoiding deadlocks while calling getSystemCameraKind()

The following scenario can occur:

 T1 serving Client A's disconnect() call:
   T2 : serving Client B's connect() call
     T2 : CameraProviderManager::openSession() locks mInterfaceMutex and waits on open() HAL
          interface call
       T1: updateStatus() locks mStatusListenerMutex
         T1: CameraProviderManager::isPublicallyHiddenSecureCamera() / getSystemCameraKind() waits
             on mInterfaceMutex
           T2: while waiting on open(), gets a torchModeStatus() callback from the camera HAL
              T2: onStatusChanged()
                T2: broadcastTorchModeStatus() which waits on mStatusListenerMutex

As a result there's a possible circular hold and wait between T1 and T2.

We cache the system camera kind in CameraState. That doesn't completely
avoid having to hold mInterfaceLock while calling getSystemCameraKind in
CameraService (cache updates), instead it reduces it. We instead need to
hold mCameraStates lock which has a smaller scope.

Bug: 141756275

Test: CTS
Test: GCA (sanity)

Change-Id: I4a697c1eaccc3603007be4a595febea981fbeb64
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>

services/camera/libcameraservice/common/CameraProviderManager.h

diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index f4cf667..4112711 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -292,8 +292,8 @@
      */
     bool isLogicalCamera(const std::string& id, std::vector<std::string>* physicalCameraIds);
 
-    SystemCameraKind getSystemCameraKind(const std::string& id) const;
-    bool isHiddenPhysicalCamera(const std::string& cameraId);
+    status_t getSystemCameraKind(const std::string& id, SystemCameraKind *kind) const;
+    bool isHiddenPhysicalCamera(const std::string& cameraId) const;
 
     static const float kDepthARTolerance;
 private:
@@ -616,7 +616,8 @@
             CameraMetadata* characteristics) const;
     void filterLogicalCameraIdsLocked(std::vector<std::string>& deviceIds) const;
 
-    SystemCameraKind getSystemCameraKindLocked(const std::string& id) const;
+    status_t getSystemCameraKindLocked(const std::string& id, SystemCameraKind *kind) const;
+    std::pair<bool, ProviderInfo::DeviceInfo *> isHiddenPhysicalCameraInternal(const std::string& cameraId) const;
 
     void collectDeviceIdsLocked(const std::vector<std::string> deviceIds,
             std::vector<std::string>& normalDeviceIds,