commit 37eca2499bfb3cda510f0a2e2a4e9d629b405a36
author Jayant Chowdhary <jchowdhary@google.com> Mon Nov 18 08:55:56 2019 -0800
committer Jayant Chowdhary <jchowdhary@google.com> Wed Dec 11 14:10:36 2019 -0800
tree fe7f6d9647772349d4c87203b1db141e78c64474
parent 3066e206366b25b6b64cf0ad29188bd04b56b183

Functions called by RequestThread::threadLoop must not hold mInterfaceMutex.

Functions called by RequestThread::threadLoop must not hold
mInterfaceMutex since the following deadlock scenario may occur:

T1: disconnect() -> holds mInterfaceMutex and waits for RequestThread to
    exit
  T2: RequestThread::threadLoop()->reconfigureCamera (or any other function) that waits on
      mInterfaceMutex

leading to a deadlock

Bug: 143513518

Test: GCA
Test: CTS

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

services/camera/libcameraservice/device3/Camera3Device.cpp

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 4227a3b..bda35f3 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -2165,7 +2165,9 @@
 }
 
 void Camera3Device::pauseStateNotify(bool enable) {
-    Mutex::Autolock il(mInterfaceLock);
+    // We must not hold mInterfaceLock here since this function is called from
+    // RequestThread::threadLoop and holding mInterfaceLock could lead to
+    // deadlocks (http://b/143513518)
     Mutex::Autolock l(mLock);
 
     mPauseStateNotify = enable;
@@ -2742,7 +2744,9 @@
     ATRACE_CALL();
     bool ret = false;
 
-    Mutex::Autolock il(mInterfaceLock);
+    // We must not hold mInterfaceLock here since this function is called from
+    // RequestThread::threadLoop and holding mInterfaceLock could lead to
+    // deadlocks (http://b/143513518)
     nsecs_t maxExpectedDuration = getExpectedInFlightDuration();
 
     Mutex::Autolock l(mLock);
@@ -5371,6 +5375,9 @@
 bool Camera3Device::RequestThread::threadLoop() {
     ATRACE_CALL();
     status_t res;
+    // Any function called from threadLoop() must not hold mInterfaceLock since
+    // it could lead to deadlocks (disconnect() -> hold mInterfaceMutex -> wait for request thread
+    // to finish -> request thread waits on mInterfaceMutex) http://b/143513518
 
     // Handle paused state.
     if (waitIfPaused()) {