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

Commit 674f7356 authored by Keith Mok's avatar Keith Mok Committed by Shunkai Yao
Browse files

Fix race condition in thread creation

There is a race condition in thread creation in initializer list of a
class. If the thread is created in initializer list of a class, it must
be the last member variable of the class, as the thread is trying to
access some of the member variables in the thread which might not be
initialized yet.

There are two ways to fix this. One is to move the thread creation to
the constructor which all member variables of the class will be
initialized by that time or need to ensure the mThread variable is the
last variable of the class and the last one in the initializer list.

Bug: 235136279
Test: build
Change-Id: Iafb48377e29580cdfb30bedbd9e49c4b1ff6a148
Merged-In: Iafb48377e29580cdfb30bedbd9e49c4b1ff6a148
parent a05abbf5
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -174,7 +174,6 @@ class SensorPoseProviderImpl : public SensorPoseProvider {
    sp<Looper> mLooper;
    Listener* const mListener;
    SensorManager* const mSensorManager;
    std::thread mThread;
    std::mutex mMutex;
    std::map<int32_t, SensorEnableGuard> mEnabledSensors;
    std::map<int32_t, SensorExtra> mEnabledSensorsExtra GUARDED_BY(mMutex);
@@ -187,11 +186,13 @@ class SensorPoseProviderImpl : public SensorPoseProvider {
    // the worker thread and that thread would notify, via the promise below whenever initialization
    // is finished, and whether it was successful.
    std::promise<bool> mInitPromise;
    std::thread mThread;

    SensorPoseProviderImpl(const char* packageName, Listener* listener)
        : mListener(listener),
          mSensorManager(&SensorManager::getInstanceForPackage(String16(packageName))),
          mThread([this] { threadFunc(); }) {}
          mSensorManager(&SensorManager::getInstanceForPackage(String16(packageName))) {
        mThread = std::thread([this] { threadFunc(); });
    }

    void initFinished(bool success) { mInitPromise.set_value(success); }

+4 −1
Original line number Diff line number Diff line
@@ -100,7 +100,10 @@ SpatializerPoseController::SpatializerPoseController(Listener* listener,
              .screenStillnessRotationalThreshold = kScreenStillnessRotationThreshold,
      })),
      mPoseProvider(SensorPoseProvider::create("headtracker", this)),
      mThread([this, maxUpdatePeriod] {
      mThread([this, maxUpdatePeriod] { // It's important that mThread is initialized after
                                        // everything else because it runs a member
                                        // function that may use any member
                                        // of this class.
          while (true) {
              Pose3f headToStage;
              std::optional<HeadTrackingMode> modeIfChanged;
+4 −1
Original line number Diff line number Diff line
@@ -123,12 +123,15 @@ class SpatializerPoseController : private media::SensorPoseProvider::Listener {
    int32_t mHeadSensor = media::SensorPoseProvider::INVALID_HANDLE;
    int32_t mScreenSensor = media::SensorPoseProvider::INVALID_HANDLE;
    std::optional<media::HeadTrackingMode> mActualMode;
    std::thread mThread;
    std::condition_variable mCondVar;
    bool mShouldCalculate = true;
    bool mShouldExit = false;
    bool mCalculated = false;

    // It's important that mThread is the last variable in this class
    // since we starts mThread in initializer list
    std::thread mThread;

    void onPose(int64_t timestamp, int32_t sensor, const media::Pose3f& pose,
                const std::optional<media::Twist3f>& twist, bool isNewReference) override;