camera2 ndk/vndk: cleanup->stop CameraDevice's looper in ~ACameraDevice()
It's possible that the following sequence happens:
1) hwbinder / binder thread T1: onResultReceived() starts -> promotes wp<CameraDevice> to sp<>;
2) Some other app thread T2 : ACameraDevice_close() -> delete ACameraDevice -> doesn't result in
CameraDevice's destructor running since mCameraDevice has another live
reference, app destroys some object O1.
3) T3 (callback looper thread): callback is received since looper is still running which accesses
dead app object O1 -> results in undefined behavior.
4) T1: onResultReceived completes and CameraDevice is destructed
We need to stop CameraDevice's looper thread (that waits for all callbacks queued to complete) in
~ACameraDevice() so we receive no callbacks after ACameraDevice is closed.
Bug: 135641415
Test: CTS native tests: no new failures
Test: AImageReaderVendorTest; enroll; while(1) auth;
Change-Id: Ia24de753f6ee409d941fff39616f09df2164880a
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>
diff --git a/camera/ndk/impl/ACameraDevice.cpp b/camera/ndk/impl/ACameraDevice.cpp
index 25a81eb..d24cb81 100644
--- a/camera/ndk/impl/ACameraDevice.cpp
+++ b/camera/ndk/impl/ACameraDevice.cpp
@@ -28,6 +28,10 @@
#include "ACameraCaptureSession.inc"
+ACameraDevice::~ACameraDevice() {
+ mDevice->stopLooper();
+}
+
namespace android {
namespace acam {
@@ -116,14 +120,10 @@
if (!isClosed()) {
disconnectLocked(session);
}
+ LOG_ALWAYS_FATAL_IF(mCbLooper != nullptr,
+ "CameraDevice looper should've been stopped before ~CameraDevice");
mCurrentSession = nullptr;
- if (mCbLooper != nullptr) {
- mCbLooper->unregisterHandler(mHandler->id());
- mCbLooper->stop();
- }
}
- mCbLooper.clear();
- mHandler.clear();
}
void
@@ -892,6 +892,16 @@
return;
}
+void CameraDevice::stopLooper() {
+ Mutex::Autolock _l(mDeviceLock);
+ if (mCbLooper != nullptr) {
+ mCbLooper->unregisterHandler(mHandler->id());
+ mCbLooper->stop();
+ }
+ mCbLooper.clear();
+ mHandler.clear();
+}
+
CameraDevice::CallbackHandler::CallbackHandler(const char* id) : mId(id) {
}
diff --git a/camera/ndk/impl/ACameraDevice.h b/camera/ndk/impl/ACameraDevice.h
index c92a95f..7a35bf0 100644
--- a/camera/ndk/impl/ACameraDevice.h
+++ b/camera/ndk/impl/ACameraDevice.h
@@ -109,6 +109,9 @@
inline ACameraDevice* getWrapper() const { return mWrapper; };
+ // Stop the looper thread and unregister the handler
+ void stopLooper();
+
private:
friend ACameraCaptureSession;
camera_status_t checkCameraClosedOrErrorLocked() const;
@@ -354,7 +357,7 @@
sp<ACameraMetadata> chars) :
mDevice(new android::acam::CameraDevice(id, cb, chars, this)) {}
- ~ACameraDevice() {};
+ ~ACameraDevice();
/*******************
* NDK public APIs *
diff --git a/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp b/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp
index 8a3bb46..35c8355 100644
--- a/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp
+++ b/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp
@@ -45,13 +45,7 @@
using namespace android;
ACameraDevice::~ACameraDevice() {
- Mutex::Autolock _l(mDevice->mDeviceLock);
- if (mDevice->mCbLooper != nullptr) {
- mDevice->mCbLooper->unregisterHandler(mDevice->mHandler->id());
- mDevice->mCbLooper->stop();
- }
- mDevice->mCbLooper.clear();
- mDevice->mHandler.clear();
+ mDevice->stopLooper();
}
namespace android {
@@ -140,13 +134,9 @@
disconnectLocked(session);
}
mCurrentSession = nullptr;
- if (mCbLooper != nullptr) {
- mCbLooper->unregisterHandler(mHandler->id());
- mCbLooper->stop();
- }
+ LOG_ALWAYS_FATAL_IF(mCbLooper != nullptr,
+ "CameraDevice looper should've been stopped before ~CameraDevice");
}
- mCbLooper.clear();
- mHandler.clear();
}
void
@@ -1410,6 +1400,16 @@
}
}
+void CameraDevice::stopLooper() {
+ Mutex::Autolock _l(mDeviceLock);
+ if (mCbLooper != nullptr) {
+ mCbLooper->unregisterHandler(mHandler->id());
+ mCbLooper->stop();
+ }
+ mCbLooper.clear();
+ mHandler.clear();
+}
+
/**
* Camera service callback implementation
*/
diff --git a/camera/ndk/ndk_vendor/impl/ACameraDevice.h b/camera/ndk/ndk_vendor/impl/ACameraDevice.h
index d8df568..9e034c4 100644
--- a/camera/ndk/ndk_vendor/impl/ACameraDevice.h
+++ b/camera/ndk/ndk_vendor/impl/ACameraDevice.h
@@ -133,6 +133,9 @@
bool setDeviceMetadataQueues();
inline ACameraDevice* getWrapper() const { return mWrapper; };
+ // Stop the looper thread and unregister the handler
+ void stopLooper();
+
private:
friend ACameraCaptureSession;
friend ACameraDevice;
