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

Commit 393eb796 authored by Dave Sparks's avatar Dave Sparks
Browse files

Don't callback on NULL client. Bug 2180510. 

We weren't checking to see if there was a valid camera client when
calling the notify callback function. Now we grab a strong pointer
before the callback to guarantee that the client is not destroyed
before we complete the callback. This change also fixes other
places in the code where we weren't holding a local strong pointer.
parent 39c999a5

camera/libcameraservice/CameraService.cpp

+60 −27
Original line number Diff line number Diff line
@@ -309,7 +309,7 @@ status_t CameraService::Client::connect(const sp<ICameraClient>& client) 
            oldClient = mCameraClient;



            // did the client actually change?

            if (client->asBinder() == mCameraClient->asBinder()) {

            if ((mCameraClient != NULL) && (client->asBinder() == mCameraClient->asBinder())) {

                LOGD("Connect to the same client");

                return NO_ERROR;

            }
@@ -878,7 +878,10 @@ void CameraService::Client::handleShutter() 
        mSurface->unregisterBuffers();

    }



    mCameraClient->notifyCallback(CAMERA_MSG_SHUTTER, 0, 0);

    sp<ICameraClient> c = mCameraClient;

    if (c != NULL) {

        c->notifyCallback(CAMERA_MSG_SHUTTER, 0, 0);

    }

    mHardware->disableMsgType(CAMERA_MSG_SHUTTER);



    // It takes some time before yuvPicture callback to be called.
@@ -932,31 +935,38 @@ void CameraService::Client::handlePreviewData(const sp<IMemory>& mem) 
        }

    }



    // Is the callback enabled or not?

    if (!(mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ENABLE_MASK)) {

    // local copy of the callback flags

    int flags = mPreviewCallbackFlag;



    // is callback enabled?

    if (!(flags & FRAME_CALLBACK_FLAG_ENABLE_MASK)) {

        // If the enable bit is off, the copy-out and one-shot bits are ignored

        LOGV("frame callback is diabled");

        return;

    }



    // Is the received frame copied out or not?

    if (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_COPY_OUT_MASK) {

        LOGV("frame is copied out");

        copyFrameAndPostCopiedFrame(heap, offset, size);

    } else {

        LOGV("frame is directly sent out without copying");

        mCameraClient->dataCallback(CAMERA_MSG_PREVIEW_FRAME, mem);

    }

    // hold a strong pointer to the client

    sp<ICameraClient> c = mCameraClient;



    // Is this is one-shot only?

    if (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ONE_SHOT_MASK) {

        LOGV("One-shot only, thus clear the bits and disable frame callback");

    // clear callback flags if no client or one-shot mode

    if ((c == NULL) || (mPreviewCallbackFlag & FRAME_CALLBACK_FLAG_ONE_SHOT_MASK)) {

        LOGV("Disable preview callback");

        mPreviewCallbackFlag &= ~(FRAME_CALLBACK_FLAG_ONE_SHOT_MASK |

                                FRAME_CALLBACK_FLAG_COPY_OUT_MASK |

                                FRAME_CALLBACK_FLAG_ENABLE_MASK);

        // TODO: Shouldn't we use this API for non-overlay hardware as well?

        if (mUseOverlay)

            mHardware->disableMsgType(CAMERA_MSG_PREVIEW_FRAME);

    }



    // Is the received frame copied out or not?

    if (flags & FRAME_CALLBACK_FLAG_COPY_OUT_MASK) {

        LOGV("frame is copied");

        copyFrameAndPostCopiedFrame(c, heap, offset, size);

    } else {

        LOGV("frame is forwarded");

        c->dataCallback(CAMERA_MSG_PREVIEW_FRAME, mem);

    }

}



// picture callback - postview image ready
@@ -972,7 +982,10 @@ void CameraService::Client::handlePostview(const sp<IMemory>& mem) 
    }

#endif



    mCameraClient->dataCallback(CAMERA_MSG_POSTVIEW_FRAME, mem);

    sp<ICameraClient> c = mCameraClient;

    if (c != NULL) {

        c->dataCallback(CAMERA_MSG_POSTVIEW_FRAME, mem);

    }

    mHardware->disableMsgType(CAMERA_MSG_POSTVIEW_FRAME);

}
@@ -997,7 +1010,10 @@ void CameraService::Client::handleRawPicture(const sp<IMemory>& mem) 
        mSurface->postBuffer(offset);

    }



    mCameraClient->dataCallback(CAMERA_MSG_RAW_IMAGE, mem);

    sp<ICameraClient> c = mCameraClient;

    if (c != NULL) {

        c->dataCallback(CAMERA_MSG_RAW_IMAGE, mem);

    }

    mHardware->disableMsgType(CAMERA_MSG_RAW_IMAGE);

}
@@ -1014,7 +1030,10 @@ void CameraService::Client::handleCompressedPicture(const sp<IMemory>& mem) 
    }

#endif



    mCameraClient->dataCallback(CAMERA_MSG_COMPRESSED_IMAGE, mem);

    sp<ICameraClient> c = mCameraClient;

    if (c != NULL) {

        c->dataCallback(CAMERA_MSG_COMPRESSED_IMAGE, mem);

    }

    mHardware->disableMsgType(CAMERA_MSG_COMPRESSED_IMAGE);

}
@@ -1032,7 +1051,10 @@ void CameraService::Client::notifyCallback(int32_t msgType, int32_t ext1, int32_ 
            client->handleShutter();

            break;

        default:

            client->mCameraClient->notifyCallback(msgType, ext1, ext2);

            sp<ICameraClient> c = client->mCameraClient;

            if (c != NULL) {

                c->notifyCallback(msgType, ext1, ext2);

            }

            break;

    }
@@ -1053,10 +1075,13 @@ void CameraService::Client::dataCallback(int32_t msgType, const sp<IMemory>& dat 
        return;

    }



    sp<ICameraClient> c = client->mCameraClient;

    if (dataPtr == NULL) {

        LOGE("Null data returned in data callback");

        client->mCameraClient->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);

        client->mCameraClient->dataCallback(msgType, NULL);

        if (c != NULL) {

            c->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);

            c->dataCallback(msgType, NULL);

        }

        return;

    }
@@ -1074,7 +1099,9 @@ void CameraService::Client::dataCallback(int32_t msgType, const sp<IMemory>& dat 
            client->handleCompressedPicture(dataPtr);

            break;

        default:

            client->mCameraClient->dataCallback(msgType, dataPtr);

            if (c != NULL) {

                c->dataCallback(msgType, dataPtr);

            }

            break;

    }
@@ -1095,15 +1122,20 @@ void CameraService::Client::dataCallbackTimestamp(nsecs_t timestamp, int32_t msg 
    if (client == 0) {

        return;

    }

    sp<ICameraClient> c = client->mCameraClient;



    if (dataPtr == NULL) {

        LOGE("Null data returned in data with timestamp callback");

        client->mCameraClient->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);

        client->mCameraClient->dataCallbackTimestamp(0, msgType, NULL);

        if (c != NULL) {

            c->notifyCallback(CAMERA_MSG_ERROR, UNKNOWN_ERROR, 0);

            c->dataCallbackTimestamp(0, msgType, NULL);

        }

        return;

    }



    client->mCameraClient->dataCallbackTimestamp(timestamp, msgType, dataPtr);

    if (c != NULL) {

        c->dataCallbackTimestamp(timestamp, msgType, dataPtr);

    }



#if DEBUG_CLIENT_REFERENCES

    if (client->getStrongCount() == 1) {
@@ -1161,7 +1193,8 @@ status_t CameraService::Client::sendCommand(int32_t cmd, int32_t arg1, int32_t a 
    return mHardware->sendCommand(cmd, arg1, arg2);

}



void CameraService::Client::copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, size_t offset, size_t size)

void CameraService::Client::copyFrameAndPostCopiedFrame(const sp<ICameraClient>& client,

        const sp<IMemoryHeap>& heap, size_t offset, size_t size)

{

    LOGV("copyFrameAndPostCopiedFrame");

    // It is necessary to copy out of pmem before sending this to
@@ -1186,7 +1219,7 @@ void CameraService::Client::copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, si 
        LOGE("failed to allocate space for frame callback");

        return;

    }

    mCameraClient->dataCallback(CAMERA_MSG_PREVIEW_FRAME, frame);

    client->dataCallback(CAMERA_MSG_PREVIEW_FRAME, frame);

}



status_t CameraService::dump(int fd, const Vector<String16>& args)

camera/libcameraservice/CameraService.h

+3 −3
Original line number Diff line number Diff line
@@ -23,10 +23,9 @@ 
#include <ui/CameraHardwareInterface.h>

#include <ui/Camera.h>



class android::MemoryHeapBase;



namespace android {



class MemoryHeapBase;

class MediaPlayer;



// ----------------------------------------------------------------------------
@@ -151,7 +150,8 @@ private: 
                    void        handleRawPicture(const sp<IMemory>&);

                    void        handleCompressedPicture(const sp<IMemory>&);



                    void        copyFrameAndPostCopiedFrame(sp<IMemoryHeap> heap, size_t offset, size_t size);

                    void        copyFrameAndPostCopiedFrame(const sp<ICameraClient>& client,

                                    const sp<IMemoryHeap>& heap, size_t offset, size_t size);



        // camera operation mode

        enum camera_mode {