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

Commit a8480cd2 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge "Fix flaky VibratorCallbackSchedulerTest" into main

parents 9cdb51f3 1fc5c892
Loading
Loading
Loading
Loading
+4 −4
Original line number Diff line number Diff line
@@ -87,13 +87,13 @@ void CallbackScheduler::loop() {
            lock.lock();
        }
        if (mQueue.empty()) {
            // Wait until a new callback is scheduled.
            mCondition.wait(mMutex);
            // Wait until a new callback is scheduled or destructor was called.
            mCondition.wait(lock, [this] { return mFinished || !mQueue.empty(); });
        } else {
            // Wait until next callback expires, or a new one is scheduled.
            // Wait until next callback expires or a new one is scheduled.
            // Use the monotonic steady clock to wait for the measured delay interval via wait_for
            // instead of using a wall clock via wait_until.
            mCondition.wait_for(mMutex, mQueue.top().getWaitForExpirationDuration());
            mCondition.wait_for(lock, mQueue.top().getWaitForExpirationDuration());
        }
    }
}
+22 −47
Original line number Diff line number Diff line
@@ -14,20 +14,13 @@
 * limitations under the License.
 */

#define LOG_TAG "VibratorHalWrapperAidlTest"

#include <android-base/thread_annotations.h>
#include <android/hardware/vibrator/IVibrator.h>
#include <condition_variable>

#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include <utils/Log.h>
#include <thread>

#include <vibratorservice/VibratorCallbackScheduler.h>

#include "test_utils.h"

using std::chrono::milliseconds;
using std::chrono::steady_clock;
using std::chrono::time_point;
@@ -39,29 +32,25 @@ using namespace testing;
// -------------------------------------------------------------------------------------------------

// Delay allowed for the scheduler to process callbacks during this test.
static const auto TEST_TIMEOUT = 50ms;
static const auto TEST_TIMEOUT = 100ms;

class VibratorCallbackSchedulerTest : public Test {
public:
    void SetUp() override {
        mScheduler = std::make_unique<vibrator::CallbackScheduler>();
        std::lock_guard<std::mutex> lock(mMutex);
        mExpiredCallbacks.clear();
    }
    void SetUp() override { mScheduler = std::make_unique<vibrator::CallbackScheduler>(); }

protected:
    std::mutex mMutex;
    std::condition_variable_any mCondition;
    std::unique_ptr<vibrator::CallbackScheduler> mScheduler = nullptr;
    vibrator::TestCounter mCallbackCounter;
    std::vector<int32_t> mExpiredCallbacks GUARDED_BY(mMutex);

    std::function<void()> createCallback(int32_t id) {
        return [=]() {
        return [this, id]() {
            {
                std::lock_guard<std::mutex> lock(mMutex);
                mExpiredCallbacks.push_back(id);
            }
            mCondition.notify_all();
            mCallbackCounter.increment();
        };
    }

@@ -71,56 +60,42 @@ protected:
    }

    int32_t waitForCallbacks(int32_t callbackCount, milliseconds timeout) {
        time_point<steady_clock> expirationTime = steady_clock::now() + timeout + TEST_TIMEOUT;
        int32_t expiredCallbackCount = 0;
        while (steady_clock::now() < expirationTime) {
            std::lock_guard<std::mutex> lock(mMutex);
            expiredCallbackCount = mExpiredCallbacks.size();
            if (callbackCount <= expiredCallbackCount) {
                return expiredCallbackCount;
            }
            auto currentTimeout = std::chrono::duration_cast<std::chrono::milliseconds>(
                    expirationTime - steady_clock::now());
            if (currentTimeout > currentTimeout.zero()) {
                // Use the monotonic steady clock to wait for the requested timeout via wait_for
                // instead of using a wall clock via wait_until.
                mCondition.wait_for(mMutex, currentTimeout);
            }
        }
        return expiredCallbackCount;
        mCallbackCounter.tryWaitUntilCountIsAtLeast(callbackCount, timeout);
        return mCallbackCounter.get();
    }
};

// -------------------------------------------------------------------------------------------------

TEST_F(VibratorCallbackSchedulerTest, TestScheduleRunsOnlyAfterDelay) {
    auto callbackDuration = 50ms;
    time_point<steady_clock> startTime = steady_clock::now();
    mScheduler->schedule(createCallback(1), 50ms);
    mScheduler->schedule(createCallback(1), callbackDuration);

    ASSERT_EQ(1, waitForCallbacks(1, 50ms));
    ASSERT_THAT(waitForCallbacks(1, callbackDuration + TEST_TIMEOUT), Eq(1));
    time_point<steady_clock> callbackTime = steady_clock::now();

    // Callback happened at least 50ms after the beginning of the test.
    ASSERT_TRUE(startTime + 50ms <= callbackTime);
    ASSERT_THAT(getExpiredCallbacks(), ElementsAre(1));
    // Callback took at least the required duration to trigger.
    ASSERT_THAT(callbackTime, Ge(startTime + callbackDuration));
}

TEST_F(VibratorCallbackSchedulerTest, TestScheduleMultipleCallbacksRunsInDelayOrder) {
    // Schedule first callbacks long enough that all 3 will be scheduled together and run in order.
    mScheduler->schedule(createCallback(1), 50ms);
    mScheduler->schedule(createCallback(2), 40ms);
    mScheduler->schedule(createCallback(3), 10ms);
    mScheduler->schedule(createCallback(1), 50ms + 2 * TEST_TIMEOUT);
    mScheduler->schedule(createCallback(2), 50ms + TEST_TIMEOUT);
    mScheduler->schedule(createCallback(3), 50ms);

    ASSERT_EQ(3, waitForCallbacks(3, 50ms));
    // Callbacks triggered in the expected order based on the requested durations.
    ASSERT_THAT(waitForCallbacks(3, 50ms + 3 * TEST_TIMEOUT), Eq(3));
    ASSERT_THAT(getExpiredCallbacks(), ElementsAre(3, 2, 1));
}

TEST_F(VibratorCallbackSchedulerTest, TestDestructorDropsPendingCallbacksAndKillsThread) {
    // Schedule callback long enough that scheduler will be destroyed while it's still scheduled.
    mScheduler->schedule(createCallback(1), 50ms);
    mScheduler->schedule(createCallback(1), 100ms);
    mScheduler.reset(nullptr);

    // Should timeout waiting for callback to run.
    ASSERT_EQ(0, waitForCallbacks(1, 50ms));
    ASSERT_TRUE(getExpiredCallbacks().empty());
    ASSERT_THAT(waitForCallbacks(1, 100ms + TEST_TIMEOUT), Eq(0));
    ASSERT_THAT(getExpiredCallbacks(), IsEmpty());
}
+28 −0
Original line number Diff line number Diff line
@@ -85,6 +85,34 @@ private:
    ~TestFactory() = delete;
};

class TestCounter {
public:
    TestCounter(int32_t init = 0) : mMutex(), mCondVar(), mCount(init) {}

    int32_t get() {
        std::lock_guard<std::mutex> lock(mMutex);
        return mCount;
    }

    void increment() {
        {
            std::lock_guard<std::mutex> lock(mMutex);
            mCount += 1;
        }
        mCondVar.notify_all();
    }

    void tryWaitUntilCountIsAtLeast(int32_t count, std::chrono::milliseconds timeout) {
        std::unique_lock<std::mutex> lock(mMutex);
        mCondVar.wait_for(lock, timeout, [&] { return mCount >= count; });
    }

private:
    std::mutex mMutex;
    std::condition_variable mCondVar;
    int32_t mCount GUARDED_BY(mMutex);
};

// -------------------------------------------------------------------------------------------------

} // namespace vibrator