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

Commit d09801b9 authored by Eino-Ville Talvala's avatar Eino-Ville Talvala Committed by Igor Murashkin
Browse files

Camera2: Fix deadlock on shutdown due to client getting killed. 

When the binder connection dies and is the only holder of a strong
pointer to the Camera2Client, disconnect is called from the
destructor. At this point, all weak pointers to Camera2Client are no
longer promotable, and lots of cleanup code paths are broken as a
result.

Rework all such code paths to not need the client pointer, and to
discard image buffers that arrive during shutdown.

Bug: 8696047
Change-Id: Ic0672ecde7c1baaf65079f925a45bd5be45f1fb3
parent ec771223

services/camera/libcameraservice/Camera2Client.cpp

+18 −2
Original line number Diff line number Diff line
@@ -135,6 +135,7 @@ status_t Camera2Client::initialize(camera_module_t *module) 


Camera2Client::~Camera2Client() {

    ATRACE_CALL();

    ALOGV("~Camera2Client");



    mDestructionStarted = true;
@@ -369,6 +370,12 @@ void Camera2Client::disconnect() { 


    ALOGV("Camera %d: Shutting down", mCameraId);



    /**

     * disconnect() cannot call any methods that might need to promote a

     * wp<Camera2Client>, since disconnect can be called from the destructor, at

     * which point all such promotions will fail.

     */



    stopPreviewL();



    {
@@ -538,7 +545,12 @@ status_t Camera2Client::setPreviewWindowL(const sp<IBinder>& binder, 
            break;

        case Parameters::PREVIEW:

            // Already running preview - need to stop and create a new stream

            mStreamingProcessor->stopStream();

            res = stopStream();

            if (res != OK) {

                ALOGE("%s: Unable to stop preview to swap windows: %s (%d)",

                        __FUNCTION__, strerror(-res), res);

                return res;

            }

            state = Parameters::WAITING_FOR_PREVIEW_WINDOW;

            break;

    }
@@ -745,7 +757,11 @@ void Camera2Client::stopPreviewL() { 
            // no break

        case Parameters::RECORD:

        case Parameters::PREVIEW:

            mStreamingProcessor->stopStream();

            res = stopStream();

            if (res != OK) {

                ALOGE("%s: Camera %d: Can't stop streaming: %s (%d)",

                        __FUNCTION__, mCameraId, strerror(-res), res);

            }

            res = mDevice->waitUntilDrained();

            if (res != OK) {

                ALOGE("%s: Camera %d: Waiting to stop streaming failed: %s (%d)",

services/camera/libcameraservice/Camera2Device.cpp

+2 −1
Original line number Diff line number Diff line
@@ -1133,7 +1133,8 @@ cleanUpBuffers: 
status_t Camera2Device::StreamAdapter::release() {

    ATRACE_CALL();

    status_t res;

    ALOGV("%s: Releasing stream %d", __FUNCTION__, mId);

    ALOGV("%s: Releasing stream %d (%d x %d, format %d)", __FUNCTION__, mId,

            mWidth, mHeight, mFormat);

    if (mState >= ALLOCATED) {

        res = mHal2Device->ops->release_stream(mHal2Device, mId);

        if (res != OK) {

services/camera/libcameraservice/CameraService.cpp

+4 −0
Original line number Diff line number Diff line
@@ -793,6 +793,7 @@ CameraService::Client::Client(const sp<CameraService>& cameraService, 


// tear down the client

CameraService::Client::~Client() {

    ALOGV("~Client");

    mDestructionStarted = true;



    mCameraService->releaseSound();
@@ -820,10 +821,12 @@ CameraService::BasicClient::BasicClient(const sp<CameraService>& cameraService, 
}



CameraService::BasicClient::~BasicClient() {

    ALOGV("~BasicClient");

    mDestructionStarted = true;

}



void CameraService::BasicClient::disconnect() {

    ALOGV("BasicClient::disconnect");

    mCameraService->removeClientByRemote(mRemoteBinder);

    // client shouldn't be able to call into us anymore

    mClientPid = 0;
@@ -922,6 +925,7 @@ void CameraService::Client::notifyError() { 


// NOTE: function is idempotent

void CameraService::Client::disconnect() {

    ALOGV("Client::disconnect");

    BasicClient::disconnect();

    mCameraService->setCameraFree(mCameraId);

    mCameraService->updateStatus(ICameraServiceListener::STATUS_PRESENT,

services/camera/libcameraservice/camera2/CallbackProcessor.cpp

+55 −23
Original line number Diff line number Diff line
@@ -30,9 +30,11 @@ 
namespace android {

namespace camera2 {



CallbackProcessor::CallbackProcessor(wp<Camera2Client> client):

CallbackProcessor::CallbackProcessor(sp<Camera2Client> client):

        Thread(false),

        mClient(client),

        mDevice(client->getCameraDevice()),

        mId(client->getCameraId()),

        mCallbackAvailable(false),

        mCallbackStreamId(NO_STREAM) {

}
@@ -56,9 +58,11 @@ status_t CallbackProcessor::updateStream(const Parameters &params) { 


    Mutex::Autolock l(mInputMutex);



    sp<Camera2Client> client = mClient.promote();

    if (client == 0) return OK;

    sp<CameraDeviceBase> device = client->getCameraDevice();

    sp<CameraDeviceBase> device = mDevice.promote();

    if (device == 0) {

        ALOGE("%s: Camera %d: Device does not exist", __FUNCTION__, mId);

        return INVALID_OPERATION;

    }



    if (mCallbackConsumer == 0) {

        // Create CPU buffer queue endpoint
@@ -76,7 +80,7 @@ status_t CallbackProcessor::updateStream(const Parameters &params) { 
                &currentWidth, &currentHeight, &currentFormat);

        if (res != OK) {

            ALOGE("%s: Camera %d: Error querying callback output stream info: "

                    "%s (%d)", __FUNCTION__, client->getCameraId(),

                    "%s (%d)", __FUNCTION__, mId,

                    strerror(-res), res);

            return res;

        }
@@ -87,11 +91,11 @@ status_t CallbackProcessor::updateStream(const Parameters &params) { 
            // assuming that all existing use of old callback stream is

            // completed.

            ALOGV("%s: Camera %d: Deleting stream %d since the buffer dimensions changed",

                __FUNCTION__, client->getCameraId(), mCallbackStreamId);

                __FUNCTION__, mId, mCallbackStreamId);

            res = device->deleteStream(mCallbackStreamId);

            if (res != OK) {

                ALOGE("%s: Camera %d: Unable to delete old output stream "

                        "for callbacks: %s (%d)", __FUNCTION__, client->getCameraId(),

                        "for callbacks: %s (%d)", __FUNCTION__, mId,

                        strerror(-res), res);

                return res;

            }
@@ -108,7 +112,7 @@ status_t CallbackProcessor::updateStream(const Parameters &params) { 
                params.previewFormat, 0, &mCallbackStreamId);

        if (res != OK) {

            ALOGE("%s: Camera %d: Can't create output stream for callbacks: "

                    "%s (%d)", __FUNCTION__, client->getCameraId(),

                    "%s (%d)", __FUNCTION__, mId,

                    strerror(-res), res);

            return res;

        }
@@ -119,16 +123,25 @@ status_t CallbackProcessor::updateStream(const Parameters &params) { 


status_t CallbackProcessor::deleteStream() {

    ATRACE_CALL();

    sp<CameraDeviceBase> device;



    {

        Mutex::Autolock l(mInputMutex);



    if (mCallbackStreamId != NO_STREAM) {

        sp<Camera2Client> client = mClient.promote();

        if (client == 0) return OK;

        sp<CameraDeviceBase> device = client->getCameraDevice();



        if (mCallbackStreamId == NO_STREAM) {

            return OK;

        }

        device = mDevice.promote();

        if (device == 0) {

            ALOGE("%s: Camera %d: Device does not exist", __FUNCTION__, mId);

            return INVALID_OPERATION;

        }

    }

    device->deleteStream(mCallbackStreamId);



    {

        Mutex::Autolock l(mInputMutex);



        mCallbackHeap.clear();

        mCallbackWindow.clear();

        mCallbackConsumer.clear();
@@ -161,13 +174,32 @@ bool CallbackProcessor::threadLoop() { 


    do {

        sp<Camera2Client> client = mClient.promote();

        if (client == 0) return false;

        if (client == 0) {

            res = discardNewCallback();

        } else {

            res = processNewCallback(client);

        }

    } while (res == OK);



    return true;

}



status_t CallbackProcessor::discardNewCallback() {

    ATRACE_CALL();

    status_t res;

    CpuConsumer::LockedBuffer imgBuffer;

    res = mCallbackConsumer->lockNextBuffer(&imgBuffer);

    if (res != OK) {

        if (res != BAD_VALUE) {

            ALOGE("%s: Camera %d: Error receiving next callback buffer: "

                    "%s (%d)", __FUNCTION__, mId, strerror(-res), res);

        }

        return res;

    }

    mCallbackConsumer->unlockBuffer(imgBuffer);

    return OK;

}



status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) {

    ATRACE_CALL();

    status_t res;
@@ -181,12 +213,12 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 
    if (res != OK) {

        if (res != BAD_VALUE) {

            ALOGE("%s: Camera %d: Error receiving next callback buffer: "

                    "%s (%d)", __FUNCTION__, client->getCameraId(), strerror(-res), res);

                    "%s (%d)", __FUNCTION__, mId, strerror(-res), res);

        }

        return res;

    }

    ALOGV("%s: Camera %d: Preview callback available", __FUNCTION__,

            client->getCameraId());

            mId);



    {

        SharedParameters::Lock l(client->getParameters());
@@ -195,7 +227,7 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 
                && l.mParameters.state != Parameters::RECORD

                && l.mParameters.state != Parameters::VIDEO_SNAPSHOT) {

            ALOGV("%s: Camera %d: No longer streaming",

                    __FUNCTION__, client->getCameraId());

                    __FUNCTION__, mId);

            mCallbackConsumer->unlockBuffer(imgBuffer);

            return OK;

        }
@@ -216,7 +248,7 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 


        if (imgBuffer.format != l.mParameters.previewFormat) {

            ALOGE("%s: Camera %d: Unexpected format for callback: "

                    "%x, expected %x", __FUNCTION__, client->getCameraId(),

                    "%x, expected %x", __FUNCTION__, mId,

                    imgBuffer.format, l.mParameters.previewFormat);

            mCallbackConsumer->unlockBuffer(imgBuffer);

            return INVALID_OPERATION;
@@ -241,7 +273,7 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 
                "Camera2Client::CallbackHeap");

        if (mCallbackHeap->mHeap->getSize() == 0) {

            ALOGE("%s: Camera %d: Unable to allocate memory for callbacks",

                    __FUNCTION__, client->getCameraId());

                    __FUNCTION__, mId);

            mCallbackConsumer->unlockBuffer(imgBuffer);

            return INVALID_OPERATION;

        }
@@ -252,7 +284,7 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 


    if (mCallbackHeapFree == 0) {

        ALOGE("%s: Camera %d: No free callback buffers, dropping frame",

                __FUNCTION__, client->getCameraId());

                __FUNCTION__, mId);

        mCallbackConsumer->unlockBuffer(imgBuffer);

        return OK;

    }
@@ -282,7 +314,7 @@ status_t CallbackProcessor::processNewCallback(sp<Camera2Client> &client) { 
            l(client->mSharedCameraCallbacks);

        if (l.mRemoteCallback != 0) {

            ALOGV("%s: Camera %d: Invoking client data callback",

                    __FUNCTION__, client->getCameraId());

                    __FUNCTION__, mId);

            l.mRemoteCallback->dataCallback(CAMERA_MSG_PREVIEW_FRAME,

                    mCallbackHeap->mBuffers[heapIdx], NULL);

        }

services/camera/libcameraservice/camera2/CallbackProcessor.h

+6 −2
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
namespace android {



class Camera2Client;

class CameraDeviceBase;



namespace camera2 {
@@ -39,7 +40,7 @@ namespace camera2 { 
class CallbackProcessor:

            public Thread, public CpuConsumer::FrameAvailableListener {

  public:

    CallbackProcessor(wp<Camera2Client> client);

    CallbackProcessor(sp<Camera2Client> client);

    ~CallbackProcessor();



    void onFrameAvailable();
@@ -52,6 +53,8 @@ class CallbackProcessor: 
  private:

    static const nsecs_t kWaitDuration = 10000000; // 10 ms

    wp<Camera2Client> mClient;

    wp<CameraDeviceBase> mDevice;

    int mId;



    mutable Mutex mInputMutex;

    bool mCallbackAvailable;
@@ -72,7 +75,8 @@ class CallbackProcessor: 
    virtual bool threadLoop();



    status_t processNewCallback(sp<Camera2Client> &client);



    // Used when shutting down

    status_t discardNewCallback();

};