Fix deadlock when notifyPhysicalCameraDevice is called while waitUntilDrained hasn't completed
The following situation is possible :
thread 1: handling waitUntilIdle from client holds Camera3Device::mInterfaceLock and waits for the
HAL to drain all results.
android::Camera3Device::waitUntilStateThenRelock()
android::Camera3Device::waitUntilDrainedLocked()
android::Camera3Device::waitUntilDrained()
android::CameraDeviceClient::waitUntilIdle()
thread 2: handling processCaptureResult callback from the HAL waits on Camera3Device::mInterfaceLock
NonPI::MutexLockWithTimeout()
android::Camera3Device::setRotateAndCropAutoBehavior()
android::Camera2ClientBase<android::CameraDeviceClientBase>::notifyPhysicalCameraChange()
android::camera3::processCaptureResult()
setRotateAndCropAutoBehaviour() in this case, is an effect of a HAL
callback and shouldn't hold mInterfaceLock. Since mLock already protects
the rotate and crop state we can do without holding mInterfaceLock when
called from processCaptureResult.
Bug: 298705937
Bug: 299348355
Test: GCA (basic validity)
Test: Camera CTS
Test: trigger notifyPhysicalCameraDevice from processCaptureResult while
Camera3Device::waitUntilDrained() is still executing, no deadlock
observed.
Change-Id: If6a89ebc7d38510eece00dfbee62af01b5b5b065
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 65b11e7..890c18f 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -362,7 +362,7 @@
static bool isValidAudioRestriction(int32_t mode);
// Override rotate-and-crop AUTO behavior
- virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop) = 0;
+ virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop, bool fromHal = false) = 0;
// Override autoframing AUTO behaviour
virtual status_t setAutoframingOverride(uint8_t autoframingValue) = 0;
diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp
index 5b5892a..9bbb82e 100644
--- a/services/camera/libcameraservice/api1/Camera2Client.cpp
+++ b/services/camera/libcameraservice/api1/Camera2Client.cpp
@@ -2350,7 +2350,7 @@
return mDevice->setCameraServiceWatchdog(enabled);
}
-status_t Camera2Client::setRotateAndCropOverride(uint8_t rotateAndCrop) {
+status_t Camera2Client::setRotateAndCropOverride(uint8_t rotateAndCrop, bool fromHal) {
if (rotateAndCrop > ANDROID_SCALER_ROTATE_AND_CROP_AUTO) return BAD_VALUE;
{
@@ -2364,7 +2364,7 @@
}
return mDevice->setRotateAndCropAutoBehavior(
- static_cast<camera_metadata_enum_android_scaler_rotate_and_crop_t>(rotateAndCrop));
+ static_cast<camera_metadata_enum_android_scaler_rotate_and_crop_t>(rotateAndCrop), fromHal);
}
status_t Camera2Client::setAutoframingOverride(uint8_t autoframingValue) {
diff --git a/services/camera/libcameraservice/api1/Camera2Client.h b/services/camera/libcameraservice/api1/Camera2Client.h
index a7ea823..78bbb69 100644
--- a/services/camera/libcameraservice/api1/Camera2Client.h
+++ b/services/camera/libcameraservice/api1/Camera2Client.h
@@ -81,7 +81,7 @@
virtual status_t setVideoTarget(const sp<IGraphicBufferProducer>& bufferProducer);
virtual status_t setAudioRestriction(int mode);
virtual int32_t getGlobalAudioRestriction();
- virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop);
+ virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop, bool fromHal = false);
virtual status_t setAutoframingOverride(uint8_t autoframingMode);
virtual bool supportsCameraMute();
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
index 1720b55..06f96f3 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
@@ -1761,11 +1761,11 @@
return mDevice->setCameraServiceWatchdog(enabled);
}
-status_t CameraDeviceClient::setRotateAndCropOverride(uint8_t rotateAndCrop) {
+status_t CameraDeviceClient::setRotateAndCropOverride(uint8_t rotateAndCrop, bool fromHal) {
if (rotateAndCrop > ANDROID_SCALER_ROTATE_AND_CROP_AUTO) return BAD_VALUE;
return mDevice->setRotateAndCropAutoBehavior(
- static_cast<camera_metadata_enum_android_scaler_rotate_and_crop_t>(rotateAndCrop));
+ static_cast<camera_metadata_enum_android_scaler_rotate_and_crop_t>(rotateAndCrop), fromHal);
}
status_t CameraDeviceClient::setAutoframingOverride(uint8_t autoframingValue) {
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h
index 95563ee..87b0cd3 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.h
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h
@@ -198,7 +198,8 @@
virtual status_t initialize(sp<CameraProviderManager> manager,
const String8& monitorTags) override;
- virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop) override;
+ virtual status_t setRotateAndCropOverride(uint8_t rotateAndCrop,
+ bool fromHal = false) override;
virtual status_t setAutoframingOverride(uint8_t autoframingValue) override;
diff --git a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
index 86a0ebc..acaf4f2 100644
--- a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp
@@ -80,7 +80,8 @@
return OK;
}
-status_t CameraOfflineSessionClient::setRotateAndCropOverride(uint8_t /*rotateAndCrop*/) {
+status_t CameraOfflineSessionClient::setRotateAndCropOverride(uint8_t /*rotateAndCrop*/,
+ bool /*fromHal*/) {
// Since we're not submitting more capture requests, changes to rotateAndCrop override
// make no difference.
return OK;
diff --git a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.h b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.h
index ad763f9..76dc1cf 100644
--- a/services/camera/libcameraservice/api2/CameraOfflineSessionClient.h
+++ b/services/camera/libcameraservice/api2/CameraOfflineSessionClient.h
@@ -80,7 +80,7 @@
status_t initialize(sp<CameraProviderManager> /*manager*/,
const String8& /*monitorTags*/) override;
- status_t setRotateAndCropOverride(uint8_t rotateAndCrop) override;
+ status_t setRotateAndCropOverride(uint8_t rotateAndCrop, bool fromHal = false) override;
status_t setAutoframingOverride(uint8_t autoframingValue) override;
diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp
index 6e10f30..65c3bd2 100644
--- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp
+++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp
@@ -377,7 +377,8 @@
rotateAndCropMode = ANDROID_SCALER_ROTATE_AND_CROP_90;
}
- static_cast<TClientBase *>(this)->setRotateAndCropOverride(rotateAndCropMode);
+ static_cast<TClientBase *>(this)->setRotateAndCropOverride(rotateAndCropMode,
+ /*fromHal*/ true);
}
}
diff --git a/services/camera/libcameraservice/common/CameraDeviceBase.h b/services/camera/libcameraservice/common/CameraDeviceBase.h
index 919108d..d3af629 100644
--- a/services/camera/libcameraservice/common/CameraDeviceBase.h
+++ b/services/camera/libcameraservice/common/CameraDeviceBase.h
@@ -444,7 +444,8 @@
* and defaults to NONE.
*/
virtual status_t setRotateAndCropAutoBehavior(
- camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue) = 0;
+ camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue,
+ bool fromHal = false) = 0;
/**
* Set the current behavior for the AUTOFRAMING control when in AUTO.
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 71e49fd..312f5ba 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -67,6 +67,7 @@
#include "utils/TraceHFR.h"
#include <algorithm>
+#include <optional>
#include <tuple>
using namespace android::camera3;
@@ -5396,9 +5397,13 @@
}
status_t Camera3Device::setRotateAndCropAutoBehavior(
- camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue) {
+ camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue, bool fromHal) {
ATRACE_CALL();
- Mutex::Autolock il(mInterfaceLock);
+ // We shouldn't hold mInterfaceLock when called as an effect of a HAL
+ // callback since this can lead to a deadlock : b/299348355.
+ // mLock still protects state.
+ std::optional<Mutex::Autolock> maybeMutex =
+ fromHal ? std::nullopt : std::optional<Mutex::Autolock>(mInterfaceLock);
Mutex::Autolock l(mLock);
if (mRequestThread == nullptr) {
return INVALID_OPERATION;
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index aa1d55a..9180a10 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -275,7 +275,7 @@
* and defaults to NONE.
*/
status_t setRotateAndCropAutoBehavior(
- camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue);
+ camera_metadata_enum_android_scaler_rotate_and_crop_t rotateAndCropValue, bool fromHal);
/**
* Set the current behavior for the AUTOFRAMING control when in AUTO.