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

Commit 63e810a8 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

Change-Id: Icb7477e9d75338b9742e144fc2687d5ced91e89e
parent 534288a2

services/sensorservice/hidl/SensorManager.cpp

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




#include <sched.h>

#include <sched.h>





#include <thread>





#include "EventQueue.h"

#include "EventQueue.h"

#include "DirectReportChannel.h"

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

using ::android::hardware::sensors::V1_0::SensorsEventFormatOffset;

using ::android::hardware::hidl_vec;

using ::android::hardware::hidl_vec;

using ::android::hardware::Void;

using ::android::hardware::Void;

using ::android::sp;





static const char* POLL_THREAD_NAME = "hidl_ssvc_poll";

static const char* POLL_THREAD_NAME = "hidl_ssvc_poll";





SensorManager::SensorManager(JavaVM* vm)

SensorManager::SensorManager(JavaVM* vm)

        : mJavaVm(vm) {

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

}

}





SensorManager::~SensorManager() {

SensorManager::~SensorManager() {

    // Stops pollAll inside the thread.

    // Stops pollAll inside the thread.

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

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



    mStopThread = true;

    if (mLooper != nullptr) {

    if (mLooper != nullptr) {

        mLooper->wake();

        mLooper->wake();

    }

    }

    if (mPollThread.joinable()) {

        mPollThread.join();

    }

}

}





// Methods from ::android::frameworks::sensorservice::V1_0::ISensorManager follow.

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

}





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

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

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

sp<Looper> SensorManager::getLooper() {

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

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

    if (mLooper == nullptr) {

        std::condition_variable looperSet;





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

            struct sched_param p = {0};

            p.sched_priority = 10;

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

                        << strerror(errno);

            }

            }





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

            // set looper

            if (looper != nullptr) {

            Looper::setForThread(looper);

                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

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

            // is from Java.

            // eventually calls into Java.

            JavaVMAttachArgs args{

            JavaVMAttachArgs args{

                .version = JNI_VERSION_1_2,

                .version = JNI_VERSION_1_2,

                .name = POLL_THREAD_NAME,

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

                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 */);

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

            if (pollResult != ALOOPER_POLL_WAKE) {

                if (pollResult == Looper::POLL_WAKE) {

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

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

            if (javaVm->DetachCurrentThread() != JNI_OK) {

                LOG(ERROR) << "Cannot detach SensorManager looper thread from Java VM.";

                LOG(ERROR) << "Cannot detach SensorManager looper thread from Java VM.";

            }

            }





            LOG(INFO) << "Looper thread is terminated.";

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

        }}.detach();

        }};

        looperSet.wait(lock, [this]{ return this->mLooper != nullptr; });

        mPollThread = std::move(pollThread);

    }

    }

    return mLooper;

    return mLooper;

}

}
@@ -199,6 +209,12 @@ Return<void> SensorManager::createEventQueue( 
    }

    }





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

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

    if (looper == nullptr) {

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

        _hidl_cb(nullptr, Result::UNKNOWN_ERROR);

        return Void();

    }



    String8 package(String8::format("hidl_client_pid_%d",

    String8 package(String8::format("hidl_client_pid_%d",

                                    android::hardware::IPCThreadState::self()->getCallingPid()));

                                    android::hardware::IPCThreadState::self()->getCallingPid()));

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

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

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

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

#include <jni.h>





#include <mutex>

#include <mutex>

#include <thread>





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

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

#include <android/frameworks/sensorservice/1.0/types.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_handle;

using ::android::hardware::hidl_memory;

using ::android::hardware::hidl_memory;

using ::android::hardware::Return;

using ::android::hardware::Return;

using ::android::sp;





struct SensorManager final : public ISensorManager {

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

private:

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

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

    ::android::SensorManager& getInternalManager();

    ::android::SensorManager& getInternalManager();

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

    sp<Looper> getLooper();





    std::mutex mInternalManagerMutex;

    std::mutex mInternalManagerMutex;

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

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

    sp<Looper> mLooper;





    std::mutex mLooperMutex;

    volatile bool mStopThread;

    sp<::android::Looper> mLooper;

    std::mutex mThreadMutex; //protects mPollThread

    std::thread mPollThread;





    JavaVM* mJavaVm;

    JavaVM* mJavaVm;

};

};