Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 44d5f626 authored Sep 20, 2023 by Jayant Chowdhary

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>

parent 35b1ff92

services/camera/libcameraservice/CameraService.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -362,7 +362,7 @@ public:
	static bool isValidAudioRestriction(int32_t mode);		static bool isValidAudioRestriction(int32_t mode);

	// Override rotate-and-crop AUTO behavior		// 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		// Override autoframing AUTO behaviour
	virtual status_t setAutoframingOverride(uint8_t autoframingValue) = 0;		virtual status_t setAutoframingOverride(uint8_t autoframingValue) = 0;

services/camera/libcameraservice/api1/Camera2Client.cpp

+2 −2

Original line number	Original line	Diff line number	Diff line
	@@ -2350,7 +2350,7 @@ status_t Camera2Client::setCameraServiceWatchdog(bool enabled) {
	return mDevice->setCameraServiceWatchdog(enabled);		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;		if (rotateAndCrop > ANDROID_SCALER_ROTATE_AND_CROP_AUTO) return BAD_VALUE;

	{		{
	@@ -2364,7 +2364,7 @@ status_t Camera2Client::setRotateAndCropOverride(uint8_t rotateAndCrop) {
	}		}

	return mDevice->setRotateAndCropAutoBehavior(		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) {		status_t Camera2Client::setAutoframingOverride(uint8_t autoframingValue) {

services/camera/libcameraservice/api1/Camera2Client.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -81,7 +81,7 @@ public:
	virtual status_t setVideoTarget(const sp<IGraphicBufferProducer>& bufferProducer);		virtual status_t setVideoTarget(const sp<IGraphicBufferProducer>& bufferProducer);
	virtual status_t setAudioRestriction(int mode);		virtual status_t setAudioRestriction(int mode);
	virtual int32_t getGlobalAudioRestriction();		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 status_t setAutoframingOverride(uint8_t autoframingMode);

	virtual bool supportsCameraMute();		virtual bool supportsCameraMute();

services/camera/libcameraservice/api2/CameraDeviceClient.cpp

+2 −2

Original line number	Original line	Diff line number	Diff line
	@@ -1761,11 +1761,11 @@ status_t CameraDeviceClient::setCameraServiceWatchdog(bool enabled) {
	return mDevice->setCameraServiceWatchdog(enabled);		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;		if (rotateAndCrop > ANDROID_SCALER_ROTATE_AND_CROP_AUTO) return BAD_VALUE;

	return mDevice->setRotateAndCropAutoBehavior(		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) {		status_t CameraDeviceClient::setAutoframingOverride(uint8_t autoframingValue) {

services/camera/libcameraservice/api2/CameraDeviceClient.h

+2 −1

Original line number	Original line	Diff line number	Diff line
	@@ -198,7 +198,8 @@ public:
	virtual status_t initialize(sp<CameraProviderManager> manager,		virtual status_t initialize(sp<CameraProviderManager> manager,
	const String8& monitorTags) override;		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;		virtual status_t setAutoframingOverride(uint8_t autoframingValue) override;