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

Commit 886ac21e authored by Emilian Peev's avatar Emilian Peev
Browse files

Camera: Fix offline session close locking 

The HIDL/AIDL session close is expected to trigger processCaptureResult
callbacks. Currently both methods will try to acquire the same
'mLock' which can result in a potential deadlock.
Revert back to the previous locking scheme where session close and
release are synchronized via 'mInterfaceLock' and 'mLock'
respectively.
Additionally handle failing offline client registration by
ensuring that the client is initialized before it gets released.
This will allow CameraHals to return/fail any pending offline
requests.

Bug: 267723307
Test: Camera CTS

Change-Id: Ib55d654bfa213ec2df0fc4590017f77c4492b28e
parent 1753209c

services/camera/libcameraservice/api2/CameraDeviceClient.cpp

+4 −0
Original line number Diff line number Diff line
@@ -1851,6 +1851,10 @@ binder::Status CameraDeviceClient::switchToOffline( 
        mCompositeStreamMap.clear();

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

    } else {

        // In case we failed to register the offline client, ensure that it still initialized

        // so that all failing requests can return back correctly once the object is released.

        offlineClient->initialize(nullptr /*cameraProviderManager*/, String8()/*monitorTags*/);



        switch(ret) {

            case BAD_VALUE:

                return STATUS_ERROR_FMT(CameraService::ERROR_ILLEGAL_ARGUMENT,

services/camera/libcameraservice/api2/CameraOfflineSessionClient.cpp

+5 −0
Original line number Diff line number Diff line
@@ -29,6 +29,11 @@ using binder::Status; 
status_t CameraOfflineSessionClient::initialize(sp<CameraProviderManager>, const String8&) {

    ATRACE_CALL();



    if (mFrameProcessor.get() != nullptr) {

        // Already initialized

        return OK;

    }



    // Verify ops permissions

    auto res = startCameraOps();

    if (res != OK) {

services/camera/libcameraservice/device3/Camera3OfflineSession.cpp

+3 −2
Original line number Diff line number Diff line
@@ -81,7 +81,6 @@ Camera3OfflineSession::Camera3OfflineSession(const String8 &id, 
Camera3OfflineSession::~Camera3OfflineSession() {

    ATRACE_CALL();

    ALOGV("%s: Tearing down offline session for camera id %s", __FUNCTION__, mId.string());

    disconnectImpl();

}



const String8& Camera3OfflineSession::getId() const {
@@ -96,7 +95,6 @@ status_t Camera3OfflineSession::dump(int /*fd*/) { 


status_t Camera3OfflineSession::disconnect() {

    ATRACE_CALL();

    disconnectSession();

    return disconnectImpl();

}
@@ -132,6 +130,8 @@ status_t Camera3OfflineSession::disconnectImpl() { 
        streams.push_back(mInputStream);

    }



    closeSessionLocked();



    FlushInflightReqStates states {

        mId, mOfflineReqsLock, mOfflineReqs, mUseHalBufManager,

        listener, *this, mBufferRecords, *this, mSessionStatsBuilder};
@@ -140,6 +140,7 @@ status_t Camera3OfflineSession::disconnectImpl() { 


    {

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

        releaseSessionLocked();

        mOutputStreams.clear();

        mInputStream.clear();

        mStatus = STATUS_CLOSED;

services/camera/libcameraservice/device3/Camera3OfflineSession.h

+6 −1
Original line number Diff line number Diff line
@@ -274,7 +274,12 @@ class Camera3OfflineSession : 
    void setErrorStateLockedV(const char *fmt, va_list args);



    status_t disconnectImpl();

    virtual void disconnectSession() = 0;



    // Clients need to ensure that 'mInterfaceLock' is acquired before calling this method

    virtual void closeSessionLocked() = 0;



    // Clients need to ensure that 'mLock' is acquired before calling this method

    virtual void releaseSessionLocked() = 0;



}; // class Camera3OfflineSession

services/camera/libcameraservice/device3/aidl/AidlCamera3OfflineSession.cpp

+12 −7
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ namespace android { 
AidlCamera3OfflineSession::~AidlCamera3OfflineSession() {

    ATRACE_CALL();

    ALOGV("%s: Tearing down aidl offline session for camera id %s", __FUNCTION__, mId.string());

    AidlCamera3OfflineSession::disconnectSession();

    Camera3OfflineSession::disconnectImpl();

}



status_t AidlCamera3OfflineSession::initialize(wp<NotificationListener> listener) {
@@ -245,11 +245,16 @@ status_t AidlCamera3OfflineSession::initialize(wp<NotificationListener> listener 
    return ::ndk::ScopedAStatus::ok();

}



void AidlCamera3OfflineSession::disconnectSession() {

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

void AidlCamera3OfflineSession::closeSessionLocked() {

    if (mSession != nullptr) {

      mSession->close();

        auto err = mSession->close();

        if (!err.isOk()) {

            ALOGE("%s: Close transaction error: %s", __FUNCTION__, err.getDescription().c_str());

        }

    }

}



void AidlCamera3OfflineSession::releaseSessionLocked() {

    mSession.reset();

}