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
Merged-In: Ia24de753f6ee409d941fff39616f09df2164880a
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>
(cherry picked from commit 174084011ca8b593a8cf35412928517b9e864be9)
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 529c167..24b1cae 100644
--- a/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp
+++ b/camera/ndk/ndk_vendor/impl/ACameraDevice.cpp
@@ -33,6 +33,10 @@
using namespace android;
+ACameraDevice::~ACameraDevice() {
+ mDevice->stopLooper();
+}
+
namespace android {
namespace acam {
@@ -119,13 +123,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
@@ -1422,6 +1422,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 829b084..3328a85 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;
@@ -383,7 +386,7 @@
sp<ACameraMetadata> chars) :
mDevice(new android::acam::CameraDevice(id, cb, std::move(chars), this)) {}
- ~ACameraDevice() {};
+ ~ACameraDevice();
/*******************
* NDK public APIs *