nn: fix race condition in execution tests
The setup of the execution tests is designed to synchronize a call to
getOutputOperandDimensions whilst an execution is running. The atomics
that were controlling this were letting that call happen before the
execution had started, and in rare (high CPU load) occasions this
would deadlock.
This CL fixes the checks on those atomics to ensure the synchronization
happens as expected. A change to configureExecutionBurst was also
required in order to get the SampleDriver to call the wrapped model
(and hence the dummyExecution), otherwise the locking semaphore wouldn't
clear and the test would freeze.
BUG=b:163081732
TEST=FEATURES=test emerge-octopus aosp-frameworks-ml-nn
Change-Id: Id78d2b25f84607f9e5fe8eb50efe11e2f94edb5f
(cherry picked from commit 8230780e6e5337692d3d5447a6f594b4908b8e03)
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/frameworks/ml/+/2690807
Reviewed-by: Michael Pishchagin <mblsha@google.com>
Tested-by: Jim Pollock <jmpollock@chromium.org>
Auto-Submit: Jim Pollock <jmpollock@chromium.org>
Commit-Queue: Michael Pishchagin <mblsha@google.com>
diff --git a/nn/runtime/test/TestExecution.cpp b/nn/runtime/test/TestExecution.cpp
index 66bef6b..2199671 100644
--- a/nn/runtime/test/TestExecution.cpp
+++ b/nn/runtime/test/TestExecution.cpp
@@ -29,6 +29,7 @@
#include "Callbacks.h"
#include "CompilationBuilder.h"
+#include "ExecutionBurstServer.h"
#include "HalInterfaces.h"
#include "Manager.h"
#include "ModelBuilder.h"
@@ -172,9 +173,13 @@
const MQDescriptorSync<V1_2::FmqResultDatum>& resultChannel,
configureExecutionBurst_cb cb) override {
CHECK(mPreparedModelV1_2 != nullptr) << "V1_2 prepared model is nullptr.";
- if (mErrorStatus == ErrorStatus::NONE) {
- return mPreparedModelV1_2->configureExecutionBurst(callback, requestChannel,
- resultChannel, cb);
+ if (mErrorStatus == V1_3::ErrorStatus::NONE) {
+ const sp<V1_2::IBurstContext> burst = nn::ExecutionBurstServer::create(
+ callback, requestChannel, resultChannel, this);
+
+ cb(burst == nullptr ? V1_0::ErrorStatus::GENERAL_FAILURE : V1_0::ErrorStatus::NONE,
+ burst);
+ return Void();
} else {
cb(convertToV1_0(mErrorStatus), nullptr);
return Void();
@@ -213,13 +218,20 @@
static void pauseExecutions(bool v) { mPauseExecutions.store(v); }
// This function is only guaranteed to work in the following pattern:
- // - pauseExecutions(true);
- // - // launch execution
- // - // thread A: waitForExecutionToBegin()
- // - // thread B: pauseExecutions(false);
+ // Consider thread A as primary thread
+ // - thread A: pauseExecutions(true);
+ // - thread A: launch execution (as thread B)
+ // - thread A: waitForExecutionToBegin(), block until call to dummyExecution by
+ // thread B makes mExecutionsInFlight nonzero
+ // - thread B: dummyExecution(), which makes mExecutionsInFlight nonzero and blocks
+ // until thread A calls pauseExecutions(false)
+ // - thread A: waitForExecutionToBegin() returns
+ // - thread A: pauseExecutions(false), allowing dummyExecution() on thread B to continue
+ // - thread B: dummyExecution() zeroes mExecutionsInFlight and returns
+ // - thread B: thread exits
static void waitForExecutionToBegin() {
CHECK(mPauseExecutions.load());
- while (mExecutionsInFlight.load()) {
+ while (mExecutionsInFlight.load() == 0) {
}
}
diff --git a/nn/runtime/test/TestIntrospectionControl.cpp b/nn/runtime/test/TestIntrospectionControl.cpp
index 972619e..9caa9e2 100644
--- a/nn/runtime/test/TestIntrospectionControl.cpp
+++ b/nn/runtime/test/TestIntrospectionControl.cpp
@@ -573,10 +573,17 @@
static void pauseExecutions(bool v) { mPauseExecutions.store(v); }
// This function is only guaranteed to work in the following pattern:
- // - pauseExecutions(true);
- // - // launch execution
- // - // thread A: waitForExecutionToBegin()
- // - // thread B: pauseExecutions(false);
+ // Consider thread A as primary thread
+ // - thread A: pauseExecutions(true);
+ // - thread A: launch execution (as thread B)
+ // - thread A: waitForExecutionToBegin(), block until call to dummyExecution by
+ // thread B makes mExecutionsInFlight nonzero
+ // - thread B: dummyExecution(), which makes mExecutionsInFlight nonzero and blocks
+ // until thread A calls pauseExecutions(false)
+ // - thread A: waitForExecutionToBegin() returns
+ // - thread A: pauseExecutions(false), allowing dummyExecution() on thread B to continue
+ // - thread B: dummyExecution() zeroes mExecutionsInFlight and returns
+ // - thread B: thread exits
static void waitForExecutionToBegin() {
CHECK(mPauseExecutions.load());
while (mExecutionsInFlight.load() == 0) {