Codec2Client: Retry creation on failed transaction
This CL moves the retrying logic into ForAllStores(). Before the CL,
CreateInterfaceByName() will fail to detect C2_TRANSACTION_FAILED if the
last service that tries to create the component in ForAllStores() does
not have the requested component.
Test: atest CtsMediaTestCases -- \
--module-arg CtsMediaTestCases:size:small
Bug: 141907195
Change-Id: I98ee084ea14204bba15746e89514636254013aa5
(cherry picked from commit 1e7015ad0f62ca97c3d6f593d33201cf82da1cae)
diff --git a/media/codec2/hidl/client/client.cpp b/media/codec2/hidl/client/client.cpp
index c620bad..c747190 100644
--- a/media/codec2/hidl/client/client.cpp
+++ b/media/codec2/hidl/client/client.cpp
@@ -125,6 +125,9 @@
if (!mClient) {
mClient = Codec2Client::_CreateFromIndex(mIndex);
}
+ CHECK(mClient) << "Failed to create Codec2Client to service \""
+ << GetServiceNames()[mIndex] << "\". (Index = "
+ << mIndex << ").";
return mClient;
}
@@ -832,6 +835,7 @@
c2_status_t Codec2Client::ForAllServices(
const std::string &key,
+ size_t numberOfAttempts,
std::function<c2_status_t(const std::shared_ptr<Codec2Client>&)>
predicate) {
c2_status_t status = C2_NO_INIT; // no IComponentStores present
@@ -860,23 +864,31 @@
for (size_t index : indices) {
Cache& cache = Cache::List()[index];
- std::shared_ptr<Codec2Client> client{cache.getClient()};
- if (client) {
+ for (size_t tries = numberOfAttempts; tries > 0; --tries) {
+ std::shared_ptr<Codec2Client> client{cache.getClient()};
status = predicate(client);
if (status == C2_OK) {
std::scoped_lock lock{key2IndexMutex};
key2Index[key] = index; // update last known client index
return C2_OK;
+ } else if (status == C2_TRANSACTION_FAILED) {
+ LOG(WARNING) << "\"" << key << "\" failed for service \""
+ << client->getName()
+ << "\" due to transaction failure. "
+ << "(Service may have crashed.)"
+ << (tries > 1 ? " Retrying..." : "");
+ cache.invalidate();
+ continue;
}
- }
- if (wasMapped) {
- LOG(INFO) << "Could not find \"" << key << "\""
- " in the last instance. Retrying...";
- wasMapped = false;
- cache.invalidate();
+ if (wasMapped) {
+ LOG(INFO) << "\"" << key << "\" became invalid in service \""
+ << client->getName() << "\". Retrying...";
+ wasMapped = false;
+ }
+ break;
}
}
- return status; // return the last status from a valid client
+ return status; // return the last status from a valid client
}
std::shared_ptr<Codec2Client::Component>
@@ -885,35 +897,37 @@
const std::shared_ptr<Listener>& listener,
std::shared_ptr<Codec2Client>* owner,
size_t numberOfAttempts) {
- while (true) {
- std::shared_ptr<Component> component;
- c2_status_t status = ForAllServices(
- componentName,
- [owner, &component, componentName, &listener](
- const std::shared_ptr<Codec2Client> &client)
- -> c2_status_t {
- c2_status_t status = client->createComponent(componentName,
- listener,
- &component);
- if (status == C2_OK) {
- if (owner) {
- *owner = client;
- }
- } else if (status != C2_NOT_FOUND) {
- LOG(DEBUG) << "IComponentStore("
- << client->getServiceName()
- << ")::createComponent(\"" << componentName
- << "\") returned status = "
- << status << ".";
+ std::string key{"create:"};
+ key.append(componentName);
+ std::shared_ptr<Component> component;
+ c2_status_t status = ForAllServices(
+ key,
+ numberOfAttempts,
+ [owner, &component, componentName, &listener](
+ const std::shared_ptr<Codec2Client> &client)
+ -> c2_status_t {
+ c2_status_t status = client->createComponent(componentName,
+ listener,
+ &component);
+ if (status == C2_OK) {
+ if (owner) {
+ *owner = client;
}
- return status;
- });
- if (numberOfAttempts > 0 && status == C2_TRANSACTION_FAILED) {
- --numberOfAttempts;
- continue;
- }
- return component;
+ } else if (status != C2_NOT_FOUND) {
+ LOG(DEBUG) << "IComponentStore("
+ << client->getServiceName()
+ << ")::createComponent(\"" << componentName
+ << "\") returned status = "
+ << status << ".";
+ }
+ return status;
+ });
+ if (status != C2_OK) {
+ LOG(DEBUG) << "Failed to create component \"" << componentName
+ << "\" from all known services. "
+ "Last returned status = " << status << ".";
}
+ return component;
}
std::shared_ptr<Codec2Client::Interface>
@@ -921,34 +935,36 @@
const char* interfaceName,
std::shared_ptr<Codec2Client>* owner,
size_t numberOfAttempts) {
- while (true) {
- std::shared_ptr<Interface> interface;
- c2_status_t status = ForAllServices(
- interfaceName,
- [owner, &interface, interfaceName](
- const std::shared_ptr<Codec2Client> &client)
- -> c2_status_t {
- c2_status_t status = client->createInterface(interfaceName,
- &interface);
- if (status == C2_OK) {
- if (owner) {
- *owner = client;
- }
- } else if (status != C2_NOT_FOUND) {
- LOG(DEBUG) << "IComponentStore("
- << client->getServiceName()
- << ")::createInterface(\"" << interfaceName
- << "\") returned status = "
- << status << ".";
+ std::string key{"create:"};
+ key.append(interfaceName);
+ std::shared_ptr<Interface> interface;
+ c2_status_t status = ForAllServices(
+ key,
+ numberOfAttempts,
+ [owner, &interface, interfaceName](
+ const std::shared_ptr<Codec2Client> &client)
+ -> c2_status_t {
+ c2_status_t status = client->createInterface(interfaceName,
+ &interface);
+ if (status == C2_OK) {
+ if (owner) {
+ *owner = client;
}
- return status;
- });
- if (numberOfAttempts > 0 && status == C2_TRANSACTION_FAILED) {
- --numberOfAttempts;
- continue;
- }
- return interface;
+ } else if (status != C2_NOT_FOUND) {
+ LOG(DEBUG) << "IComponentStore("
+ << client->getServiceName()
+ << ")::createInterface(\"" << interfaceName
+ << "\") returned status = "
+ << status << ".";
+ }
+ return status;
+ });
+ if (status != C2_OK) {
+ LOG(DEBUG) << "Failed to create interface \"" << interfaceName
+ << "\" from all known services. "
+ "Last returned status = " << status << ".";
}
+ return interface;
}
std::vector<C2Component::Traits> const& Codec2Client::ListComponents() {
diff --git a/media/codec2/hidl/client/include/codec2/hidl/client.h b/media/codec2/hidl/client/include/codec2/hidl/client.h
index 848901d..c37407f 100644
--- a/media/codec2/hidl/client/include/codec2/hidl/client.h
+++ b/media/codec2/hidl/client/include/codec2/hidl/client.h
@@ -208,11 +208,25 @@
protected:
sp<Base> mBase;
- // Finds the first store where the predicate returns OK, and returns the last
- // predicate result. Uses key to remember the last store found, and if cached,
- // it tries that store before trying all stores (one retry).
+ // Finds the first store where the predicate returns C2_OK and returns the
+ // last predicate result. The predicate will be tried on all stores. The
+ // function will return C2_OK the first time the predicate returns C2_OK,
+ // or it will return the value from the last time that predicate is tried.
+ // (The latter case corresponds to a failure on every store.) The order of
+ // the stores to try is the same as the return value of GetServiceNames().
+ //
+ // key is used to remember the last store with which the predicate last
+ // succeeded. If the last successful store is cached, it will be tried
+ // first before all the stores are tried. Note that the last successful
+ // store will be tried twice---first before all the stores, and another time
+ // with all the stores.
+ //
+ // If an attempt to evaluate the predicate results in a transaction failure,
+ // repeated attempts will be made until the predicate returns without a
+ // transaction failure or numberOfAttempts attempts have been made.
static c2_status_t ForAllServices(
const std::string& key,
+ size_t numberOfAttempts,
std::function<c2_status_t(std::shared_ptr<Codec2Client> const&)>
predicate);
