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

Commit c24e2085 authored by Peng Xu's avatar Peng Xu
Browse files

Fix hidl_ssvc_poll thread issues 

1) Patch poll wake up handling logic so that thread no longer quits
   on spurious wake up.
2) Patch(simplify) thread initialization, so that it will not hang
   on waiting for conditional variable.

Test: 1) send signal 3 to system_server, does not observe thread quit.
Test: 2) adb shell "stop;start" many times, every time thread starts.

Bug: 62933449
Bug: 62933799

Merged-In: Icb7477e9d75338b9742e144fc2687d5ced91e89e
Change-Id: Icb7477e9d75338b9742e144fc2687d5ced91e89e
parent 96b54e60

services/sensorservice/hidl/SensorManager.cpp

+41 −25
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ 


#include <sched.h>



#include <thread>



#include "EventQueue.h"

#include "DirectReportChannel.h"
@@ -40,20 +39,24 @@ using ::android::hardware::sensors::V1_0::SensorInfo; 
using ::android::hardware::sensors::V1_0::SensorsEventFormatOffset;

using ::android::hardware::hidl_vec;

using ::android::hardware::Void;

using ::android::sp;



static const char* POLL_THREAD_NAME = "hidl_ssvc_poll";



SensorManager::SensorManager(JavaVM* vm)

        : mJavaVm(vm) {

        : mLooper(new Looper(false /*allowNonCallbacks*/)), mStopThread(true), mJavaVm(vm) {

}



SensorManager::~SensorManager() {

    // Stops pollAll inside the thread.

    std::unique_lock<std::mutex> lock(mLooperMutex);

    std::lock_guard<std::mutex> lock(mThreadMutex);



    mStopThread = true;

    if (mLooper != nullptr) {

        mLooper->wake();

    }

    if (mPollThread.joinable()) {

        mPollThread.join();

    }

}



// Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow.
@@ -128,12 +131,13 @@ Return<void> SensorManager::createGrallocDirectChannel( 
}



/* One global looper for all event queues created from this SensorManager. */

sp<::android::Looper> SensorManager::getLooper() {

    std::unique_lock<std::mutex> lock(mLooperMutex);

    if (mLooper == nullptr) {

        std::condition_variable looperSet;

sp<Looper> SensorManager::getLooper() {

    std::lock_guard<std::mutex> lock(mThreadMutex);



        std::thread{[&mutex = mLooperMutex, &looper = mLooper, &looperSet, javaVm = mJavaVm] {

    if (!mPollThread.joinable()) {

        // if thread not initialized, start thread

        mStopThread = false;

        std::thread pollThread{[&stopThread = mStopThread, looper = mLooper, javaVm = mJavaVm] {



            struct sched_param p = {0};

            p.sched_priority = 10;
@@ -142,16 +146,11 @@ sp<::android::Looper> SensorManager::getLooper() { 
                        << strerror(errno);

            }



            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();

            // set looper

            Looper::setForThread(looper);



            // Attach the thread to JavaVM so that pollAll do not crash if the event

            // is from Java.

            // Attach the thread to JavaVM so that pollAll do not crash if the thread

            // eventually calls into Java.

            JavaVMAttachArgs args{

                .version = JNI_VERSION_1_2,

                .name = POLL_THREAD_NAME,
@@ -162,19 +161,30 @@ sp<::android::Looper> SensorManager::getLooper() { 
                LOG(FATAL) << "Cannot attach SensorManager looper thread to Java VM.";

            }



            looperSet.notify_one();

            LOG(INFO) << POLL_THREAD_NAME << " started.";

            for (;;) {

                int pollResult = looper->pollAll(-1 /* timeout */);

            if (pollResult != ALOOPER_POLL_WAKE) {

                LOG(ERROR) << "Looper::pollAll returns unexpected " << pollResult;

                if (pollResult == Looper::POLL_WAKE) {

                    if (stopThread == true) {

                        LOG(INFO) << POLL_THREAD_NAME << ": requested to stop";

                        break;

                    } else {

                        LOG(INFO) << POLL_THREAD_NAME << ": spurious wake up, back to work";

                    }

                } else {

                    LOG(ERROR) << POLL_THREAD_NAME << ": Looper::pollAll returns unexpected "

                               << pollResult;

                    break;

                }

            }



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

            LOG(INFO) << POLL_THREAD_NAME << " is terminated.";

        }};

        mPollThread = std::move(pollThread);

    }

    return mLooper;

}
@@ -196,6 +206,12 @@ Return<void> SensorManager::createEventQueue( 
    }



    sp<::android::Looper> looper = getLooper();

    if (looper == nullptr) {

        LOG(ERROR) << "::android::SensorManager::createEventQueue cannot initialize looper";

        _hidl_cb(nullptr, Result::UNKNOWN_ERROR);

        return Void();

    }



    sp<::android::SensorEventQueue> internalQueue = getInternalManager().createEventQueue();

    if (internalQueue == nullptr) {

        LOG(WARNING) << "::android::SensorManager::createEventQueue returns nullptr.";

services/sensorservice/hidl/include/sensorservicehidl/SensorManager.h

+7 −3
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <jni.h>



#include <mutex>

#include <thread>



#include <android/frameworks/sensorservice/1.0/ISensorManager.h>

#include <android/frameworks/sensorservice/1.0/types.h>
@@ -38,6 +39,7 @@ using ::android::hardware::sensors::V1_0::SensorType; 
using ::android::hardware::hidl_handle;

using ::android::hardware::hidl_memory;

using ::android::hardware::Return;

using ::android::sp;



struct SensorManager final : public ISensorManager {
@@ -54,13 +56,15 @@ struct SensorManager final : public ISensorManager { 
private:

    // Block until ::android::SensorManager is initialized.

    ::android::SensorManager& getInternalManager();

    sp<::android::Looper> getLooper();

    sp<Looper> getLooper();



    std::mutex mInternalManagerMutex;

    ::android::SensorManager* mInternalManager = nullptr; // does not own

    sp<Looper> mLooper;



    std::mutex mLooperMutex;

    sp<::android::Looper> mLooper;

    volatile bool mStopThread;

    std::mutex mThreadMutex; //protects mPollThread

    std::thread mPollThread;



    JavaVM* mJavaVm;

};