Revert "AssetManager: Cache a pre-filtered list of configurations"
There is a race due to the modification of shared data structures in the framework ResTable.
See b/27499488
This reverts commit d4b169173ad7805369204277580d3942cb08174a.
Change-Id: I3f400a2ad3b5ffc652b84dd5fe777f7cf34b5548
diff --git a/include/androidfw/ResourceTypes.h b/include/androidfw/ResourceTypes.h
index 16bea79..d8801b8 100644
--- a/include/androidfw/ResourceTypes.h
+++ b/include/androidfw/ResourceTypes.h
@@ -1896,13 +1896,6 @@
mutable Mutex mLock;
- // Mutex that controls access to the list of pre-filtered configurations
- // to check when looking up entries.
- // When iterating over a bag, the mLock mutex is locked. While mLock is locked,
- // we do resource lookups.
- // Mutex is not reentrant, so we must use a different lock than mLock.
- mutable Mutex mFilteredConfigLock;
-
status_t mError;
ResTable_config mParams;
diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index c73bb584..3277c36 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -25,7 +25,6 @@
#include <string.h>
#include <limits>
-#include <memory>
#include <type_traits>
#include <androidfw/ByteBucketArray.h>
@@ -3147,9 +3146,6 @@
StringPoolRef keyStr;
};
-template <typename T>
-using SharedVector = std::shared_ptr<Vector<T>>;
-
struct ResTable::Type
{
Type(const Header* _header, const Package* _package, size_t count)
@@ -3162,10 +3158,6 @@
const uint32_t* typeSpecFlags;
IdmapEntries idmapEntries;
Vector<const ResTable_type*> configs;
-
- // The set of configurations that match the current parameters.
- // This will be swapped with a new set when the parameters change.
- SharedVector<const ResTable_type*> filteredConfigs;
};
struct ResTable::Package
@@ -4438,44 +4430,18 @@
void ResTable::setParameters(const ResTable_config* params)
{
- AutoMutex _lock(mLock);
- AutoMutex _lock2(mFilteredConfigLock);
-
+ mLock.lock();
if (kDebugTableGetEntry) {
ALOGI("Setting parameters: %s\n", params->toString().string());
}
mParams = *params;
- for (size_t p = 0; p < mPackageGroups.size(); p++) {
- PackageGroup* packageGroup = mPackageGroups.editItemAt(p);
+ for (size_t i=0; i<mPackageGroups.size(); i++) {
if (kDebugTableNoisy) {
- ALOGI("CLEARING BAGS FOR GROUP %zu!", p);
+ ALOGI("CLEARING BAGS FOR GROUP %zu!", i);
}
- packageGroup->clearBagCache();
-
- for (size_t t = 0; t < packageGroup->types.size(); t++) {
- TypeList& typeList = packageGroup->types.editItemAt(t);
- for (size_t ts = 0; ts < typeList.size(); ts++) {
- Type* type = typeList.editItemAt(ts);
-
- SharedVector<const ResTable_type*> newFilteredConfigs =
- std::make_shared<Vector<const ResTable_type*>>();
- for (size_t ti = 0; ti < type->configs.size(); ti++) {
- ResTable_config config;
- config.copyFromDtoH(type->configs[ti]->config);
-
- if (config.match(mParams)) {
- newFilteredConfigs->add(type->configs[ti]);
- }
- }
-
- if (kDebugTableNoisy) {
- ALOGD("Updating pkg=%zu type=%zu with %zu filtered configs",
- p, t, newFilteredConfigs->size());
- }
- type->filteredConfigs = newFilteredConfigs;
- }
- }
+ mPackageGroups[i]->clearBagCache();
}
+ mLock.unlock();
}
void ResTable::getParameters(ResTable_config* params) const
@@ -6008,29 +5974,9 @@
specFlags = -1;
}
- const Vector<const ResTable_type*>* candidateConfigs = &typeSpec->configs;
-
- SharedVector<const ResTable_type*> filteredConfigs;
- if (config && memcmp(&mParams, config, sizeof(mParams)) == 0) {
- // Grab the lock first so we can safely get the current filtered list.
- AutoMutex _lock(mFilteredConfigLock);
-
- // This configuration is equal to the one we have previously cached for,
- // so use the filtered configs.
-
- if (typeSpec->filteredConfigs) {
- // Grab a reference to the shared_ptr so it doesn't get destroyed while
- // going through this list.
- filteredConfigs = typeSpec->filteredConfigs;
-
- // Use this filtered list.
- candidateConfigs = filteredConfigs.get();
- }
- }
-
- const size_t numConfigs = candidateConfigs->size();
+ const size_t numConfigs = typeSpec->configs.size();
for (size_t c = 0; c < numConfigs; c++) {
- const ResTable_type* const thisType = candidateConfigs->itemAt(c);
+ const ResTable_type* const thisType = typeSpec->configs[c];
if (thisType == NULL) {
continue;
}