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

Commit a5131c57 authored by Lais Andrade's avatar Lais Andrade
Browse files

Fix benchmark crash on wait for destroyed callback 

Fix benchmark crash error:

`FORTIFY: pthread_mutex_lock called on a destroyed mutex`

Caused by the bench loop trying to wait on the callback promise future
after the callback instance was destroyed.

The fix grabs the promise future before performing the HAL operation, so
it can access the fulfilled value after the promise fulfilled and then
destroyed.

Change-Id: I4504129672a11ad1662ea36c79f522d754535765
Fix: 336977792
Test: atest VibratorHalIntegrationBenchmark
Flag: TEST_ONLY
parent 01868eb8

vibrator/bench/benchmark.cpp

+23 −17
Original line number Diff line number Diff line
@@ -381,6 +381,16 @@ class VibratorBench_Aidl : public BaseBench<Aidl::IVibrator> { 
        return false;

    }



    void waitForComplete(std::future<void>& callbackFuture) {

        // Wait until the HAL has finished processing previous vibration before starting a new one,

        // so the HAL state is consistent on each run and metrics are less noisy. Some of the newest

        // HAL implementations are waiting on previous vibration cleanup and might be significantly

        // slower, so make sure we measure vibrations on a clean slate.

        if (callbackFuture.valid()) {

            callbackFuture.wait_for(VIBRATION_CALLBACK_TIMEOUT);

        }

    }



    static void SlowBenchConfig(Benchmark* b) { b->Iterations(VIBRATION_ITERATIONS); }

};
@@ -402,13 +412,7 @@ class HalCallback : public Aidl::BnVibratorCallback { 
        return android::binder::Status::ok();

    }



    void waitForComplete() {

        // Wait until the HAL has finished processing previous vibration before starting a new one,

        // so the HAL state is consistent on each run and metrics are less noisy. Some of the newest

        // HAL implementations are waiting on previous vibration cleanup and might be significantly

        // slower, so make sure we measure vibrations on a clean slate.

        mPromise.get_future().wait_for(VIBRATION_CALLBACK_TIMEOUT);

    }

    std::future<void> getFuture() { return mPromise.get_future(); }



  private:

    std::promise<void> mPromise;
@@ -419,6 +423,8 @@ BENCHMARK_WRAPPER(SlowVibratorBench_Aidl, on, { 


    for (auto _ : state) {

        auto cb = hasCapabilities(Aidl::IVibrator::CAP_ON_CALLBACK) ? new HalCallback() : nullptr;

        // Grab the future before callback promise is destroyed by the HAL.

        auto cbFuture = cb ? cb->getFuture() : std::future<void>();



        // Test

        if (shouldSkipWithError(state, mVibrator->on(ms, cb))) {
@@ -430,9 +436,7 @@ BENCHMARK_WRAPPER(SlowVibratorBench_Aidl, on, { 
        if (shouldSkipWithError(state, mVibrator->off())) {

            return;

        }

        if (cb) {

            cb->waitForComplete();

        }

        waitForComplete(cbFuture);

        state.ResumeTiming();

    }

});
@@ -442,6 +446,8 @@ BENCHMARK_WRAPPER(SlowVibratorBench_Aidl, off, { 


    for (auto _ : state) {

        auto cb = hasCapabilities(Aidl::IVibrator::CAP_ON_CALLBACK) ? new HalCallback() : nullptr;

        // Grab the future before callback promise is destroyed by the HAL.

        auto cbFuture = cb ? cb->getFuture() : std::future<void>();



        // Setup

        state.PauseTiming();
@@ -457,9 +463,7 @@ BENCHMARK_WRAPPER(SlowVibratorBench_Aidl, off, { 


        // Cleanup

        state.PauseTiming();

        if (cb) {

            cb->waitForComplete();

        }

        waitForComplete(cbFuture);

        state.ResumeTiming();

    }

});
@@ -687,6 +691,8 @@ BENCHMARK_WRAPPER(SlowVibratorEffectsBench_Aidl, perform, { 
    for (auto _ : state) {

        auto cb = hasCapabilities(Aidl::IVibrator::CAP_PERFORM_CALLBACK) ? new HalCallback()

                                                                         : nullptr;

        // Grab the future before callback promise is destroyed by the HAL.

        auto cbFuture = cb ? cb->getFuture() : std::future<void>();



        // Test

        if (shouldSkipWithError(state, mVibrator->perform(effect, strength, cb, &lengthMs))) {
@@ -698,9 +704,7 @@ BENCHMARK_WRAPPER(SlowVibratorEffectsBench_Aidl, perform, { 
        if (shouldSkipWithError(state, mVibrator->off())) {

            return;

        }

        if (cb) {

            cb->waitForComplete();

        }

        waitForComplete(cbFuture);

        state.ResumeTiming();

    }

});
@@ -800,6 +804,8 @@ BENCHMARK_WRAPPER(SlowVibratorPrimitivesBench_Aidl, compose, { 


    for (auto _ : state) {

        auto cb = new HalCallback();

        // Grab the future before callback promise is moved and destroyed by the HAL.

        auto cbFuture = cb->getFuture();



        // Test

        if (shouldSkipWithError(state, mVibrator->compose(effects, cb))) {
@@ -811,7 +817,7 @@ BENCHMARK_WRAPPER(SlowVibratorPrimitivesBench_Aidl, compose, { 
        if (shouldSkipWithError(state, mVibrator->off())) {

            return;

        }

        cb->waitForComplete();

        waitForComplete(cbFuture);

        state.ResumeTiming();

    }

});