diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/GeneratorHub.cpp b/automotive/vehicle/2.0/default/impl/vhal_v2_0/GeneratorHub.cpp index 9be9ea7c8e..503afd2eb2 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/GeneratorHub.cpp +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/GeneratorHub.cpp @@ -28,11 +28,18 @@ namespace V2_0 { namespace impl { -GeneratorHub::GeneratorHub(const OnHalEvent& onHalEvent) - : mOnHalEvent(onHalEvent), mThread(&GeneratorHub::run, this) {} +GeneratorHub::GeneratorHub(const OnHalEvent& onHalEvent) : mOnHalEvent(onHalEvent) { + mThread = std::thread(&GeneratorHub::run, this); +} GeneratorHub::~GeneratorHub() { - mShuttingDownFlag.store(true); + { + // Even if the shared variable is atomic, it must be modified under the + // mutex in order to correctly publish the modification to the waiting + // thread. + std::unique_lock g(mLock); + mShuttingDownFlag.store(true); + } mCond.notify_all(); if (mThread.joinable()) { mThread.join(); diff --git a/automotive/vehicle/aidl/impl/fake_impl/GeneratorHub/src/GeneratorHub.cpp b/automotive/vehicle/aidl/impl/fake_impl/GeneratorHub/src/GeneratorHub.cpp index 1690c78ef9..d815456a56 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/GeneratorHub/src/GeneratorHub.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/GeneratorHub/src/GeneratorHub.cpp @@ -29,11 +29,18 @@ namespace fake { using ::android::base::ScopedLockAssertion; -GeneratorHub::GeneratorHub(OnHalEvent&& onHalEvent) - : mOnHalEvent(onHalEvent), mThread(&GeneratorHub::run, this) {} +GeneratorHub::GeneratorHub(OnHalEvent&& onHalEvent) : mOnHalEvent(onHalEvent) { + mThread = std::thread(&GeneratorHub::run, this); +} GeneratorHub::~GeneratorHub() { - mShuttingDownFlag.store(true); + { + // Even if the shared variable is atomic, it must be modified under the + // mutex in order to correctly publish the modification to the waiting + // thread. + std::unique_lock lock(mGeneratorsLock); + mShuttingDownFlag.store(true); + } mCond.notify_all(); if (mThread.joinable()) { mThread.join(); diff --git a/automotive/vehicle/aidl/impl/utils/common/include/PendingRequestPool.h b/automotive/vehicle/aidl/impl/utils/common/include/PendingRequestPool.h index 3f8db93ba3..28cf08e93b 100644 --- a/automotive/vehicle/aidl/impl/utils/common/include/PendingRequestPool.h +++ b/automotive/vehicle/aidl/impl/utils/common/include/PendingRequestPool.h @@ -21,7 +21,6 @@ #include #include -#include #include #include #include @@ -85,7 +84,7 @@ class PendingRequestPool final { std::unordered_map> mPendingRequestsByClient GUARDED_BY(mLock); std::thread mThread; - std::atomic mThreadStop = false; + bool mThreadStop = false; std::condition_variable mCv; std::mutex mCvLock; diff --git a/automotive/vehicle/aidl/impl/utils/common/src/PendingRequestPool.cpp b/automotive/vehicle/aidl/impl/utils/common/src/PendingRequestPool.cpp index 0196edd2ca..ab504992e4 100644 --- a/automotive/vehicle/aidl/impl/utils/common/src/PendingRequestPool.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/src/PendingRequestPool.cpp @@ -39,20 +39,27 @@ constexpr int64_t CHECK_TIME_IN_NANO = 1'000'000'000; } // namespace -PendingRequestPool::PendingRequestPool(int64_t timeoutInNano) - : mTimeoutInNano(timeoutInNano), mThread([this] { - // [this] must be alive within this thread because destructor would wait for this thread - // to exit. - int64_t sleepTime = std::min(mTimeoutInNano, static_cast(CHECK_TIME_IN_NANO)); - std::unique_lock lk(mCvLock); - while (!mCv.wait_for(lk, std::chrono::nanoseconds(sleepTime), - [this] { return mThreadStop.load(); })) { - checkTimeout(); - } - }) {} +PendingRequestPool::PendingRequestPool(int64_t timeoutInNano) : mTimeoutInNano(timeoutInNano) { + mThread = std::thread([this] { + // [this] must be alive within this thread because destructor would wait for this thread + // to exit. + int64_t sleepTime = std::min(mTimeoutInNano, static_cast(CHECK_TIME_IN_NANO)); + std::unique_lock lk(mCvLock); + while (!mCv.wait_for(lk, std::chrono::nanoseconds(sleepTime), + [this] { return mThreadStop; })) { + checkTimeout(); + } + }); +} PendingRequestPool::~PendingRequestPool() { - mThreadStop = true; + { + // Even if the shared variable is atomic, it must be modified under the + // mutex in order to correctly publish the modification to the waiting + // thread. + std::unique_lock lk(mCvLock); + mThreadStop = true; + } mCv.notify_all(); if (mThread.joinable()) { mThread.join();