commit 407571f7c7199495f506500227d2a6390b96df0a
author Jayant Chowdhary <jchowdhary@google.com> Tue Oct 05 14:36:13 2021 -0700
committer Jayant Chowdhary <jchowdhary@google.com> Tue Nov 09 18:37:10 2021 +0000
tree b4fd76647be9936d8a332029d6664607d8680bb3
parent cd2b79a8d7962fa754c0387990c9ff67fa201350

cameraserver: fix deadlock scenario in torchModeStatusChanged callback.

The following scenario can occur:

T1: CameraService::connectDevice()
      CameraService::connectDeviceHelper()
         CameraProviderManager::openSession() ---> holds mInterfaceLock
         .
         .
         . on the same thread before openSession execution completes
           CameraProviderManager::ProviderInfo::torchModeStatusChange() callback from HAL
           .
             CameraService::onTorchStatusChanged()
               CameraProviderManager::getSystemCameraKind tries to lock mInterfaceLock -> deadlock.

We now pass in system camera kind to onTorchStatusChanged in
CameraProviderManager::torchModeStatusChange() instead of calling getSystemCameraKind

This CL also removes CameraProviderManager::mStatusListenerMutex, since
it wasn't protecting any data structure.

Bug: 202198748

Test: camera CTS, GCA (basic validity)

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

services/camera/libcameraservice/CameraService.h

diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 35e13e7..69d44e2 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -109,8 +109,16 @@
     virtual void        onDeviceStatusChanged(const String8 &cameraId,
             const String8 &physicalCameraId,
             hardware::camera::common::V1_0::CameraDeviceStatus newHalStatus) override;
+    // This method may hold CameraProviderManager::mInterfaceMutex as a part
+    // of calling getSystemCameraKind() internally. Care should be taken not to
+    // directly / indirectly call this from callers who also hold
+    // mInterfaceMutex.
     virtual void        onTorchStatusChanged(const String8& cameraId,
             hardware::camera::common::V1_0::TorchModeStatus newStatus) override;
+    // Does not hold CameraProviderManager::mInterfaceMutex.
+    virtual void        onTorchStatusChanged(const String8& cameraId,
+            hardware::camera::common::V1_0::TorchModeStatus newStatus,
+            SystemCameraKind kind) override;
     virtual void        onNewProviderRegistered() override;
 
     /////////////////////////////////////////////////////////////////////