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

Commit db1e864d authored by Yin-Chia Yeh's avatar Yin-Chia Yeh
Browse files

Camera: fix bufferFreed callback object lifecycle issue

Make sure the callback object won't be freed in the middle
of callback execution.

Test: CTS + stress test
Bug: 63683767
Change-Id: I6fb1b754cadb3d499c1c246687d2f60d444d00bb
parent 7bb115f1
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -155,7 +155,7 @@ status_t Camera3Device::initialize(sp<CameraProviderManager> manager) {
        return DEAD_OBJECT;
    }

    mInterface = std::make_unique<HalInterface>(session, queue);
    mInterface = new HalInterface(session, queue);
    std::string providerType;
    mVendorTagId = manager->getProviderTagIdLocked(mId.string());

@@ -184,7 +184,7 @@ status_t Camera3Device::initializeCommonLocked() {
    mTagMonitor.initialize(mVendorTagId);

    /** Start up request queue thread */
    mRequestThread = new RequestThread(this, mStatusTracker, mInterface.get());
    mRequestThread = new RequestThread(this, mStatusTracker, mInterface);
    res = mRequestThread->run(String8::format("C3Dev-%s-ReqQueue", mId.string()).string());
    if (res != OK) {
        SET_ERR_L("Unable to start request queue thread: %s (%d)",
@@ -3515,7 +3515,7 @@ void Camera3Device::HalInterface::onBufferFreed(

Camera3Device::RequestThread::RequestThread(wp<Camera3Device> parent,
        sp<StatusTracker> statusTracker,
        HalInterface* interface) :
        sp<HalInterface> interface) :
        Thread(/*canCallJava*/false),
        mParent(parent),
        mStatusTracker(statusTracker),
+3 −3
Original line number Diff line number Diff line
@@ -337,7 +337,7 @@ class Camera3Device :
        std::vector<std::pair<int, uint64_t>> mFreedBuffers;
    };

    std::unique_ptr<HalInterface> mInterface;
    sp<HalInterface> mInterface;

    CameraMetadata             mDeviceInfo;

@@ -628,7 +628,7 @@ class Camera3Device :

        RequestThread(wp<Camera3Device> parent,
                sp<camera3::StatusTracker> statusTracker,
                HalInterface* interface);
                sp<HalInterface> interface);
        ~RequestThread();

        void     setNotificationListener(wp<NotificationListener> listener);
@@ -778,7 +778,7 @@ class Camera3Device :

        wp<Camera3Device>  mParent;
        wp<camera3::StatusTracker>  mStatusTracker;
        HalInterface*      mInterface;
        sp<HalInterface>   mInterface;

        wp<NotificationListener> mListener;

+3 −2
Original line number Diff line number Diff line
@@ -293,8 +293,9 @@ status_t Camera3InputStream::getEndpointUsage(uint32_t *usage) const {
void Camera3InputStream::onBufferFreed(const wp<GraphicBuffer>& gb) {
    const sp<GraphicBuffer> buffer = gb.promote();
    if (buffer != nullptr) {
        if (mBufferFreedListener != nullptr) {
            mBufferFreedListener->onBufferFreed(mId, buffer->handle);
        sp<Camera3StreamBufferFreedListener> callback = mBufferFreedListener.promote();
        if (callback != nullptr) {
            callback->onBufferFreed(mId, buffer->handle);
        }
    } else {
        ALOGE("%s: GraphicBuffer is freed before onBufferFreed callback finishes!", __FUNCTION__);
+1 −1
Original line number Diff line number Diff line
@@ -727,7 +727,7 @@ void Camera3OutputStream::BufferReleasedListener::onBufferReleased() {

void Camera3OutputStream::onBuffersRemovedLocked(
        const std::vector<sp<GraphicBuffer>>& removedBuffers) {
    Camera3StreamBufferFreedListener* callback = mBufferFreedListener;
    sp<Camera3StreamBufferFreedListener> callback = mBufferFreedListener.promote();
    if (callback != nullptr) {
        for (auto gb : removedBuffers) {
            callback->onBufferFreed(mId, gb->handle);
+1 −1
Original line number Diff line number Diff line
@@ -744,7 +744,7 @@ void Camera3Stream::removeBufferListener(
}

void Camera3Stream::setBufferFreedListener(
        Camera3StreamBufferFreedListener* listener) {
        wp<Camera3StreamBufferFreedListener> listener) {
    Mutex::Autolock l(mLock);
    // Only allow set listener during stream configuration because stream is guaranteed to be IDLE
    // at this state, so setBufferFreedListener won't collide with onBufferFreed callbacks
Loading