audio: Fix handling of quick worker completion in StreamWorker
In tests, it is possible that the worker exits quickly,
prior to the moment when the thread controller begins
waiting for it to start. Improper handling of this case
was causing intermittent lock-ups of VTS tests.
Bug: 205884982
Test: atest libaudioaidlcommon_test
Change-Id: I13a83113b32b859e212f1a517ad61bf5b8d43365
diff --git a/audio/aidl/common/StreamWorker.cpp b/audio/aidl/common/StreamWorker.cpp
index dda0e4a..0d2121c 100644
--- a/audio/aidl/common/StreamWorker.cpp
+++ b/audio/aidl/common/StreamWorker.cpp
@@ -25,15 +25,20 @@
bool ThreadController::start(const std::string& name, int priority) {
mThreadName = name;
mThreadPriority = priority;
- mWorker = std::thread(&ThreadController::workerThread, this);
+ if (kTestSingleThread != name) {
+ mWorker = std::thread(&ThreadController::workerThread, this);
+ } else {
+ // Simulate the case when the workerThread completes prior
+ // to the moment when we being waiting for its start.
+ workerThread();
+ }
std::unique_lock<std::mutex> lock(mWorkerLock);
android::base::ScopedLockAssertion lock_assertion(mWorkerLock);
mWorkerCv.wait(lock, [&]() {
android::base::ScopedLockAssertion lock_assertion(mWorkerLock);
- return mWorkerState == WorkerState::RUNNING || !mError.empty();
+ return mWorkerState != WorkerState::INITIAL || !mError.empty();
});
- mWorkerStateChangeRequest = false;
- return mWorkerState == WorkerState::RUNNING;
+ return mError.empty();
}
void ThreadController::stop() {
@@ -81,8 +86,8 @@
void ThreadController::workerThread() {
using Status = StreamLogic::Status;
- std::string error = mLogic->init();
- if (error.empty() && !mThreadName.empty()) {
+ std::string error;
+ if (!mThreadName.empty()) {
std::string compliantName(mThreadName.substr(0, 15));
if (int errCode = pthread_setname_np(pthread_self(), compliantName.c_str()); errCode != 0) {
error.append("Failed to set thread name: ").append(strerror(errCode));
@@ -94,6 +99,9 @@
error.append("Failed to set thread priority: ").append(strerror(errCode));
}
}
+ if (error.empty()) {
+ error.append(mLogic->init());
+ }
{
std::lock_guard<std::mutex> lock(mWorkerLock);
mWorkerState = error.empty() ? WorkerState::RUNNING : WorkerState::STOPPED;
diff --git a/audio/aidl/common/include/StreamWorker.h b/audio/aidl/common/include/StreamWorker.h
index ab2ec26..e9c1070 100644
--- a/audio/aidl/common/include/StreamWorker.h
+++ b/audio/aidl/common/include/StreamWorker.h
@@ -32,7 +32,7 @@
namespace internal {
class ThreadController {
- enum class WorkerState { STOPPED, RUNNING, PAUSE_REQUESTED, PAUSED, RESUME_REQUESTED };
+ enum class WorkerState { INITIAL, STOPPED, RUNNING, PAUSE_REQUESTED, PAUSED, RESUME_REQUESTED };
public:
explicit ThreadController(StreamLogic* logic) : mLogic(logic) {}
@@ -76,7 +76,7 @@
std::thread mWorker;
std::mutex mWorkerLock;
std::condition_variable mWorkerCv;
- WorkerState mWorkerState GUARDED_BY(mWorkerLock) = WorkerState::STOPPED;
+ WorkerState mWorkerState GUARDED_BY(mWorkerLock) = WorkerState::INITIAL;
std::string mError GUARDED_BY(mWorkerLock);
// The atomic lock-free variable is used to prevent priority inversions
// that can occur when a high priority worker tries to acquire the lock
@@ -90,6 +90,9 @@
std::atomic<bool> mWorkerStateChangeRequest GUARDED_BY(mWorkerLock) = false;
};
+// A special thread name used in tests only.
+static const std::string kTestSingleThread = "__testST__";
+
} // namespace internal
class StreamLogic {
diff --git a/audio/aidl/common/tests/streamworker_tests.cpp b/audio/aidl/common/tests/streamworker_tests.cpp
index 8ea8424..f7a30b9 100644
--- a/audio/aidl/common/tests/streamworker_tests.cpp
+++ b/audio/aidl/common/tests/streamworker_tests.cpp
@@ -283,4 +283,16 @@
EXPECT_EQ(priority, worker.getPriority());
}
+TEST_P(StreamWorkerTest, DeferredStartCheckNoError) {
+ stream.setStopStatus();
+ EXPECT_TRUE(worker.start(android::hardware::audio::common::internal::kTestSingleThread));
+ EXPECT_FALSE(worker.hasError());
+}
+
+TEST_P(StreamWorkerTest, DeferredStartCheckWithError) {
+ stream.setErrorStatus();
+ EXPECT_FALSE(worker.start(android::hardware::audio::common::internal::kTestSingleThread));
+ EXPECT_TRUE(worker.hasError());
+}
+
INSTANTIATE_TEST_SUITE_P(StreamWorker, StreamWorkerTest, testing::Bool());