commit a8488c9c32de3d4cfa6f9b96bec362bbc337a91b
author Jayant Chowdhary <jchowdhary@google.com> Fri Jun 21 12:45:34 2019 -0700
committer Jayant Chowdhary <jchowdhary@google.com> Mon Jun 24 14:43:46 2019 -0700
tree a44151003b388a6950e52fc7e8cc465ce8e5e467
parent 50f9a1320b8dfff8732ca26c73a50b1950f2831e

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>

camera/ndk/impl/ACameraDevice.cpp

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) {
 }