Merge changes from topic 'sensor_looper_crash' into oc-dev

* changes:
  Fix race condition for SensorManager looper.
  SensorManager: attach background thread to JavaVM
  Framework sensor manager init not depends on ::android::SensorManager.

2 files changed, 50 insertions, 11 deletions

diff --git a/services/sensorservice/hidl/SensorManager.cpp b/services/sensorservice/hidl/SensorManager.cpp
index 06ff95cdc8..25a3dc50d2 100644
--- a/services/sensorservice/hidl/SensorManager.cpp
+++ b/services/sensorservice/hidl/SensorManager.cpp
@@ -42,9 +42,10 @@ using ::android::hardware::hidl_vec;
 using ::android::hardware::Void;
 using ::android::sp;
 
-SensorManager::SensorManager()
-        : mInternalManager{::android::SensorManager::getInstanceForPackage(
-            String16(ISensorManager::descriptor))} {
+static const char* POLL_THREAD_NAME = "hidl_ssvc_poll";
+
+SensorManager::SensorManager(JavaVM* vm)
+        : mJavaVm(vm) {
 }
 
 SensorManager::~SensorManager() {
@@ -58,7 +59,7 @@ SensorManager::~SensorManager() {
 // Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow.
 Return<void> SensorManager::getSensorList(getSensorList_cb _hidl_cb) {
     ::android::Sensor const* const* list;
-    ssize_t count = mInternalManager.getSensorList(&list);
+    ssize_t count = getInternalManager().getSensorList(&list);
     if (count < 0 || !list) {
         LOG(ERROR) << "::android::SensorManager::getSensorList encounters " << count;
         _hidl_cb({}, Result::UNKNOWN_ERROR);
@@ -74,7 +75,7 @@ Return<void> SensorManager::getSensorList(getSensorList_cb _hidl_cb) {
 }
 
 Return<void> SensorManager::getDefaultSensor(SensorType type, getDefaultSensor_cb _hidl_cb) {
-    ::android::Sensor const* sensor = mInternalManager.getDefaultSensor(static_cast<int>(type));
+    ::android::Sensor const* sensor = getInternalManager().getDefaultSensor(static_cast<int>(type));
     if (!sensor) {
         _hidl_cb({}, Result::NOT_EXIST);
         return Void();
@@ -110,7 +111,7 @@ Return<void> SensorManager::createAshmemDirectChannel(
         return Void();
     }
 
-    createDirectChannel(mInternalManager, size, SENSOR_DIRECT_MEM_TYPE_ASHMEM,
+    createDirectChannel(getInternalManager(), size, SENSOR_DIRECT_MEM_TYPE_ASHMEM,
             mem.handle(), _hidl_cb);
 
     return Void();
@@ -120,7 +121,7 @@ Return<void> SensorManager::createGrallocDirectChannel(
         const hidl_handle& buffer, uint64_t size,
         createGrallocDirectChannel_cb _hidl_cb) {
 
-    createDirectChannel(mInternalManager, size, SENSOR_DIRECT_MEM_TYPE_GRALLOC,
+    createDirectChannel(getInternalManager(), size, SENSOR_DIRECT_MEM_TYPE_GRALLOC,
             buffer.getNativeHandle(), _hidl_cb);
 
     return Void();
@@ -132,7 +133,7 @@ sp<::android::Looper> SensorManager::getLooper() {
     if (mLooper == nullptr) {
         std::condition_variable looperSet;
 
-        std::thread{[&mutex = mLooperMutex, &looper = mLooper, &looperSet] {
+        std::thread{[&mutex = mLooperMutex, &looper = mLooper, &looperSet, javaVm = mJavaVm] {
 
             struct sched_param p = {0};
             p.sched_priority = 10;
@@ -142,14 +143,35 @@ sp<::android::Looper> SensorManager::getLooper() {
             }
 
             std::unique_lock<std::mutex> lock(mutex);
+            if (looper != nullptr) {
+                LOG(INFO) << "Another thread has already set the looper, exiting this one.";
+                return;
+            }
             looper = Looper::prepare(ALOOPER_PREPARE_ALLOW_NON_CALLBACKS /* opts */);
             lock.unlock();
 
+            // Attach the thread to JavaVM so that pollAll do not crash if the event
+            // is from Java.
+            JavaVMAttachArgs args{
+                .version = JNI_VERSION_1_2,
+                .name = POLL_THREAD_NAME,
+                .group = NULL
+            };
+            JNIEnv* env;
+            if (javaVm->AttachCurrentThread(&env, &args) != JNI_OK) {
+                LOG(FATAL) << "Cannot attach SensorManager looper thread to Java VM.";
+            }
+
             looperSet.notify_one();
             int pollResult = looper->pollAll(-1 /* timeout */);
             if (pollResult != ALOOPER_POLL_WAKE) {
                 LOG(ERROR) << "Looper::pollAll returns unexpected " << pollResult;
             }
+
+            if (javaVm->DetachCurrentThread() != JNI_OK) {
+                LOG(ERROR) << "Cannot detach SensorManager looper thread from Java VM.";
+            }
+
             LOG(INFO) << "Looper thread is terminated.";
         }}.detach();
         looperSet.wait(lock, [this]{ return this->mLooper != nullptr; });
@@ -157,6 +179,15 @@ sp<::android::Looper> SensorManager::getLooper() {
     return mLooper;
 }
 
+::android::SensorManager& SensorManager::getInternalManager() {
+    std::lock_guard<std::mutex> lock(mInternalManagerMutex);
+    if (mInternalManager == nullptr) {
+        mInternalManager = &::android::SensorManager::getInstanceForPackage(
+                String16(ISensorManager::descriptor));
+    }
+    return *mInternalManager;
+}
+
 Return<void> SensorManager::createEventQueue(
         const sp<IEventQueueCallback> &callback, createEventQueue_cb _hidl_cb) {
     if (callback == nullptr) {
@@ -165,7 +196,7 @@ Return<void> SensorManager::createEventQueue(
     }
 
     sp<::android::Looper> looper = getLooper();
-    sp<::android::SensorEventQueue> internalQueue = mInternalManager.createEventQueue();
+    sp<::android::SensorEventQueue> internalQueue = getInternalManager().createEventQueue();
     if (internalQueue == nullptr) {
         LOG(WARNING) << "::android::SensorManager::createEventQueue returns nullptr.";
         _hidl_cb(nullptr, Result::UNKNOWN_ERROR);
diff --git a/services/sensorservice/hidl/include/sensorservicehidl/SensorManager.h b/services/sensorservice/hidl/include/sensorservicehidl/SensorManager.h
index a2372df3d1..e66c8e5d22 100644
--- a/services/sensorservice/hidl/include/sensorservicehidl/SensorManager.h
+++ b/services/sensorservice/hidl/include/sensorservicehidl/SensorManager.h
@@ -17,6 +17,8 @@
 #ifndef ANDROID_FRAMEWORKS_SENSORSERVICE_V1_0_SENSORMANAGER_H
 #define ANDROID_FRAMEWORKS_SENSORSERVICE_V1_0_SENSORMANAGER_H
 
+#include <jni.h>
+
 #include <mutex>
 
 #include <android/frameworks/sensorservice/1.0/ISensorManager.h>
@@ -39,7 +41,7 @@ using ::android::hardware::Return;
 
 struct SensorManager final : public ISensorManager {
 
-    SensorManager();
+    SensorManager(JavaVM* vm);
     ~SensorManager();
 
     // Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow.
@@ -50,11 +52,17 @@ struct SensorManager final : public ISensorManager {
     Return<void> createEventQueue(const sp<IEventQueueCallback> &callback, createEventQueue_cb _hidl_cb);
 
 private:
+    // Block until ::android::SensorManager is initialized.
+    ::android::SensorManager& getInternalManager();
     sp<::android::Looper> getLooper();
 
-    ::android::SensorManager& mInternalManager;
+    std::mutex mInternalManagerMutex;
+    ::android::SensorManager* mInternalManager = nullptr; // does not own
+
     std::mutex mLooperMutex;
     sp<::android::Looper> mLooper;
+
+    JavaVM* mJavaVm;
 };
 
 }  // namespace implementation