From 82f17d9ed4d10106c3aad700815f000806edb5bf Mon Sep 17 00:00:00 2001
From: Weilin Xu <xuweilin@google.com>
Date: Mon, 11 Dec 2023 15:03:07 -0800
Subject: [PATCH] Fix timeout issue for bcradio worker thread

Accessed mIsTerminating inside the same lock as what is used in while
loop in worker thread class of broadcast radio HAL utils lib. This fixed
the race condition that mIsTerminating is accessed as true in threadLoop
while the destructor is setting mIsTerminating as false, which causes the
thread waits forever for lock after lock is released in the desctructor.

Bug: 314100017
Test: atest VtsHalBroadcastradioAidlTargetTest WorkerThreadTest
Change-Id: I885e1487ac39525fc2d1ee2134d24409264ca0fc
---
 broadcastradio/common/tests/Android.bp                    | 3 +++
 broadcastradio/common/utils/WorkerThread.cpp              | 5 ++++-
 .../utils/include/broadcastradio-utils/WorkerThread.h     | 8 +++++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/broadcastradio/common/tests/Android.bp b/broadcastradio/common/tests/Android.bp
index 0a6a3a27d0..1ea4e8584b 100644
--- a/broadcastradio/common/tests/Android.bp
+++ b/broadcastradio/common/tests/Android.bp
@@ -86,4 +86,7 @@ cc_test {
     ],
     static_libs: ["android.hardware.broadcastradio@common-utils-lib"],
     test_suites: ["general-tests"],
+    shared_libs: [
+        "libbase",
+    ],
 }
diff --git a/broadcastradio/common/utils/WorkerThread.cpp b/broadcastradio/common/utils/WorkerThread.cpp
index 43fd04a89a..5c8e59c03f 100644
--- a/broadcastradio/common/utils/WorkerThread.cpp
+++ b/broadcastradio/common/utils/WorkerThread.cpp
@@ -69,8 +69,11 @@ void WorkerThread::cancelAll() {
 }
 
 void WorkerThread::threadLoop() {
-    while (!mIsTerminating) {
+    while (true) {
         unique_lock<mutex> lk(mMut);
+        if (mIsTerminating) {
+            return;
+        }
         if (mTasks.empty()) {
             mCond.wait(lk);
             continue;
diff --git a/broadcastradio/common/utils/include/broadcastradio-utils/WorkerThread.h b/broadcastradio/common/utils/include/broadcastradio-utils/WorkerThread.h
index 457b57e403..e381f5a132 100644
--- a/broadcastradio/common/utils/include/broadcastradio-utils/WorkerThread.h
+++ b/broadcastradio/common/utils/include/broadcastradio-utils/WorkerThread.h
@@ -20,6 +20,8 @@
 #include <queue>
 #include <thread>
 
+#include <android-base/thread_annotations.h>
+
 namespace android {
 
 class WorkerThread {
@@ -40,11 +42,11 @@ class WorkerThread {
     };
     friend bool operator<(const Task& lhs, const Task& rhs);
 
-    std::atomic<bool> mIsTerminating;
     std::mutex mMut;
-    std::condition_variable mCond;
+    bool mIsTerminating GUARDED_BY(mMut);
+    std::condition_variable mCond GUARDED_BY(mMut);
     std::thread mThread;
-    std::priority_queue<Task> mTasks;
+    std::priority_queue<Task> mTasks GUARDED_BY(mMut);
 
     void threadLoop();
 };