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

Commit 49c350e5 authored by Treehugger Robot's avatar Treehugger Robot Committed by Automerger Merge Worker
Browse files

Merge "cameraserver: fix deadlock scenario in torchModeStatusChanged callback." am: 2658976c 

Original change: https://android-review.googlesource.com/c/platform/frameworks/av/+/1865119

Change-Id: I0cf0a161a5fa194e6c3173d41e4b8f692ec79ea2
parents 021bbbdf 2658976c

services/camera/libcameraservice/CameraService.cpp

+7 −0
Original line number Diff line number Diff line
@@ -560,6 +560,13 @@ void CameraService::onTorchStatusChanged(const String8& cameraId, 
    onTorchStatusChangedLocked(cameraId, newStatus, systemCameraKind);

}





void CameraService::onTorchStatusChanged(const String8& cameraId,

        TorchModeStatus newStatus, SystemCameraKind systemCameraKind) {

    Mutex::Autolock al(mTorchStatusMutex);

    onTorchStatusChangedLocked(cameraId, newStatus, systemCameraKind);

}



void CameraService::onTorchStatusChangedLocked(const String8& cameraId,

        TorchModeStatus newStatus, SystemCameraKind systemCameraKind) {

    ALOGI("%s: Torch status changed for cameraId=%s, newStatus=%d",

services/camera/libcameraservice/CameraService.h

+8 −0
Original line number Diff line number Diff line
@@ -110,8 +110,16 @@ public: 
    virtual void        onDeviceStatusChanged(const String8 &cameraId,

            const String8 &physicalCameraId,

            hardware::camera::common::V1_0::CameraDeviceStatus newHalStatus) override;

    // This method may hold CameraProviderManager::mInterfaceMutex as a part

    // of calling getSystemCameraKind() internally. Care should be taken not to

    // directly / indirectly call this from callers who also hold

    // mInterfaceMutex.

    virtual void        onTorchStatusChanged(const String8& cameraId,

            hardware::camera::common::V1_0::TorchModeStatus newStatus) override;

    // Does not hold CameraProviderManager::mInterfaceMutex.

    virtual void        onTorchStatusChanged(const String8& cameraId,

            hardware::camera::common::V1_0::TorchModeStatus newStatus,

            SystemCameraKind kind) override;

    virtual void        onNewProviderRegistered() override;



    /////////////////////////////////////////////////////////////////////

services/camera/libcameraservice/common/CameraProviderManager.cpp

+15 −4
Original line number Diff line number Diff line
@@ -1950,16 +1950,19 @@ hardware::Return<void> CameraProviderManager::ProviderInfo::torchModeStatusChang 
        const hardware::hidl_string& cameraDeviceName,

        TorchModeStatus newStatus) {

    sp<StatusListener> listener;

    SystemCameraKind systemCameraKind = SystemCameraKind::PUBLIC;

    std::string id;

    {

        std::lock_guard<std::mutex> lock(mManager->mStatusListenerMutex);

    bool known = false;

    {

        // Hold mLock for accessing mDevices

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

        for (auto& deviceInfo : mDevices) {

            if (deviceInfo->mName == cameraDeviceName) {

                ALOGI("Camera device %s torch status is now %s", cameraDeviceName.c_str(),

                        torchStatusToString(newStatus));

                id = deviceInfo->mId;

                known = true;

                systemCameraKind = deviceInfo->mSystemCameraKind;

                if (TorchModeStatus::AVAILABLE_ON != newStatus) {

                    mManager->removeRef(DeviceMode::TORCH, id);

                }
@@ -1971,11 +1974,19 @@ hardware::Return<void> CameraProviderManager::ProviderInfo::torchModeStatusChang 
                    mProviderName.c_str(), cameraDeviceName.c_str(), newStatus);

            return hardware::Void();

        }

        // no lock needed since listener is set up only once during

        // CameraProviderManager initialization and then never changed till it is

        // destructed.

        listener = mManager->getStatusListener();

     }

    // Call without lock held to allow reentrancy into provider manager

    // The problem with holding mLock here is that we

    // might be limiting re-entrancy : CameraService::onTorchStatusChanged calls

    // back into CameraProviderManager which might try to hold mLock again (eg:

    // findDeviceInfo, which should be holding mLock while iterating through

    // each provider's devices).

    if (listener != nullptr) {

        listener->onTorchStatusChanged(String8(id.c_str()), newStatus);

        listener->onTorchStatusChanged(String8(id.c_str()), newStatus, systemCameraKind);

    }

    return hardware::Void();

}

services/camera/libcameraservice/common/CameraProviderManager.h

+3 −2
Original line number Diff line number Diff line
@@ -155,6 +155,9 @@ public: 
        virtual void onDeviceStatusChanged(const String8 &cameraId,

                const String8 &physicalCameraId,

                hardware::camera::common::V1_0::CameraDeviceStatus newStatus) = 0;

        virtual void onTorchStatusChanged(const String8 &cameraId,

                hardware::camera::common::V1_0::TorchModeStatus newStatus,

                SystemCameraKind kind) = 0;

        virtual void onTorchStatusChanged(const String8 &cameraId,

                hardware::camera::common::V1_0::TorchModeStatus newStatus) = 0;

        virtual void onNewProviderRegistered() = 0;
@@ -336,8 +339,6 @@ private: 
    // All private members, unless otherwise noted, expect mInterfaceMutex to be locked before use

    mutable std::mutex mInterfaceMutex;



    // the status listener update callbacks will lock mStatusMutex

    mutable std::mutex mStatusListenerMutex;

    wp<StatusListener> mListener;

    ServiceInteractionProxy* mServiceProxy;

services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp

+2 −0
Original line number Diff line number Diff line
@@ -274,6 +274,8 @@ struct TestStatusListener : public CameraProviderManager::StatusListener { 
            hardware::camera::common::V1_0::CameraDeviceStatus) override {}

    void onTorchStatusChanged(const String8 &,

            hardware::camera::common::V1_0::TorchModeStatus) override {}

    void onTorchStatusChanged(const String8 &,

            hardware::camera::common::V1_0::TorchModeStatus, SystemCameraKind) override {}

    void onNewProviderRegistered() override {}

};