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

Commit 4184a7dd authored by Shuzhen Wang's avatar Shuzhen Wang
Browse files

Camera: Fix deadlock in ACameraCaptureSession destructor 

mSequenceCallbackMap's entry should be cleared before calling
onSequenceCompletedCallback. This is to make sure the
ACameraCaptureSession object is destroyed in the callback thread.

The fix is for a regression introduced in ag/11835321.

Test: Camera CTS
Bug: 176034707
Change-Id: Ib38f4ca8ec499cd88f31b0be09f1db74e5ec35c6
parent 6bb359e9

camera/ndk/impl/ACameraDevice.cpp

+31 −29
Original line number Diff line number Diff line
@@ -1361,31 +1361,11 @@ CameraDevice::checkAndFireSequenceCompleteLocked() { 
                it->second.isSequenceCompleted = true;

            }



            if (it->second.isSequenceCompleted && hasCallback) {

                auto cbIt = mSequenceCallbackMap.find(sequenceId);

                CallbackHolder cbh = cbIt->second;



                // send seq complete callback

                sp<AMessage> msg = new AMessage(kWhatCaptureSeqEnd, mHandler);

                msg->setPointer(kContextKey, cbh.mContext);

                msg->setObject(kSessionSpKey, cbh.mSession);

                msg->setPointer(kCallbackFpKey, (void*) cbh.mOnCaptureSequenceCompleted);

                msg->setInt32(kSequenceIdKey, sequenceId);

                msg->setInt64(kFrameNumberKey, lastFrameNumber);



                // Clear the session sp before we send out the message

                // This will guarantee the rare case where the message is processed

                // before cbh goes out of scope and causing we call the session

                // destructor while holding device lock

                cbh.mSession.clear();

                postSessionMsgAndCleanup(msg);

            }

        }



        if (it->second.isSequenceCompleted && it->second.isInflightCompleted) {

            if (mSequenceCallbackMap.find(sequenceId) != mSequenceCallbackMap.end()) {

                mSequenceCallbackMap.erase(sequenceId);

            }

            sendCaptureSequenceCompletedLocked(sequenceId, lastFrameNumber);



            it = mSequenceLastFrameNumberMap.erase(it);

            ALOGV("%s: Remove holder for sequenceId %d", __FUNCTION__, sequenceId);

        } else {
@@ -1412,13 +1392,7 @@ CameraDevice::removeCompletedCallbackHolderLocked(int64_t lastCompletedRegularFr 
                lastCompletedRegularFrameNumber);

        if (lastFrameNumber <= lastCompletedRegularFrameNumber) {

            if (it->second.isSequenceCompleted) {

                // Check if there is callback for this sequence

                // This should not happen because we always register callback (with nullptr inside)

                if (mSequenceCallbackMap.count(sequenceId) == 0) {

                    ALOGW("No callback found for sequenceId %d", sequenceId);

                } else {

                    mSequenceCallbackMap.erase(sequenceId);

                }

                sendCaptureSequenceCompletedLocked(sequenceId, lastFrameNumber);



                it = mSequenceLastFrameNumberMap.erase(it);

                ALOGV("%s: Remove holder for sequenceId %d", __FUNCTION__, sequenceId);
@@ -1709,5 +1683,33 @@ CameraDevice::ServiceCallback::onRepeatingRequestError( 
    return ret;

}



void

CameraDevice::sendCaptureSequenceCompletedLocked(int sequenceId, int64_t lastFrameNumber) {

    auto cbIt = mSequenceCallbackMap.find(sequenceId);

    if (cbIt != mSequenceCallbackMap.end()) {

        CallbackHolder cbh = cbIt->second;

        mSequenceCallbackMap.erase(cbIt);



        // send seq complete callback

        sp<AMessage> msg = new AMessage(kWhatCaptureSeqEnd, mHandler);

        msg->setPointer(kContextKey, cbh.mContext);

        msg->setObject(kSessionSpKey, cbh.mSession);

        msg->setPointer(kCallbackFpKey, (void*) cbh.mOnCaptureSequenceCompleted);

        msg->setInt32(kSequenceIdKey, sequenceId);

        msg->setInt64(kFrameNumberKey, lastFrameNumber);



        // Clear the session sp before we send out the message

        // This will guarantee the rare case where the message is processed

        // before cbh goes out of scope and causing we call the session

        // destructor while holding device lock

        cbh.mSession.clear();

        postSessionMsgAndCleanup(msg);

    } else {

        // Check if there is callback for this sequence

        // This should not happen because we always register callback (with nullptr inside)

        ALOGW("No callback found for sequenceId %d", sequenceId);

    }

}



} // namespace acam

} // namespace android

camera/ndk/impl/ACameraDevice.h

+1 −0
Original line number Diff line number Diff line
@@ -354,6 +354,7 @@ class CameraDevice final : public RefBase { 
    void checkRepeatingSequenceCompleteLocked(const int sequenceId, const int64_t lastFrameNumber);

    void checkAndFireSequenceCompleteLocked();

    void removeCompletedCallbackHolderLocked(int64_t lastCompletedRegularFrameNumber);

    void sendCaptureSequenceCompletedLocked(int sequenceId, int64_t lastFrameNumber);



    // Misc variables

    int32_t mShadingMapSize[2];   // const after constructor