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

Commit 7fe6c425 authored by Emilian Peev's avatar Emilian Peev
Browse files

Camera: Avoid recursive deadlock during device state changes 

Device state change notifications are currently triggered while
holding 'mInterfaceMutex'. In case the camera provider tries
to add or remove a certain device/devices, then the provider
manager will try to recursively obtain the same 'mInterfaceMutex'.
Also avoid holding the 'mServiceLock' when notifying the
camera provider manager, access to its public interface
is already synchronized via 'mInterfaceMutex'.

Bug: 199240726
Test: Camera CTS
Change-Id: I9c71cc93ab91adc905488f954294b641d107de72
parent b7457908

services/camera/libcameraservice/CameraService.cpp

+0 −1
Original line number Original line Diff line number Diff line
@@ -2178,7 +2178,6 @@ Status CameraService::notifyDeviceStateChange(int64_t newState) { 
    newDeviceState |= vendorBits;

    newDeviceState |= vendorBits;





    ALOGV("%s: New device state 0x%" PRIx64, __FUNCTION__, newDeviceState);

    ALOGV("%s: New device state 0x%" PRIx64, __FUNCTION__, newDeviceState);

    Mutex::Autolock l(mServiceLock);

    mCameraProviderManager->notifyDeviceStateChange(newDeviceState);

    mCameraProviderManager->notifyDeviceStateChange(newDeviceState);





    return Status::ok();

    return Status::ok();

services/camera/libcameraservice/common/CameraProviderManager.cpp

+8 −0
Original line number Original line Diff line number Diff line
@@ -359,7 +359,13 @@ status_t CameraProviderManager::notifyDeviceStateChange( 
    for (auto& provider : mProviders) {

    for (auto& provider : mProviders) {

        ALOGV("%s: Notifying %s for new state 0x%" PRIx64,

        ALOGV("%s: Notifying %s for new state 0x%" PRIx64,

                __FUNCTION__, provider->mProviderName.c_str(), newState);

                __FUNCTION__, provider->mProviderName.c_str(), newState);

        // b/199240726 Camera providers can for example try to add/remove

        // camera devices as part of the state change notification. Holding

        // 'mInterfaceMutex' while calling 'notifyDeviceStateChange' can

        // result in a recursive deadlock.

        mInterfaceMutex.unlock();

        status_t singleRes = provider->notifyDeviceStateChange(mDeviceState);

        status_t singleRes = provider->notifyDeviceStateChange(mDeviceState);

        mInterfaceMutex.lock();

        if (singleRes != OK) {

        if (singleRes != OK) {

            ALOGE("%s: Unable to notify provider %s about device state change",

            ALOGE("%s: Unable to notify provider %s about device state change",

                    __FUNCTION__,

                    __FUNCTION__,
@@ -1185,10 +1191,12 @@ status_t CameraProviderManager::getSystemCameraKindLocked(const std::string& id, 
}

}





bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) const {

bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) const {

    std::lock_guard<std::mutex> lock(mInterfaceMutex);

    return isHiddenPhysicalCameraInternal(cameraId).first;

    return isHiddenPhysicalCameraInternal(cameraId).first;

}

}





status_t CameraProviderManager::filterSmallJpegSizes(const std::string& cameraId) {

status_t CameraProviderManager::filterSmallJpegSizes(const std::string& cameraId) {

    std::lock_guard<std::mutex> lock(mInterfaceMutex);

    for (auto& provider : mProviders) {

    for (auto& provider : mProviders) {

        for (auto& deviceInfo : provider->mDevices) {

        for (auto& deviceInfo : provider->mDevices) {

            if (deviceInfo->mId == cameraId) {

            if (deviceInfo->mId == cameraId) {

services/camera/libcameraservice/common/CameraProviderManager.h

+10 −6
Original line number Original line Diff line number Diff line
@@ -283,12 +283,6 @@ public: 
            /*out*/

            /*out*/

            sp<hardware::camera::device::V3_2::ICameraDeviceSession> *session);

            sp<hardware::camera::device::V3_2::ICameraDeviceSession> *session);





    /**

     * Save the ICameraProvider while it is being used by a camera or torch client

     */

    void saveRef(DeviceMode usageType, const std::string &cameraId,

            sp<hardware::camera::provider::V2_4::ICameraProvider> provider);



    /**

    /**

     * Notify that the camera or torch is no longer being used by a camera client

     * Notify that the camera or torch is no longer being used by a camera client

     */

     */
@@ -434,6 +428,10 @@ private: 




        /**

        /**

         * Notify provider about top-level device physical state changes

         * Notify provider about top-level device physical state changes

         *

         * Note that 'mInterfaceMutex' should not be held when calling this method.

         * It is possible for camera providers to add/remove devices and try to

         * acquire it.

         */

         */

        status_t notifyDeviceStateChange(

        status_t notifyDeviceStateChange(

                hardware::hidl_bitfield<hardware::camera::provider::V2_5::DeviceState>

                hardware::hidl_bitfield<hardware::camera::provider::V2_5::DeviceState>
@@ -662,6 +660,12 @@ private: 
                sp<hardware::camera::provider::V2_6::ICameraProvider> &interface2_6);

                sp<hardware::camera::provider::V2_6::ICameraProvider> &interface2_6);

    };

    };





    /**

     * Save the ICameraProvider while it is being used by a camera or torch client

     */

    void saveRef(DeviceMode usageType, const std::string &cameraId,

            sp<hardware::camera::provider::V2_4::ICameraProvider> provider);



    // Utility to find a DeviceInfo by ID; pointer is only valid while mInterfaceMutex is held

    // Utility to find a DeviceInfo by ID; pointer is only valid while mInterfaceMutex is held

    // and the calling code doesn't mutate the list of providers or their lists of devices.

    // and the calling code doesn't mutate the list of providers or their lists of devices.

    // Finds the first device of the given ID that falls within the requested version range

    // Finds the first device of the given ID that falls within the requested version range