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

Commit 5681f787 authored by Bharatt Kukreja's avatar Bharatt Kukreja
Browse files

Revert "Add mInterfaceLock back to Camera3Device methods called by RequestThread." 

This reverts commit 646c31b0.

Reason for revert: mInterfaceLock should not be held by methods which don't make a HAL interface call. It leads to a deadlock bw threadLoop and methods like createInputStream in scenarios where both try to trigger reconfiguration (for eg on updating session parameters).

Bug: 241137777
Test: Camera CTS

Change-Id: I818420a0a02caaf616dabc9f21ed8487a5674890
parent 34e55a46

services/camera/libcameraservice/device3/Camera3Device.cpp

+78 −80
Original line number Diff line number Diff line
@@ -265,12 +265,12 @@ status_t Camera3Device::disconnect() { 


status_t Camera3Device::disconnectImpl() {

    ATRACE_CALL();

    Mutex::Autolock il(mInterfaceLock);



    ALOGI("%s: E", __FUNCTION__);



    status_t res = OK;

    std::vector<wp<Camera3StreamInterface>> streams;

    {

        Mutex::Autolock il(mInterfaceLock);

    nsecs_t maxExpectedDuration = getExpectedInFlightDuration();

    {

        Mutex::Autolock l(mLock);
@@ -320,19 +320,16 @@ status_t Camera3Device::disconnectImpl() { 
            streams.push_back(mInputStream);

        }

    }

    }

    // Joining done without holding mLock and mInterfaceLock, otherwise deadlocks may ensue

    // as the threads try to access parent state (b/143513518)



    // Joining done without holding mLock, otherwise deadlocks may ensue

    // as the threads try to access parent state

    if (mRequestThread != NULL && mStatus != STATUS_ERROR) {

        // HAL may be in a bad state, so waiting for request thread

        // (which may be stuck in the HAL processCaptureRequest call)

        // could be dangerous.

        // give up mInterfaceLock here and then lock it again. Could this lead

        // to other deadlocks

        mRequestThread->join();

    }

    {

        Mutex::Autolock il(mInterfaceLock);



    if (mStatusTracker != NULL) {

        mStatusTracker->join();

    }
@@ -373,7 +370,6 @@ status_t Camera3Device::disconnectImpl() { 
                    __FUNCTION__, stream->getId(), stream->getStrongCount() - 1);

        }

    }

    }

    ALOGI("%s: X", __FUNCTION__);



    if (mCameraServiceWatchdog != NULL) {
@@ -2310,7 +2306,9 @@ bool Camera3Device::reconfigureCamera(const CameraMetadata& sessionParams, int c 


    nsecs_t startTime = systemTime();



    Mutex::Autolock il(mInterfaceLock);

    // We must not hold mInterfaceLock here since this function is called from

    // RequestThread::threadLoop and holding mInterfaceLock could lead to

    // deadlocks (http://b/143513518)

    nsecs_t maxExpectedDuration = getExpectedInFlightDuration();



    Mutex::Autolock l(mLock);

services/camera/libcameraservice/device3/Camera3Device.h

+1 −0
Original line number Diff line number Diff line
@@ -366,6 +366,7 @@ class Camera3Device : 


    // A lock to enforce serialization on the input/configure side

    // of the public interface.

    // Only locked by public methods inherited from CameraDeviceBase.

    // Not locked by methods guarded by mOutputLock, since they may act

    // concurrently to the input/configure side of the interface.

    // Must be locked before mLock if both will be locked by a method