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

Commit a8bf1c6e authored by Jayant Chowdhary's avatar Jayant Chowdhary
Browse files

camera2ndk: ~ACameraCaptureSession shouldn't hold device lock in ACameraDevice_close(). 



We call disconnectLocked before stopping CameraDevice's looper, in order to avoid this situation:

1) Its possible that an OnResultReceived callback is received, and posts an
   AMessage with sp<ACameraCaptureSession> on CameraDevice's looper.

2) Before the looper message is processsed, ACameraDevice_close() is
   called by the client, which results in the looper being stopped and
   cleared with device lock held.

3) When the looper is getting cleared, the AMessage containg the
   ACameraCaptureSession pointer is destructed leading to
   ~ACameraCaptureSession running without knowing that the device is
   being closed, as a result it tries to hold device lock, resulting in
   a deadlock.

Bug: 141603005

Test: CTS native tests
Test: use camera2 vndk client for extended periods of time

Change-Id: Ia0d47fc2975981055cd1f2103c1cbe8d76642fe4
Signed-off-by: default avatarJayant Chowdhary <jchowdhary@google.com>
parent 9ddd6e8d

camera/ndk/impl/ACameraDevice.cpp

+9 −15
Original line number Diff line number Diff line
@@ -29,7 +29,7 @@ 
#include "ACameraCaptureSession.inc"



ACameraDevice::~ACameraDevice() {

    mDevice->stopLooper();

    mDevice->stopLooperAndDisconnect();

}



namespace android {
@@ -112,19 +112,7 @@ CameraDevice::CameraDevice( 
    }

}



// Device close implementaiton

CameraDevice::~CameraDevice() {

    sp<ACameraCaptureSession> session = mCurrentSession.promote();

    {

        Mutex::Autolock _l(mDeviceLock);

        if (!isClosed()) {

            disconnectLocked(session);

        }

        LOG_ALWAYS_FATAL_IF(mCbLooper != nullptr,

                "CameraDevice looper should've been stopped before ~CameraDevice");

        mCurrentSession = nullptr;

    }

}

CameraDevice::~CameraDevice() { }



void

CameraDevice::postSessionMsgAndCleanup(sp<AMessage>& msg) {
@@ -892,8 +880,14 @@ CameraDevice::onCaptureErrorLocked( 
    return;

}



void CameraDevice::stopLooper() {

void CameraDevice::stopLooperAndDisconnect() {

    Mutex::Autolock _l(mDeviceLock);

    sp<ACameraCaptureSession> session = mCurrentSession.promote();

    if (!isClosed()) {

        disconnectLocked(session);

    }

    mCurrentSession = nullptr;



    if (mCbLooper != nullptr) {

      mCbLooper->unregisterHandler(mHandler->id());

      mCbLooper->stop();

camera/ndk/impl/ACameraDevice.h

+2 −1
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ 


#include <camera/NdkCameraManager.h>

#include <camera/NdkCameraCaptureSession.h>



#include "ACameraMetadata.h"



namespace android {
@@ -110,7 +111,7 @@ class CameraDevice final : public RefBase { 
    inline ACameraDevice* getWrapper() const { return mWrapper; };



    // Stop the looper thread and unregister the handler

    void stopLooper();

    void stopLooperAndDisconnect();



  private:

    friend ACameraCaptureSession;

camera/ndk/ndk_vendor/impl/ACameraDevice.cpp

+9 −15
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ 
using namespace android;



ACameraDevice::~ACameraDevice() {

    mDevice->stopLooper();

    mDevice->stopLooperAndDisconnect();

}



namespace android {
@@ -125,19 +125,7 @@ CameraDevice::CameraDevice( 
    }

}



// Device close implementaiton

CameraDevice::~CameraDevice() {

    sp<ACameraCaptureSession> session = mCurrentSession.promote();

    {

        Mutex::Autolock _l(mDeviceLock);

        if (!isClosed()) {

            disconnectLocked(session);

        }

        mCurrentSession = nullptr;

        LOG_ALWAYS_FATAL_IF(mCbLooper != nullptr,

            "CameraDevice looper should've been stopped before ~CameraDevice");

    }

}

CameraDevice::~CameraDevice() { }



void

CameraDevice::postSessionMsgAndCleanup(sp<AMessage>& msg) {
@@ -1388,6 +1376,7 @@ CameraDevice::checkAndFireSequenceCompleteLocked() { 
            // before cbh goes out of scope and causing we call the session

            // destructor while holding device lock

            cbh.mSession.clear();



            postSessionMsgAndCleanup(msg);

        }
@@ -1400,8 +1389,13 @@ CameraDevice::checkAndFireSequenceCompleteLocked() { 
    }

}



void CameraDevice::stopLooper() {

void CameraDevice::stopLooperAndDisconnect() {

    Mutex::Autolock _l(mDeviceLock);

    sp<ACameraCaptureSession> session = mCurrentSession.promote();

    if (!isClosed()) {

        disconnectLocked(session);

    }

    mCurrentSession = nullptr;

    if (mCbLooper != nullptr) {

      mCbLooper->unregisterHandler(mHandler->id());

      mCbLooper->stop();

camera/ndk/ndk_vendor/impl/ACameraDevice.h

+2 −1
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ 


#include <camera/NdkCameraManager.h>

#include <camera/NdkCameraCaptureSession.h>



#include "ACameraMetadata.h"

#include "utils.h"
@@ -134,7 +135,7 @@ class CameraDevice final : public RefBase { 
    inline ACameraDevice* getWrapper() const { return mWrapper; };



    // Stop the looper thread and unregister the handler

    void stopLooper();

    void stopLooperAndDisconnect();



  private:

    friend ACameraCaptureSession;