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

Commit 2f5d601b authored by Emilian Peev's avatar Emilian Peev
Browse files

Camera: Synchronize callback access to composite map 

Access to the camera device client composite map is not
thread safe during device callbacks. To avoid possible
deadlocks, synchronize access to the map via separate
dedicated lock.
Additionally correct the composite graphic producer query
when switching to offline mode.

Bug: 212496312
Test: Camera CTS, partner testing
Change-Id: I25b24466fbcf88a85d1c28b874812ff5c102c250
parent cfbaa7e2

services/camera/libcameraservice/api2/CameraDeviceClient.cpp

+30 −11
Original line number Diff line number Diff line
@@ -206,6 +206,7 @@ binder::Status CameraDeviceClient::insertGbpLocked(const sp<IGraphicBufferProduc 
    int compositeIdx;

    int idx = mStreamMap.indexOfKey(IInterface::asBinder(gbp));



    Mutex::Autolock l(mCompositeLock);

    // Trying to submit request with surface that wasn't created

    if (idx == NAME_NOT_FOUND) {

        ALOGE("%s: Camera %s: Tried to submit a request with a surface that"
@@ -640,6 +641,7 @@ binder::Status CameraDeviceClient::endConfigure(int operatingMode, 
        offlineStreamIds->clear();

        mDevice->getOfflineStreamIds(offlineStreamIds);



        Mutex::Autolock l(mCompositeLock);

        for (size_t i = 0; i < mCompositeStreamMap.size(); ++i) {

            err = mCompositeStreamMap.valueAt(i)->configureStream();

            if (err != OK) {
@@ -774,6 +776,7 @@ binder::Status CameraDeviceClient::deleteStream(int streamId) { 
            }

        }



        Mutex::Autolock l(mCompositeLock);

        for (size_t i = 0; i < mCompositeStreamMap.size(); ++i) {

            if (streamId == mCompositeStreamMap.valueAt(i)->getStreamId()) {

                compositeIndex = i;
@@ -812,6 +815,7 @@ binder::Status CameraDeviceClient::deleteStream(int streamId) { 
            }



            if (compositeIndex != NAME_NOT_FOUND) {

                Mutex::Autolock l(mCompositeLock);

                status_t ret;

                if ((ret = mCompositeStreamMap.valueAt(compositeIndex)->deleteStream())

                        != OK) {
@@ -935,6 +939,7 @@ binder::Status CameraDeviceClient::createStream( 
                &streamId, physicalCameraId, streamInfo.sensorPixelModesUsed, &surfaceIds,

                outputConfiguration.getSurfaceSetID(), isShared, isMultiResolution);

        if (err == OK) {

            Mutex::Autolock l(mCompositeLock);

            mCompositeStreamMap.add(IInterface::asBinder(surfaces[0]->getIGraphicBufferProducer()),

                    compositeStream);

        }
@@ -1754,8 +1759,9 @@ binder::Status CameraDeviceClient::switchToOffline( 
            return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.string());

        }



        Mutex::Autolock l(mCompositeLock);

        bool isCompositeStream = false;

        for (const auto& gbp : mConfiguredOutputs[streamId].getGraphicBufferProducers()) {

        for (const auto& gbp : mConfiguredOutputs.valueAt(index).getGraphicBufferProducers()) {

            sp<Surface> s = new Surface(gbp, false /*controlledByApp*/);

            isCompositeStream = camera3::DepthCompositeStream::isDepthCompositeStream(s) |

                camera3::HeicCompositeStream::isHeicCompositeStream(s);
@@ -1804,6 +1810,7 @@ binder::Status CameraDeviceClient::switchToOffline( 
        mConfiguredOutputs.clear();

        mDeferredStreams.clear();

        mStreamInfoMap.clear();

        Mutex::Autolock l(mCompositeLock);

        mCompositeStreamMap.clear();

        mInputStream = {false, 0, 0, 0, 0};

    } else {
@@ -1899,11 +1906,16 @@ void CameraDeviceClient::notifyError(int32_t errorCode, 
    // Thread safe. Don't bother locking.

    sp<hardware::camera2::ICameraDeviceCallbacks> remoteCb = getRemoteCallback();



    // Composites can have multiple internal streams. Error notifications coming from such internal

    // streams may need to remain within camera service.

    bool skipClientNotification = false;

    {

        // Access to the composite stream map must be synchronized

        Mutex::Autolock l(mCompositeLock);

        // Composites can have multiple internal streams. Error notifications coming from such

        // internal streams may need to remain within camera service.

        for (size_t i = 0; i < mCompositeStreamMap.size(); i++) {

        skipClientNotification |= mCompositeStreamMap.valueAt(i)->onError(errorCode, resultExtras);

            skipClientNotification |= mCompositeStreamMap.valueAt(i)->onError(errorCode,

                    resultExtras);

        }

    }



    if ((remoteCb != 0) && (!skipClientNotification)) {
@@ -1943,6 +1955,8 @@ void CameraDeviceClient::notifyShutter(const CaptureResultExtras& resultExtras, 
    }

    Camera2ClientBase::notifyShutter(resultExtras, timestamp);



    // Access to the composite stream map must be synchronized

    Mutex::Autolock l(mCompositeLock);

    for (size_t i = 0; i < mCompositeStreamMap.size(); i++) {

        mCompositeStreamMap.valueAt(i)->onShutter(resultExtras, timestamp);

    }
@@ -1992,6 +2006,8 @@ void CameraDeviceClient::detachDevice() { 
        }

    }



    {

        Mutex::Autolock l(mCompositeLock);

        for (size_t i = 0; i < mCompositeStreamMap.size(); i++) {

            auto ret = mCompositeStreamMap.valueAt(i)->deleteInternalStreams();

            if (ret != OK) {
@@ -2000,6 +2016,7 @@ void CameraDeviceClient::detachDevice() { 
            }

        }

        mCompositeStreamMap.clear();

    }



    Camera2ClientBase::detachDevice();
@@ -2019,6 +2036,8 @@ void CameraDeviceClient::onResultAvailable(const CaptureResult& result) { 
                result.mPhysicalMetadatas);

    }



    // Access to the composite stream map must be synchronized

    Mutex::Autolock l(mCompositeLock);

    for (size_t i = 0; i < mCompositeStreamMap.size(); i++) {

        mCompositeStreamMap.valueAt(i)->onResultAvailable(result);

    }

services/camera/libcameraservice/api2/CameraDeviceClient.h

+2 −0
Original line number Diff line number Diff line
@@ -339,6 +339,8 @@ private: 
    // set of high resolution camera id (logical / physical)

    std::unordered_set<std::string> mHighResolutionSensors;



    // Synchronize access to 'mCompositeStreamMap'

    Mutex mCompositeLock;

    KeyedVector<sp<IBinder>, sp<CompositeStream>> mCompositeStreamMap;



    sp<CameraProviderManager> mProviderManager;