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

Commit e467e013 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

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
parent 549a822b

audio/aidl/common/StreamWorker.cpp

+14 −6
Original line number Diff line number Diff line
@@ -25,15 +25,20 @@ namespace android::hardware::audio::common::internal { 
bool ThreadController::start(const std::string& name, int priority) {

    mThreadName = name;

    mThreadPriority = priority;

    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::switchWorkerStateSync(WorkerState oldState, WorkerState n 
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 @@ void ThreadController::workerThread() { 
            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;

audio/aidl/common/include/StreamWorker.h

+5 −2
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ class StreamLogic; 
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 @@ class ThreadController { 
    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 @@ class ThreadController { 
    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 {

audio/aidl/common/tests/streamworker_tests.cpp

+12 −0
Original line number Diff line number Diff line
@@ -283,4 +283,16 @@ TEST_P(StreamWorkerTest, ThreadPriority) { 
    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());