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
diff --git a/vibrator/bench/benchmark.cpp b/vibrator/bench/benchmark.cpp
index deaa6f2..8e8d78f 100644
--- a/vibrator/bench/benchmark.cpp
+++ b/vibrator/bench/benchmark.cpp
@@ -381,6 +381,16 @@
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 @@
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 @@
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 @@
if (shouldSkipWithError(state, mVibrator->off())) {
return;
}
- if (cb) {
- cb->waitForComplete();
- }
+ waitForComplete(cbFuture);
state.ResumeTiming();
}
});
@@ -442,6 +446,8 @@
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 @@
// Cleanup
state.PauseTiming();
- if (cb) {
- cb->waitForComplete();
- }
+ waitForComplete(cbFuture);
state.ResumeTiming();
}
});
@@ -687,6 +691,8 @@
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 @@
if (shouldSkipWithError(state, mVibrator->off())) {
return;
}
- if (cb) {
- cb->waitForComplete();
- }
+ waitForComplete(cbFuture);
state.ResumeTiming();
}
});
@@ -800,6 +804,8 @@
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 @@
if (shouldSkipWithError(state, mVibrator->off())) {
return;
}
- cb->waitForComplete();
+ waitForComplete(cbFuture);
state.ResumeTiming();
}
});