From 26d975de2fa0119745d6af120428617b2ffa1b09 Mon Sep 17 00:00:00 2001
From: Emilian Peev <epeev@google.com>
Date: Thu, 5 Jul 2018 14:52:57 +0100
Subject: [PATCH] Camera: Use separate lock for status tracker access sync

A race condition is possible during access to status tracker
in 'disonnect' and inflight updates like 'registerInFlight' and
'removeInFlightMapEntryLocked'. To avoid this, access is serialized
using 'mLock'. However 'mLock' is also used in other contexts which
under the right conditions could result in a deadlock. One such instance
could occur during consecutive reprocess requests. The request thread
while holding the request lock will try to obtain a free input buffer.
In case the input stream doesn't have any free buffers left, it will
block on an internal condition and wait. In the meantime if another
capture request gets submitted, then the request itself will block on
the request lock within request thread while holding 'mLock' inside
submit helper. This will not allow incoming input buffer results
to get processed as they could call 'removeInFlightMapEntryLocked' and
try to acquire the already locked 'mLock'. The deadlock will continue
until the input stream timeout expires, which will fail the request.
 One way to resolve this is by adding a separate lock which will only be
used for the status tracker access synchronization.

Bug: 79972865
Test: Camera CTS
Change-Id: Ic63f891202ba102f6408ed714c5eef29b41404e3
---
 .../camera/libcameraservice/device3/Camera3Device.cpp | 11 +++++++----
 .../camera/libcameraservice/device3/Camera3Device.h   |  3 +++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 543914e5a4..4794bfc66a 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -263,6 +263,7 @@ status_t Camera3Device::initializeCommonLocked() {
 status_t Camera3Device::disconnect() {
     ATRACE_CALL();
     Mutex::Autolock il(mInterfaceLock);
+    Mutex::Autolock stLock(mTrackerLock);
 
     ALOGI("%s: E", __FUNCTION__);
 
@@ -2699,8 +2700,9 @@ status_t Camera3Device::registerInFlight(uint32_t frameNumber,
     if (res < 0) return res;
 
     if (mInFlightMap.size() == 1) {
-        // hold mLock to prevent race with disconnect
-        Mutex::Autolock l(mLock);
+        // Hold a separate dedicated tracker lock to prevent race with disconnect and also
+        // avoid a deadlock during reprocess requests.
+        Mutex::Autolock l(mTrackerLock);
         if (mStatusTracker != nullptr) {
             mStatusTracker->markComponentActive(mInFlightStatusId);
         }
@@ -2733,8 +2735,9 @@ void Camera3Device::removeInFlightMapEntryLocked(int idx) {
 
     // Indicate idle inFlightMap to the status tracker
     if (mInFlightMap.size() == 0) {
-        // hold mLock to prevent race with disconnect
-        Mutex::Autolock l(mLock);
+        // Hold a separate dedicated tracker lock to prevent race with disconnect and also
+        // avoid a deadlock during reprocess requests.
+        Mutex::Autolock l(mTrackerLock);
         if (mStatusTracker != nullptr) {
             mStatusTracker->markComponentIdle(mInFlightStatusId, Fence::NO_FENCE);
         }
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index d8fe19fa66..e78bb6166d 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -1208,6 +1208,9 @@ class Camera3Device :
 
     static callbacks_notify_t sNotify;
 
+    // Synchronizes access to status tracker between inflight updates and disconnect.
+    // b/79972865
+    Mutex mTrackerLock;
 }; // class Camera3Device
 
 }; // namespace android