From 1cf74939f4623609341f1dd9b69616da88482c58 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Mon, 1 May 2023 14:35:48 -0700 Subject: Reland "Use reference counted pointers for ApkAssets" This reverts commit cf6e79f809034386b04ae551db815ab087f76c0a Updates: Prepare the shared pointers for the whole operation at once instead of re-locking them on each iteration. Still a regression of about 5% for changing theme's AssetManager object, vs the original 40% + change the log message to a warning as it doesn't break the app Original comment: Use reference counted pointers for ApkAssets The primary reason for memory corruption is freed ApkAssets Java expected them to only be freed in the finalizers, but there are explicit close() calls now, destroying objects that are still in use in some AssetManager2 objects This CL makes sure those AssetManagers don't assume ApkAssets always exist, but instead tries to lock them in memory for any access It also adds logging in case of deleting an assets object with any weak pointers still existing. Those will get into the bugreports attached to related bugs to help with investigation. Benchmarks don't regress, and the device appears to be working. Given that the crashes used to be pretty rare, let's wait for any new reports or lack of those. + add a missing .clang-format file to the jni directory + enabled C++23 in the project that uses AssetManager headers Bug: 197260547 Bug: 276922628 Test: unit tests + boot + benchmarks Old change id: I495fd9e012fe370a1f725dbb0265b4ee1be8d805 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b3455190124b41e2deb1774d9c05b396b73b41a2) Merged-In: Id668fbcf07db17b09691a344c04e98df83006f97 Change-Id: Id668fbcf07db17b09691a344c04e98df83006f97 --- libs/androidfw/AssetManager2.cpp | 305 ++++++++++++++++++++++----------------- 1 file changed, 175 insertions(+), 130 deletions(-) (limited to 'libs/androidfw/AssetManager2.cpp') diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 68f5e4a88c7e..3a7c91ef5b08 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -91,13 +91,14 @@ struct FindEntryResult { StringPoolRef entry_string_ref; }; -AssetManager2::AssetManager2() { - memset(&configuration_, 0, sizeof(configuration_)); +AssetManager2::AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration) + : configuration_(configuration) { + // Don't invalidate caches here as there's nothing cached yet. + SetApkAssets(apk_assets, false); } -bool AssetManager2::SetApkAssets(std::vector apk_assets, bool invalidate_caches) { - apk_assets_ = std::move(apk_assets); - BuildDynamicRefTable(); +bool AssetManager2::SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches) { + BuildDynamicRefTable(apk_assets); RebuildFilterList(); if (invalidate_caches) { InvalidateCaches(static_cast(-1)); @@ -105,7 +106,13 @@ bool AssetManager2::SetApkAssets(std::vector apk_assets, bool return true; } -void AssetManager2::BuildDynamicRefTable() { +bool AssetManager2::SetApkAssets(std::initializer_list apk_assets, + bool invalidate_caches) { + return SetApkAssets(ApkAssetsList(apk_assets.begin(), apk_assets.size()), invalidate_caches); +} + +void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { + apk_assets_.assign(apk_assets.begin(), apk_assets.end()); package_groups_.clear(); package_ids_.fill(0xff); @@ -116,16 +123,19 @@ void AssetManager2::BuildDynamicRefTable() { // Overlay resources are not directly referenced by an application so their resource ids // can change throughout the application's lifetime. Assign overlay package ids last. - std::vector sorted_apk_assets(apk_assets_); - std::stable_partition(sorted_apk_assets.begin(), sorted_apk_assets.end(), [](const ApkAssets* a) { - return !a->IsOverlay(); - }); + std::vector sorted_apk_assets; + sorted_apk_assets.reserve(apk_assets_.size()); + for (auto& asset : apk_assets) { + sorted_apk_assets.push_back(asset.get()); + } + std::stable_partition(sorted_apk_assets.begin(), sorted_apk_assets.end(), + [](auto a) { return !a->IsOverlay(); }); // The assets cookie must map to the position of the apk assets in the unsorted apk assets list. std::unordered_map apk_assets_cookies; - apk_assets_cookies.reserve(apk_assets_.size()); - for (size_t i = 0, n = apk_assets_.size(); i < n; i++) { - apk_assets_cookies[apk_assets_[i]] = static_cast(i); + apk_assets_cookies.reserve(apk_assets.size()); + for (size_t i = 0, n = apk_assets.size(); i < n; i++) { + apk_assets_cookies[apk_assets[i].get()] = static_cast(i); } // 0x01 is reserved for the android package. @@ -242,7 +252,8 @@ void AssetManager2::DumpToLog() const { std::string list; for (const auto& apk_assets : apk_assets_) { - base::StringAppendF(&list, "%s,", apk_assets->GetDebugName().c_str()); + auto assets = apk_assets.promote(); + base::StringAppendF(&list, "%s,", assets ? assets->GetDebugName().c_str() : "nullptr"); } LOG(INFO) << "ApkAssets: " << list; @@ -279,7 +290,8 @@ const ResStringPool* AssetManager2::GetStringPoolForCookie(ApkAssetsCookie cooki if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return nullptr; } - return apk_assets_[cookie]->GetLoadedArsc()->GetStringPool(); + auto assets = apk_assets_[cookie].promote(); + return assets ? assets->GetLoadedArsc()->GetStringPool() : nullptr; } const DynamicRefTable* AssetManager2::GetDynamicRefTableForPackage(uint32_t package_id) const { @@ -331,7 +343,11 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, std::string* out) const { uint8_t package_id = 0U; for (const auto& apk_assets : apk_assets_) { - const LoadedArsc* loaded_arsc = apk_assets->GetLoadedArsc(); + auto assets = apk_assets.promote(); + if (!assets) { + continue; + } + const LoadedArsc* loaded_arsc = assets->GetLoadedArsc(); if (loaded_arsc == nullptr) { continue; } @@ -384,8 +400,10 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, } bool AssetManager2::ContainsAllocatedTable() const { - return std::find_if(apk_assets_.begin(), apk_assets_.end(), - std::mem_fn(&ApkAssets::IsTableAllocated)) != apk_assets_.end(); + return std::find_if(apk_assets_.begin(), apk_assets_.end(), [](auto&& assets_weak) { + auto assets = assets_weak.promote(); + return assets && assets->IsTableAllocated(); + }) != apk_assets_.end(); } void AssetManager2::SetConfiguration(const ResTable_config& configuration) { @@ -398,8 +416,8 @@ void AssetManager2::SetConfiguration(const ResTable_config& configuration) { } } -std::set AssetManager2::GetNonSystemOverlays() const { - std::set non_system_overlays; +std::set AssetManager2::GetNonSystemOverlays() const { + std::set non_system_overlays; for (const PackageGroup& package_group : package_groups_) { bool found_system_package = false; for (const ConfiguredPackage& package : package_group.packages_) { @@ -411,7 +429,9 @@ std::set AssetManager2::GetNonSystemOverlays() const { if (!found_system_package) { for (const ConfiguredOverlay& overlay : package_group.overlays_) { - non_system_overlays.insert(apk_assets_[overlay.cookie]); + if (auto asset = apk_assets_[overlay.cookie].promote()) { + non_system_overlays.insert(std::move(asset)); + } } } } @@ -423,21 +443,24 @@ base::expected, IOError> AssetManager2::GetResourceCon bool exclude_system, bool exclude_mipmap) const { ATRACE_NAME("AssetManager::GetResourceConfigurations"); const auto non_system_overlays = - (exclude_system) ? GetNonSystemOverlays() : std::set(); + exclude_system ? GetNonSystemOverlays() : std::set(); std::set configurations; for (const PackageGroup& package_group : package_groups_) { for (size_t i = 0; i < package_group.packages_.size(); i++) { const ConfiguredPackage& package = package_group.packages_[i]; - if (exclude_system && package.loaded_package_->IsSystem()) { - continue; - } - - auto apk_assets = apk_assets_[package_group.cookies_[i]]; - if (exclude_system && apk_assets->IsOverlay() && - non_system_overlays.find(apk_assets) == non_system_overlays.end()) { - // Exclude overlays that target system resources. - continue; + if (exclude_system) { + if (package.loaded_package_->IsSystem()) { + continue; + } + if (!non_system_overlays.empty()) { + // Exclude overlays that target only system resources. + auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + if (apk_assets && apk_assets->IsOverlay() && + non_system_overlays.find(apk_assets) == non_system_overlays.end()) { + continue; + } + } } auto result = package.loaded_package_->CollectConfigurations(exclude_mipmap, &configurations); @@ -454,20 +477,23 @@ std::set AssetManager2::GetResourceLocales(bool exclude_system, ATRACE_NAME("AssetManager::GetResourceLocales"); std::set locales; const auto non_system_overlays = - (exclude_system) ? GetNonSystemOverlays() : std::set(); + exclude_system ? GetNonSystemOverlays() : std::set(); for (const PackageGroup& package_group : package_groups_) { for (size_t i = 0; i < package_group.packages_.size(); i++) { const ConfiguredPackage& package = package_group.packages_[i]; - if (exclude_system && package.loaded_package_->IsSystem()) { - continue; - } - - auto apk_assets = apk_assets_[package_group.cookies_[i]]; - if (exclude_system && apk_assets->IsOverlay() && - non_system_overlays.find(apk_assets) == non_system_overlays.end()) { - // Exclude overlays that target system resources. - continue; + if (exclude_system) { + if (package.loaded_package_->IsSystem()) { + continue; + } + if (!non_system_overlays.empty()) { + // Exclude overlays that target only system resources. + auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + if (apk_assets && apk_assets->IsOverlay() && + non_system_overlays.find(apk_assets) == non_system_overlays.end()) { + continue; + } + } } package.loaded_package_->CollectLocales(merge_equivalent_languages, &locales); @@ -492,13 +518,12 @@ std::unique_ptr AssetManager2::OpenDir(const std::string& dirname) con ATRACE_NAME("AssetManager::OpenDir"); std::string full_path = "assets/" + dirname; - std::unique_ptr> files = - util::make_unique>(); + auto files = util::make_unique>(); // Start from the back. for (auto iter = apk_assets_.rbegin(); iter != apk_assets_.rend(); ++iter) { - const ApkAssets* apk_assets = *iter; - if (apk_assets->IsOverlay()) { + auto apk_assets = iter->promote(); + if (!apk_assets || apk_assets->IsOverlay()) { continue; } @@ -527,14 +552,15 @@ std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, Asset::AccessMode mode, ApkAssetsCookie* out_cookie) const { for (int32_t i = apk_assets_.size() - 1; i >= 0; i--) { + const auto assets = apk_assets_[i].promote(); // Prevent RRO from modifying assets and other entries accessed by file // path. Explicitly asking for a path in a given package (denoted by a // cookie) is still OK. - if (apk_assets_[i]->IsOverlay()) { + if (!assets || assets->IsOverlay()) { continue; } - std::unique_ptr asset = apk_assets_[i]->GetAssetsProvider()->Open(filename, mode); + std::unique_ptr asset = assets->GetAssetsProvider()->Open(filename, mode); if (asset) { if (out_cookie != nullptr) { *out_cookie = i; @@ -555,7 +581,8 @@ std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return {}; } - return apk_assets_[cookie]->GetAssetsProvider()->Open(filename, mode); + auto assets = apk_assets_[cookie].promote(); + return assets ? assets->GetAssetsProvider()->Open(filename, mode) : nullptr; } base::expected AssetManager2::FindEntry( @@ -603,90 +630,97 @@ base::expected AssetManager2::FindEntry( } bool overlaid = false; - if (!stop_at_first_match && !ignore_configuration && !apk_assets_[result->cookie]->IsLoader()) { - for (const auto& id_map : package_group.overlays_) { - auto overlay_entry = id_map.overlay_res_maps_.Lookup(resid); - if (!overlay_entry) { - // No id map entry exists for this target resource. - continue; - } - if (overlay_entry.IsInlineValue()) { - // The target resource is overlaid by an inline value not represented by a resource. - ConfigDescription best_frro_config; - Res_value best_frro_value; - bool frro_found = false; - for( const auto& [config, value] : overlay_entry.GetInlineValue()) { - if ((!frro_found || config.isBetterThan(best_frro_config, desired_config)) - && config.match(*desired_config)) { - frro_found = true; - best_frro_config = config; - best_frro_value = value; - } - } - if (!frro_found) { + if (!stop_at_first_match && !ignore_configuration) { + auto assets = apk_assets_[result->cookie].promote(); + if (!assets) { + ALOGE("Found expired ApkAssets #%d for resource ID 0x%08x.", result->cookie, resid); + return base::unexpected(std::nullopt); + } + if (!assets->IsLoader()) { + for (const auto& id_map : package_group.overlays_) { + auto overlay_entry = id_map.overlay_res_maps_.Lookup(resid); + if (!overlay_entry) { + // No id map entry exists for this target resource. continue; } - result->entry = best_frro_value; - result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); - result->cookie = id_map.cookie; - - if (UNLIKELY(logging_enabled)) { - last_resolution_.steps.push_back( - Resolution::Step{Resolution::Step::Type::OVERLAID_INLINE, String8(), result->cookie}); - if (auto path = apk_assets_[result->cookie]->GetPath()) { - const std::string overlay_path = path->data(); - if (IsFabricatedOverlay(overlay_path)) { - // FRRO don't have package name so we use the creating package here. - String8 frro_name = String8("FRRO"); - // Get the first part of it since the expected one should be like - // {overlayPackageName}-{overlayName}-{4 alphanumeric chars}.frro - // under /data/resource-cache/. - const std::string name = overlay_path.substr(overlay_path.rfind('/') + 1); - const size_t end = name.find('-'); - if (frro_name.size() != overlay_path.size() && end != std::string::npos) { - frro_name.append(base::StringPrintf(" created by %s", - name.substr(0 /* pos */, - end).c_str()).c_str()); + if (overlay_entry.IsInlineValue()) { + // The target resource is overlaid by an inline value not represented by a resource. + ConfigDescription best_frro_config; + Res_value best_frro_value; + bool frro_found = false; + for( const auto& [config, value] : overlay_entry.GetInlineValue()) { + if ((!frro_found || config.isBetterThan(best_frro_config, desired_config)) + && config.match(*desired_config)) { + frro_found = true; + best_frro_config = config; + best_frro_value = value; + } + } + if (!frro_found) { + continue; + } + result->entry = best_frro_value; + result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); + result->cookie = id_map.cookie; + + if (UNLIKELY(logging_enabled)) { + last_resolution_.steps.push_back( + Resolution::Step{Resolution::Step::Type::OVERLAID_INLINE, String8(), result->cookie}); + if (auto path = assets->GetPath()) { + const std::string overlay_path = path->data(); + if (IsFabricatedOverlay(overlay_path)) { + // FRRO don't have package name so we use the creating package here. + String8 frro_name = String8("FRRO"); + // Get the first part of it since the expected one should be like + // {overlayPackageName}-{overlayName}-{4 alphanumeric chars}.frro + // under /data/resource-cache/. + const std::string name = overlay_path.substr(overlay_path.rfind('/') + 1); + const size_t end = name.find('-'); + if (frro_name.size() != overlay_path.size() && end != std::string::npos) { + frro_name.append(base::StringPrintf(" created by %s", + name.substr(0 /* pos */, + end).c_str()).c_str()); + } + last_resolution_.best_package_name = frro_name; + } else { + last_resolution_.best_package_name = result->package_name->c_str(); } - last_resolution_.best_package_name = frro_name; - } else { - last_resolution_.best_package_name = result->package_name->c_str(); } + overlaid = true; } - overlaid = true; + continue; } - continue; - } - auto overlay_result = FindEntry(overlay_entry.GetResourceId(), density_override, - false /* stop_at_first_match */, - false /* ignore_configuration */); - if (UNLIKELY(IsIOError(overlay_result))) { - return base::unexpected(overlay_result.error()); - } - if (!overlay_result.has_value()) { - continue; - } + auto overlay_result = FindEntry(overlay_entry.GetResourceId(), density_override, + false /* stop_at_first_match */, + false /* ignore_configuration */); + if (UNLIKELY(IsIOError(overlay_result))) { + return base::unexpected(overlay_result.error()); + } + if (!overlay_result.has_value()) { + continue; + } - if (!overlay_result->config.isBetterThan(result->config, desired_config) - && overlay_result->config.compare(result->config) != 0) { - // The configuration of the entry for the overlay must be equal to or better than the target - // configuration to be chosen as the better value. - continue; - } + if (!overlay_result->config.isBetterThan(result->config, desired_config) + && overlay_result->config.compare(result->config) != 0) { + // The configuration of the entry for the overlay must be equal to or better than the target + // configuration to be chosen as the better value. + continue; + } - result->cookie = overlay_result->cookie; - result->entry = overlay_result->entry; - result->config = overlay_result->config; - result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); + result->cookie = overlay_result->cookie; + result->entry = overlay_result->entry; + result->config = overlay_result->config; + result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); - if (UNLIKELY(logging_enabled)) { - last_resolution_.steps.push_back( - Resolution::Step{Resolution::Step::Type::OVERLAID, overlay_result->config.toString(), - overlay_result->cookie}); - last_resolution_.best_package_name = - overlay_result->package_name->c_str(); - overlaid = true; + if (UNLIKELY(logging_enabled)) { + last_resolution_.steps.push_back( + Resolution::Step{Resolution::Step::Type::OVERLAID, overlay_result->config.toString(), + overlay_result->cookie}); + last_resolution_.best_package_name = + overlay_result->package_name->c_str(); + overlaid = true; + } } } } @@ -868,7 +902,9 @@ std::string AssetManager2::GetLastResourceResolution() const { } const uint32_t resid = last_resolution_.resid; - const auto package = apk_assets_[cookie]->GetLoadedArsc()->GetPackageById(get_package_id(resid)); + auto assets = apk_assets_[cookie].promote(); + const auto package = + assets ? assets->GetLoadedArsc()->GetPackageById(get_package_id(resid)) : nullptr; std::string resource_name_string; if (package != nullptr) { @@ -898,8 +934,9 @@ std::string AssetManager2::GetLastResourceResolution() const { if (prefix == kStepStrings.end()) { continue; } - - log_stream << "\n\t" << prefix->second << ": " << apk_assets_[step.cookie]->GetDebugName(); + auto assets = apk_assets_[step.cookie].promote(); + log_stream << "\n\t" << prefix->second << ": " + << (assets ? assets->GetDebugName() : "") << " #" << step.cookie; if (!step.config_name.isEmpty()) { log_stream << " - " << step.config_name; } @@ -1563,12 +1600,20 @@ base::expected Theme::SetTo(const Theme& source) { // Determine which ApkAssets are loaded in both theme AssetManagers. const auto& src_assets = source.asset_manager_->GetApkAssets(); - for (size_t i = 0; i < src_assets.size(); i++) { - const ApkAssets* src_asset = src_assets[i]; + const auto& dest_assets = asset_manager_->GetApkAssets(); + std::vector promoted_src_assets; + promoted_src_assets.reserve(src_assets.size()); + for (const auto& src_asset : src_assets) { + promoted_src_assets.emplace_back(src_asset.promote()); + } - const auto& dest_assets = asset_manager_->GetApkAssets(); - for (size_t j = 0; j < dest_assets.size(); j++) { - const ApkAssets* dest_asset = dest_assets[j]; + for (size_t j = 0; j < dest_assets.size(); j++) { + auto dest_asset = dest_assets[j].promote(); + if (!dest_asset) { + continue; + } + for (size_t i = 0; i < promoted_src_assets.size(); i++) { + const auto& src_asset = promoted_src_assets[i]; if (src_asset != dest_asset) { // ResourcesManager caches and reuses ApkAssets when the same apk must be present in // multiple AssetManagers. Two ApkAssets point to the same version of the same resources -- cgit v1.2.3-59-g8ed1b From 657f3c6bbfb7b0a7782562c5de5fd1aecc1346b3 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Wed, 10 May 2023 13:16:23 -0700 Subject: [res] Speed up AssetManager pointer locking AssetManager needs to lock (promote()) apk asset weak pointers to use them in pretty much any operation, and often mulitple times for the same object. This CL introduces a concept of an 'operation' in AssetManager, and adds a cache for the locked assets that are retained until the end of that operation. This way we only need to lock each assets object exactly once, losing pretty much no performance. Benchmarks all returned to the pre-memory-safe state values. Bug: 280951693 Bug: 197260547 Bug: 276922628 Test: UTs + native benchmarks + java benchmarks + boot (cherry-picked from https://googleplex-android-review.git.corp.google.com/c/platform/frameworks/base/+/23146856) Merged-In: I26ef88df4f6267b5e8b89f1588f2382db74030c0 Change-Id: I26ef88df4f6267b5e8b89f1588f2382db74030c0 --- core/jni/android_util_AssetManager.cpp | 154 +++++++++++++---------- libs/androidfw/AssetManager2.cpp | 137 +++++++++++++------- libs/androidfw/include/androidfw/AssetManager2.h | 31 ++++- libs/androidfw/tests/AssetManager2_test.cpp | 32 ++++- libs/androidfw/tests/Idmap_test.cpp | 4 +- tools/aapt2/process/SymbolTable.cpp | 3 +- 6 files changed, 243 insertions(+), 118 deletions(-) (limited to 'libs/androidfw/AssetManager2.cpp') diff --git a/core/jni/android_util_AssetManager.cpp b/core/jni/android_util_AssetManager.cpp index 3d1d1433ba1c..fc53a766b772 100644 --- a/core/jni/android_util_AssetManager.cpp +++ b/core/jni/android_util_AssetManager.cpp @@ -17,6 +17,9 @@ #define ATRACE_TAG ATRACE_TAG_RESOURCES #define LOG_TAG "asset" +#include "android_runtime/android_util_AssetManager.h" + +#include #include #include #include @@ -31,7 +34,7 @@ #include "android-base/logging.h" #include "android-base/properties.h" #include "android-base/stringprintf.h" -#include "android_runtime/android_util_AssetManager.h" +#include "android_content_res_ApkAssets.h" #include "android_runtime/AndroidRuntime.h" #include "android_util_Binder.h" #include "androidfw/Asset.h" @@ -39,11 +42,9 @@ #include "androidfw/AssetManager2.h" #include "androidfw/AttributeResolution.h" #include "androidfw/MutexGuard.h" -#include +#include "androidfw/ResourceTimer.h" #include "androidfw/ResourceTypes.h" #include "androidfw/ResourceUtils.h" - -#include "android_content_res_ApkAssets.h" #include "core_jni_helpers.h" #include "jni.h" #include "nativehelper/JNIPlatformHelp.h" @@ -161,9 +162,30 @@ static Guarded& AssetManagerFromLong(jlong ptr) { return *AssetManagerForNdkAssetManager(reinterpret_cast(ptr)); } +struct ScopedLockedAssetsOperation { + ScopedLockedAssetsOperation(Guarded& guarded_am) + : am_(guarded_am), op_(am_->StartOperation()) {} + + AssetManager2& operator*() { return *am_; } + + AssetManager2* operator->() { return am_.get(); } + + AssetManager2* get() { return am_.get(); } + + private: + DISALLOW_COPY_AND_ASSIGN(ScopedLockedAssetsOperation); + + ScopedLock am_; + AssetManager2::ScopedOperation op_; +}; + +ScopedLockedAssetsOperation LockAndStartAssetManager(jlong ptr) { + return ScopedLockedAssetsOperation(AssetManagerFromLong(ptr)); +} + static jobject NativeGetOverlayableMap(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstring package_name) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); const ScopedUtfChars package_name_utf8(env, package_name); CHECK(package_name_utf8.c_str() != nullptr); const std::string std_package_name(package_name_utf8.c_str()); @@ -209,7 +231,7 @@ static jobject NativeGetOverlayableMap(JNIEnv* env, jclass /*clazz*/, jlong ptr, static jstring NativeGetOverlayablesToString(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstring package_name) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); const ScopedUtfChars package_name_utf8(env, package_name); CHECK(package_name_utf8.c_str() != nullptr); const std::string std_package_name(package_name_utf8.c_str()); @@ -320,7 +342,7 @@ static void NativeSetApkAssets(JNIEnv* env, jclass /*clazz*/, jlong ptr, apk_assets.emplace_back(*scoped_assets); } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); assetmanager->SetApkAssets(apk_assets, invalidate_caches); } @@ -370,14 +392,14 @@ static void NativeSetConfiguration(JNIEnv* env, jclass /*clazz*/, jlong ptr, jin configuration.screenLayout2 = static_cast((screen_layout & kScreenLayoutRoundMask) >> kScreenLayoutRoundShift); - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); assetmanager->SetConfiguration(configuration); } static jobject NativeGetAssignedPackageIdentifiers(JNIEnv* env, jclass /*clazz*/, jlong ptr, jboolean includeOverlays, jboolean includeLoaders) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); jobject sparse_array = env->NewObject(gSparseArrayOffsets.classObject, gSparseArrayOffsets.constructor); @@ -407,7 +429,7 @@ static jobject NativeGetAssignedPackageIdentifiers(JNIEnv* env, jclass /*clazz*/ } static jboolean ContainsAllocatedTable(JNIEnv* env, jclass /*clazz*/, jlong ptr) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); return assetmanager->ContainsAllocatedTable(); } @@ -418,7 +440,7 @@ static jobjectArray NativeList(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstring return nullptr; } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset_dir = assetmanager->OpenDir(path_utf8.c_str()); if (asset_dir == nullptr) { @@ -466,7 +488,7 @@ static jlong NativeOpenAsset(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstring a return 0; } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset = assetmanager->Open(asset_path_utf8.c_str(), static_cast(access_mode)); if (!asset) { @@ -486,7 +508,7 @@ static jobject NativeOpenAssetFd(JNIEnv* env, jclass /*clazz*/, jlong ptr, jstri ATRACE_NAME(base::StringPrintf("AssetManager::OpenAssetFd(%s)", asset_path_utf8.c_str()).c_str()); - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset = assetmanager->Open(asset_path_utf8.c_str(), Asset::ACCESS_RANDOM); if (!asset) { jniThrowException(env, "java/io/FileNotFoundException", asset_path_utf8.c_str()); @@ -512,7 +534,7 @@ static jlong NativeOpenNonAsset(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint j return 0; } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset; if (cookie != kInvalidCookie) { asset = assetmanager->OpenNonAsset(asset_path_utf8.c_str(), cookie, @@ -540,7 +562,7 @@ static jobject NativeOpenNonAssetFd(JNIEnv* env, jclass /*clazz*/, jlong ptr, ji ATRACE_NAME(base::StringPrintf("AssetManager::OpenNonAssetFd(%s)", asset_path_utf8.c_str()).c_str()); - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset; if (cookie != kInvalidCookie) { asset = assetmanager->OpenNonAsset(asset_path_utf8.c_str(), cookie, Asset::ACCESS_RANDOM); @@ -566,7 +588,7 @@ static jlong NativeOpenXmlAsset(JNIEnv* env, jobject /*clazz*/, jlong ptr, jint ATRACE_NAME(base::StringPrintf("AssetManager::OpenXmlAsset(%s)", asset_path_utf8.c_str()).c_str()); - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::unique_ptr asset; if (cookie != kInvalidCookie) { asset = assetmanager->OpenNonAsset(asset_path_utf8.c_str(), cookie, Asset::ACCESS_RANDOM); @@ -614,7 +636,8 @@ static jlong NativeOpenXmlAssetFd(JNIEnv* env, jobject /*clazz*/, jlong ptr, int std::unique_ptr asset(Asset::createFromFd(dup_fd.release(), nullptr, Asset::AccessMode::ACCESS_BUFFER)); - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + ApkAssetsCookie cookie = JavaCookieToApkAssetsCookie(jcookie); const incfs::map_ptr buffer = asset->getIncFsBuffer(true /* aligned */); @@ -637,8 +660,9 @@ static jlong NativeOpenXmlAssetFd(JNIEnv* env, jobject /*clazz*/, jlong ptr, int static jint NativeGetResourceValue(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid, jshort density, jobject typed_value, jboolean resolve_references) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); ResourceTimer _timer(ResourceTimer::Counter::GetResourceValue); + auto value = assetmanager->GetResource(static_cast(resid), false /*may_be_bag*/, static_cast(density)); if (!value.has_value()) { @@ -656,7 +680,8 @@ static jint NativeGetResourceValue(JNIEnv* env, jclass /*clazz*/, jlong ptr, jin static jint NativeGetResourceBagValue(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid, jint bag_entry_id, jobject typed_value) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag = assetmanager->GetBag(static_cast(resid)); if (!bag.has_value()) { return ApkAssetsCookieToJavaCookie(kInvalidCookie); @@ -683,7 +708,8 @@ static jint NativeGetResourceBagValue(JNIEnv* env, jclass /*clazz*/, jlong ptr, } static jintArray NativeGetStyleAttributes(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag_result = assetmanager->GetBag(static_cast(resid)); if (!bag_result.has_value()) { return nullptr; @@ -704,7 +730,8 @@ static jintArray NativeGetStyleAttributes(JNIEnv* env, jclass /*clazz*/, jlong p static jobjectArray NativeGetResourceStringArray(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag_result = assetmanager->GetBag(static_cast(resid)); if (!bag_result.has_value()) { return nullptr; @@ -725,8 +752,8 @@ static jobjectArray NativeGetResourceStringArray(JNIEnv* env, jclass /*clazz*/, } if (attr_value.type == Res_value::TYPE_STRING) { - auto apk_assets_weak = assetmanager->GetApkAssets()[attr_value.cookie]; - if (auto apk_assets = apk_assets_weak.promote()) { + const auto& apk_assets = assetmanager->GetApkAssets(attr_value.cookie); + if (apk_assets) { const ResStringPool* pool = apk_assets->GetLoadedArsc()->GetStringPool(); jstring java_string; @@ -762,7 +789,8 @@ static jobjectArray NativeGetResourceStringArray(JNIEnv* env, jclass /*clazz*/, static jintArray NativeGetResourceStringArrayInfo(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag_result = assetmanager->GetBag(static_cast(resid)); if (!bag_result.has_value()) { return nullptr; @@ -800,7 +828,8 @@ static jintArray NativeGetResourceStringArrayInfo(JNIEnv* env, jclass /*clazz*/, } static jintArray NativeGetResourceIntArray(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag_result = assetmanager->GetBag(static_cast(resid)); if (!bag_result.has_value()) { return nullptr; @@ -835,21 +864,22 @@ static jintArray NativeGetResourceIntArray(JNIEnv* env, jclass /*clazz*/, jlong } static jint NativeGetResourceArraySize(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); - auto bag = assetmanager->GetBag(static_cast(resid)); - if (!bag.has_value()) { - return -1; - } + auto assetmanager = LockAndStartAssetManager(ptr); + auto bag = assetmanager->GetBag(static_cast(resid)); + if (!bag.has_value()) { + return -1; + } return static_cast((*bag)->entry_count); } static jint NativeGetResourceArray(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid, jintArray out_data) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); - auto bag_result = assetmanager->GetBag(static_cast(resid)); - if (!bag_result.has_value()) { + auto assetmanager = LockAndStartAssetManager(ptr); + + auto bag_result = assetmanager->GetBag(static_cast(resid)); + if (!bag_result.has_value()) { return -1; - } + } const jsize out_data_length = env->GetArrayLength(out_data); if (env->ExceptionCheck()) { @@ -896,7 +926,7 @@ static jint NativeGetResourceArray(JNIEnv* env, jclass /*clazz*/, jlong ptr, jin } static jint NativeGetParentThemeIdentifier(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); const auto parentThemeResId = assetmanager->GetParentThemeResourceId(resid); return parentThemeResId.value_or(0); } @@ -923,7 +953,7 @@ static jint NativeGetResourceIdentifier(JNIEnv* env, jclass /*clazz*/, jlong ptr package = package_utf8.c_str(); } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto resid = assetmanager->GetResourceId(name_utf8.c_str(), type, package); if (!resid.has_value()) { return 0; @@ -933,7 +963,7 @@ static jint NativeGetResourceIdentifier(JNIEnv* env, jclass /*clazz*/, jlong ptr } static jstring NativeGetResourceName(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto name = assetmanager->GetResourceName(static_cast(resid)); if (!name.has_value()) { return nullptr; @@ -944,7 +974,7 @@ static jstring NativeGetResourceName(JNIEnv* env, jclass /*clazz*/, jlong ptr, j } static jstring NativeGetResourcePackageName(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto name = assetmanager->GetResourceName(static_cast(resid)); if (!name.has_value()) { return nullptr; @@ -957,7 +987,7 @@ static jstring NativeGetResourcePackageName(JNIEnv* env, jclass /*clazz*/, jlong } static jstring NativeGetResourceTypeName(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto name = assetmanager->GetResourceName(static_cast(resid)); if (!name.has_value()) { return nullptr; @@ -972,7 +1002,7 @@ static jstring NativeGetResourceTypeName(JNIEnv* env, jclass /*clazz*/, jlong pt } static jstring NativeGetResourceEntryName(JNIEnv* env, jclass /*clazz*/, jlong ptr, jint resid) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto name = assetmanager->GetResourceName(static_cast(resid)); if (!name.has_value()) { return nullptr; @@ -990,14 +1020,14 @@ static void NativeSetResourceResolutionLoggingEnabled(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr, jboolean enabled) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); assetmanager->SetResourceResolutionLoggingEnabled(enabled); } static jstring NativeGetLastResourceResolution(JNIEnv* env, jclass /*clazz*/, jlong ptr) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::string resolution = assetmanager->GetLastResourceResolution(); if (resolution.empty()) { return nullptr; @@ -1008,7 +1038,7 @@ static jstring NativeGetLastResourceResolution(JNIEnv* env, static jobjectArray NativeGetLocales(JNIEnv* env, jclass /*class*/, jlong ptr, jboolean exclude_system) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); std::set locales = assetmanager->GetResourceLocales(exclude_system, true /*merge_equivalent_languages*/); @@ -1046,7 +1076,7 @@ static jobject ConstructConfigurationObject(JNIEnv* env, const ResTable_config& } static jobjectArray GetSizeAndUiModeConfigurations(JNIEnv* env, jlong ptr) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); auto configurations = assetmanager->GetResourceConfigurations(true /*exclude_system*/, false /*exclude_mipmap*/); if (!configurations.has_value()) { @@ -1080,12 +1110,10 @@ static jobjectArray NativeGetSizeAndUiModeConfigurations(JNIEnv* env, jclass /*c return GetSizeAndUiModeConfigurations(env, ptr); } -static jintArray NativeAttributeResolutionStack( - JNIEnv* env, jclass /*clazz*/, jlong ptr, - jlong theme_ptr, jint xml_style_res, - jint def_style_attr, jint def_style_resid) { - - ScopedLock assetmanager(AssetManagerFromLong(ptr)); +static jintArray NativeAttributeResolutionStack(JNIEnv* env, jclass /*clazz*/, jlong ptr, + jlong theme_ptr, jint xml_style_res, + jint def_style_attr, jint def_style_resid) { + auto assetmanager = LockAndStartAssetManager(ptr); Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; @@ -1120,7 +1148,7 @@ static jintArray NativeAttributeResolutionStack( static void NativeApplyStyle(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlong theme_ptr, jint def_style_attr, jint def_style_resid, jlong xml_parser_ptr, jintArray java_attrs, jlong out_values_ptr, jlong out_indices_ptr) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; @@ -1195,7 +1223,7 @@ static jboolean NativeResolveAttrs(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlo } } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; @@ -1254,7 +1282,7 @@ static jboolean NativeRetrieveAttributes(JNIEnv* env, jclass /*clazz*/, jlong pt } } - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); ResourceTimer _timer(ResourceTimer::Counter::RetrieveAttributes); ResXMLParser* xml_parser = reinterpret_cast(xml_parser_ptr); auto result = @@ -1272,7 +1300,7 @@ static jboolean NativeRetrieveAttributes(JNIEnv* env, jclass /*clazz*/, jlong pt } static jlong NativeThemeCreate(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); return reinterpret_cast(assetmanager->NewTheme().release()); } @@ -1287,7 +1315,7 @@ static jlong NativeGetThemeFreeFunction(JNIEnv* /*env*/, jclass /*clazz*/) { static void NativeThemeApplyStyle(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlong theme_ptr, jint resid, jboolean force) { // AssetManager is accessed via the theme, so grab an explicit lock here. - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; @@ -1305,7 +1333,7 @@ static void NativeThemeRebase(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlong th jint style_count) { // Lock both the original asset manager of the theme and the new asset manager to be used for the // theme. - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); uint32_t* style_id_args = nullptr; if (style_ids != nullptr) { @@ -1348,25 +1376,23 @@ static void NativeThemeCopy(JNIEnv* env, jclass /*clazz*/, jlong dst_asset_manag Theme* dst_theme = reinterpret_cast(dst_theme_ptr); Theme* src_theme = reinterpret_cast(src_theme_ptr); - ScopedLock src_assetmanager(AssetManagerFromLong(src_asset_manager_ptr)); + auto src_assetmanager = LockAndStartAssetManager(src_asset_manager_ptr); CHECK(src_theme->GetAssetManager() == &(*src_assetmanager)); - (void) src_assetmanager; if (dst_asset_manager_ptr != src_asset_manager_ptr) { - ScopedLock dst_assetmanager(AssetManagerFromLong(dst_asset_manager_ptr)); + auto dst_assetmanager = LockAndStartAssetManager(dst_asset_manager_ptr); CHECK(dst_theme->GetAssetManager() == &(*dst_assetmanager)); - (void) dst_assetmanager; - dst_theme->SetTo(*src_theme); } else { - dst_theme->SetTo(*src_theme); + dst_theme->SetTo(*src_theme); } } static jint NativeThemeGetAttributeValue(JNIEnv* env, jclass /*clazz*/, jlong ptr, jlong theme_ptr, jint resid, jobject typed_value, jboolean resolve_references) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); + Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; @@ -1389,7 +1415,7 @@ static jint NativeThemeGetAttributeValue(JNIEnv* env, jclass /*clazz*/, jlong pt static void NativeThemeDump(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr, jlong theme_ptr, jint priority, jstring tag, jstring prefix) { - ScopedLock assetmanager(AssetManagerFromLong(ptr)); + auto assetmanager = LockAndStartAssetManager(ptr); Theme* theme = reinterpret_cast(theme_ptr); CHECK(theme->GetAssetManager() == &(*assetmanager)); (void) assetmanager; diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 3a7c91ef5b08..d33b592bddc6 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -112,7 +112,15 @@ bool AssetManager2::SetApkAssets(std::initializer_list apk_assets, } void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { - apk_assets_.assign(apk_assets.begin(), apk_assets.end()); + auto op = StartOperation(); + + apk_assets_.resize(apk_assets.size()); + for (size_t i = 0; i != apk_assets.size(); ++i) { + apk_assets_[i].first = apk_assets[i]; + // Let's populate the locked assets right away as we're going to need them here later. + apk_assets_[i].second = apk_assets[i]; + } + package_groups_.clear(); package_ids_.fill(0xff); @@ -124,7 +132,7 @@ void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { // Overlay resources are not directly referenced by an application so their resource ids // can change throughout the application's lifetime. Assign overlay package ids last. std::vector sorted_apk_assets; - sorted_apk_assets.reserve(apk_assets_.size()); + sorted_apk_assets.reserve(apk_assets.size()); for (auto& asset : apk_assets) { sorted_apk_assets.push_back(asset.get()); } @@ -250,9 +258,10 @@ void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { void AssetManager2::DumpToLog() const { LOG(INFO) << base::StringPrintf("AssetManager2(this=%p)", this); + auto op = StartOperation(); std::string list; - for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); + for (size_t i = 0; i < apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); base::StringAppendF(&list, "%s,", assets ? assets->GetDebugName().c_str() : "nullptr"); } LOG(INFO) << "ApkAssets: " << list; @@ -290,7 +299,8 @@ const ResStringPool* AssetManager2::GetStringPoolForCookie(ApkAssetsCookie cooki if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return nullptr; } - auto assets = apk_assets_[cookie].promote(); + auto op = StartOperation(); + const auto& assets = GetApkAssets(cookie); return assets ? assets->GetLoadedArsc()->GetStringPool() : nullptr; } @@ -341,9 +351,10 @@ const std::unordered_map* bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, std::string* out) const { + auto op = StartOperation(); uint8_t package_id = 0U; - for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); + for (size_t i = 0; i != apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); if (!assets) { continue; } @@ -400,10 +411,14 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, } bool AssetManager2::ContainsAllocatedTable() const { - return std::find_if(apk_assets_.begin(), apk_assets_.end(), [](auto&& assets_weak) { - auto assets = assets_weak.promote(); - return assets && assets->IsTableAllocated(); - }) != apk_assets_.end(); + auto op = StartOperation(); + for (size_t i = 0; i != apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); + if (assets && assets->IsTableAllocated()) { + return true; + } + } + return false; } void AssetManager2::SetConfiguration(const ResTable_config& configuration) { @@ -428,8 +443,9 @@ std::set AssetManager2::GetNonSystemOverlays() cons } if (!found_system_package) { + auto op = StartOperation(); for (const ConfiguredOverlay& overlay : package_group.overlays_) { - if (auto asset = apk_assets_[overlay.cookie].promote()) { + if (const auto& asset = GetApkAssets(overlay.cookie)) { non_system_overlays.insert(std::move(asset)); } } @@ -442,6 +458,8 @@ std::set AssetManager2::GetNonSystemOverlays() cons base::expected, IOError> AssetManager2::GetResourceConfigurations( bool exclude_system, bool exclude_mipmap) const { ATRACE_NAME("AssetManager::GetResourceConfigurations"); + auto op = StartOperation(); + const auto non_system_overlays = exclude_system ? GetNonSystemOverlays() : std::set(); @@ -455,7 +473,7 @@ base::expected, IOError> AssetManager2::GetResourceCon } if (!non_system_overlays.empty()) { // Exclude overlays that target only system resources. - auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + const auto& apk_assets = GetApkAssets(package_group.cookies_[i]); if (apk_assets && apk_assets->IsOverlay() && non_system_overlays.find(apk_assets) == non_system_overlays.end()) { continue; @@ -475,6 +493,8 @@ base::expected, IOError> AssetManager2::GetResourceCon std::set AssetManager2::GetResourceLocales(bool exclude_system, bool merge_equivalent_languages) const { ATRACE_NAME("AssetManager::GetResourceLocales"); + auto op = StartOperation(); + std::set locales; const auto non_system_overlays = exclude_system ? GetNonSystemOverlays() : std::set(); @@ -488,7 +508,7 @@ std::set AssetManager2::GetResourceLocales(bool exclude_system, } if (!non_system_overlays.empty()) { // Exclude overlays that target only system resources. - auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + const auto& apk_assets = GetApkAssets(package_group.cookies_[i]); if (apk_assets && apk_assets->IsOverlay() && non_system_overlays.find(apk_assets) == non_system_overlays.end()) { continue; @@ -516,13 +536,14 @@ std::unique_ptr AssetManager2::Open(const std::string& filename, ApkAsset std::unique_ptr AssetManager2::OpenDir(const std::string& dirname) const { ATRACE_NAME("AssetManager::OpenDir"); + auto op = StartOperation(); std::string full_path = "assets/" + dirname; auto files = util::make_unique>(); // Start from the back. - for (auto iter = apk_assets_.rbegin(); iter != apk_assets_.rend(); ++iter) { - auto apk_assets = iter->promote(); + for (size_t i = apk_assets_.size(); i > 0; --i) { + const auto& apk_assets = GetApkAssets(i - 1); if (!apk_assets || apk_assets->IsOverlay()) { continue; } @@ -551,8 +572,9 @@ std::unique_ptr AssetManager2::OpenDir(const std::string& dirname) con std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, Asset::AccessMode mode, ApkAssetsCookie* out_cookie) const { - for (int32_t i = apk_assets_.size() - 1; i >= 0; i--) { - const auto assets = apk_assets_[i].promote(); + auto op = StartOperation(); + for (size_t i = apk_assets_.size(); i > 0; i--) { + const auto& assets = GetApkAssets(i - 1); // Prevent RRO from modifying assets and other entries accessed by file // path. Explicitly asking for a path in a given package (denoted by a // cookie) is still OK. @@ -581,7 +603,8 @@ std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return {}; } - auto assets = apk_assets_[cookie].promote(); + auto op = StartOperation(); + const auto& assets = GetApkAssets(cookie); return assets ? assets->GetAssetsProvider()->Open(filename, mode) : nullptr; } @@ -595,6 +618,8 @@ base::expected AssetManager2::FindEntry( last_resolution_.resid = resid; } + auto op = StartOperation(); + // Might use this if density_override != 0. ResTable_config density_override_config; @@ -631,7 +656,7 @@ base::expected AssetManager2::FindEntry( bool overlaid = false; if (!stop_at_first_match && !ignore_configuration) { - auto assets = apk_assets_[result->cookie].promote(); + const auto& assets = GetApkAssets(result->cookie); if (!assets) { ALOGE("Found expired ApkAssets #%d for resource ID 0x%08x.", result->cookie, resid); return base::unexpected(std::nullopt); @@ -873,13 +898,7 @@ base::expected AssetManager2::FindEntryInternal( } void AssetManager2::ResetResourceResolution() const { - last_resolution_.cookie = kInvalidCookie; - last_resolution_.resid = 0; - last_resolution_.steps.clear(); - last_resolution_.type_string_ref = StringPoolRef(); - last_resolution_.entry_string_ref = StringPoolRef(); - last_resolution_.best_config_name.clear(); - last_resolution_.best_package_name.clear(); + last_resolution_ = Resolution{}; } void AssetManager2::SetResourceResolutionLoggingEnabled(bool enabled) { @@ -901,8 +920,10 @@ std::string AssetManager2::GetLastResourceResolution() const { return {}; } + auto op = StartOperation(); + const uint32_t resid = last_resolution_.resid; - auto assets = apk_assets_[cookie].promote(); + const auto& assets = GetApkAssets(cookie); const auto package = assets ? assets->GetLoadedArsc()->GetPackageById(get_package_id(resid)) : nullptr; @@ -934,7 +955,7 @@ std::string AssetManager2::GetLastResourceResolution() const { if (prefix == kStepStrings.end()) { continue; } - auto assets = apk_assets_[step.cookie].promote(); + const auto& assets = GetApkAssets(step.cookie); log_stream << "\n\t" << prefix->second << ": " << (assets ? assets->GetDebugName() : "") << " #" << step.cookie; if (!step.config_name.isEmpty()) { @@ -1466,6 +1487,37 @@ void AssetManager2::ForEachPackage(base::function_ref 0) << "Must have an operation running"; + + if (cookie < 0 || cookie >= apk_assets_.size()) { + static const ApkAssetsPtr empty{}; + return empty; + } + auto& [wptr, res] = apk_assets_[cookie]; + if (!res) { + res = wptr.promote(); + } + return res; +} + Theme::Theme(AssetManager2* asset_manager) : asset_manager_(asset_manager) { } @@ -1598,22 +1650,16 @@ base::expected Theme::SetTo(const Theme& source) { using SourceToDestinationRuntimePackageMap = std::unordered_map; std::unordered_map src_asset_cookie_id_map; - // Determine which ApkAssets are loaded in both theme AssetManagers. - const auto& src_assets = source.asset_manager_->GetApkAssets(); - const auto& dest_assets = asset_manager_->GetApkAssets(); - std::vector promoted_src_assets; - promoted_src_assets.reserve(src_assets.size()); - for (const auto& src_asset : src_assets) { - promoted_src_assets.emplace_back(src_asset.promote()); - } + auto op_src = source.asset_manager_->StartOperation(); + auto op_dst = asset_manager_->StartOperation(); - for (size_t j = 0; j < dest_assets.size(); j++) { - auto dest_asset = dest_assets[j].promote(); - if (!dest_asset) { + for (size_t i = 0; i < source.asset_manager_->GetApkAssetsCount(); i++) { + const auto& src_asset = source.asset_manager_->GetApkAssets(i); + if (!src_asset) { continue; } - for (size_t i = 0; i < promoted_src_assets.size(); i++) { - const auto& src_asset = promoted_src_assets[i]; + for (int j = 0; j < asset_manager_->GetApkAssetsCount(); j++) { + const auto& dest_asset = asset_manager_->GetApkAssets(j); if (src_asset != dest_asset) { // ResourcesManager caches and reuses ApkAssets when the same apk must be present in // multiple AssetManagers. Two ApkAssets point to the same version of the same resources @@ -1739,4 +1785,11 @@ void Theme::Dump() const { } } +AssetManager2::ScopedOperation::ScopedOperation(const AssetManager2& am) : am_(am) { +} + +AssetManager2::ScopedOperation::~ScopedOperation() { + am_.FinishOperation(); +} + } // namespace android diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h index e276cd211ee7..fc391bc2ce67 100644 --- a/libs/androidfw/include/androidfw/AssetManager2.h +++ b/libs/androidfw/include/androidfw/AssetManager2.h @@ -102,9 +102,20 @@ class AssetManager2 { AssetManager2() = default; explicit AssetManager2(AssetManager2&& other) = default; - AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration); + struct ScopedOperation { + DISALLOW_COPY_AND_ASSIGN(ScopedOperation); + friend AssetManager2; + const AssetManager2& am_; + ScopedOperation(const AssetManager2& am); + + public: + ~ScopedOperation(); + }; + + [[nodiscard]] ScopedOperation StartOperation() const; + // Sets/resets the underlying ApkAssets for this AssetManager. The ApkAssets // are not owned by the AssetManager, and must have a longer lifetime. // @@ -114,8 +125,9 @@ class AssetManager2 { bool SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches = true); bool SetApkAssets(std::initializer_list apk_assets, bool invalidate_caches = true); - inline const std::vector& GetApkAssets() const { - return apk_assets_; + const ApkAssetsPtr& GetApkAssets(ApkAssetsCookie cookie) const; + int GetApkAssetsCount() const { + return int(apk_assets_.size()); } // Returns the string pool for the given asset cookie. @@ -426,9 +438,16 @@ class AssetManager2 { base::expected GetBag( uint32_t resid, std::vector& child_resids) const; + // Finish an operation that was running with the current asset manager, and clean up the + // promoted apk assets when the last operation ends. + void FinishOperation() const; + // The ordered list of ApkAssets to search. These are not owned by the AssetManager, and must // have a longer lifetime. - std::vector apk_assets_; + // The second pair element is the promoted version of the assets, that is held for the duration + // of the currently running operation. FinishOperation() clears all promoted assets to make sure + // they can be released when the system needs that. + mutable std::vector> apk_assets_; // DynamicRefTables for shared library package resolution. // These are ordered according to apk_assets_. The mappings may change depending on what is @@ -455,6 +474,10 @@ class AssetManager2 { // Cached set of resolved resource values. mutable std::unordered_map cached_resolved_values_; + // Tracking the number of the started operations running with the current AssetManager. + // Finishing the last one clears all promoted apk assets. + mutable int number_of_running_scoped_operations_ = 0; + // Whether or not to save resource resolution steps bool resource_resolution_logging_enabled_ = false; diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp index 5a5bafdf829a..df3fa02ce44c 100644 --- a/libs/androidfw/tests/AssetManager2_test.cpp +++ b/libs/androidfw/tests/AssetManager2_test.cpp @@ -207,11 +207,11 @@ TEST_F(AssetManager2Test, AssignsOverlayPackageIdLast) { AssetManager2 assetmanager; assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); - auto apk_assets = assetmanager.GetApkAssets(); - ASSERT_EQ(3, apk_assets.size()); - ASSERT_EQ(overlayable_assets_, apk_assets[0].promote()); - ASSERT_EQ(overlay_assets_, apk_assets[1].promote()); - ASSERT_EQ(lib_one_assets_, apk_assets[2].promote()); + ASSERT_EQ(3, assetmanager.GetApkAssetsCount()); + auto op = assetmanager.StartOperation(); + ASSERT_EQ(overlayable_assets_, assetmanager.GetApkAssets(0)); + ASSERT_EQ(overlay_assets_, assetmanager.GetApkAssets(1)); + ASSERT_EQ(lib_one_assets_, assetmanager.GetApkAssets(2)); auto get_first_package_id = [&assetmanager](auto apkAssets) -> uint8_t { return assetmanager.GetAssignedPackageId(apkAssets->GetLoadedArsc()->GetPackages()[0].get()); @@ -834,4 +834,26 @@ TEST_F(AssetManager2Test, GetOverlayablesToString) { std::string::npos); } +TEST_F(AssetManager2Test, GetApkAssets) { + AssetManager2 assetmanager; + assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); + + ASSERT_EQ(3, assetmanager.GetApkAssetsCount()); + EXPECT_EQ(1, overlayable_assets_->getStrongCount()); + EXPECT_EQ(1, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); + + { + auto op = assetmanager.StartOperation(); + ASSERT_EQ(overlayable_assets_, assetmanager.GetApkAssets(0)); + ASSERT_EQ(overlay_assets_, assetmanager.GetApkAssets(1)); + EXPECT_EQ(2, overlayable_assets_->getStrongCount()); + EXPECT_EQ(2, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); + } + EXPECT_EQ(1, overlayable_assets_->getStrongCount()); + EXPECT_EQ(1, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); +} + } // namespace android diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp index 568e041ebe5b..60aa7d88925d 100644 --- a/libs/androidfw/tests/Idmap_test.cpp +++ b/libs/androidfw/tests/Idmap_test.cpp @@ -66,9 +66,9 @@ class IdmapTest : public ::testing::Test { std::string GetStringFromApkAssets(const AssetManager2& asset_manager, const AssetManager2::SelectedValue& value) { - auto assets = asset_manager.GetApkAssets(); + auto op = asset_manager.StartOperation(); const ResStringPool* string_pool = - assets[value.cookie].promote()->GetLoadedArsc()->GetStringPool(); + asset_manager.GetApkAssets(value.cookie)->GetLoadedArsc()->GetStringPool(); return GetStringFromPool(string_pool, value.data); } diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index b75458a7b8b6..d78baf9ffeb4 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -260,10 +260,11 @@ bool AssetManagerSymbolSource::IsPackageDynamic(uint32_t packageId, static std::unique_ptr LookupAttributeInTable( android::AssetManager2& am, ResourceId id) { using namespace android; - if (am.GetApkAssets().empty()) { + if (am.GetApkAssetsCount() == 0) { return {}; } + auto op = am.StartOperation(); auto bag_result = am.GetBag(id.id); if (!bag_result.has_value()) { return nullptr; -- cgit v1.2.3-59-g8ed1b From acb04a0a2c5b6bf92c07c45ba0d6746a6ac9d1b5 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Wed, 6 Sep 2023 18:11:38 -0700 Subject: Move Theme::Entry definition to top of file It needs to be at least above this line: theme->entries_.reserve(kInitialReserveSize); Otherwise, after upgrading libc++, the compiler fails with errors about an incomplete type, e.g.: include/c++/v1/vector:839:62: error: arithmetic on a pointer to an incomplete type 'android::Theme::Entry' include/c++/v1/__memory/uninitialized_algorithms.h:609:5: error: cannot increment value of type 'std::reverse_iterator' Bug: b/175635923 Test: treehugger Change-Id: I6c231184c3a4c0e67dd29d43560b0c28778883db Merged-In: I6c231184c3a4c0e67dd29d43560b0c28778883db --- libs/androidfw/AssetManager2.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'libs/androidfw/AssetManager2.cpp') diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index bff197cc290f..c9bdbb4f3fbc 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -91,6 +91,12 @@ struct FindEntryResult { StringPoolRef entry_string_ref; }; +struct Theme::Entry { + ApkAssetsCookie cookie; + uint32_t type_spec_flags; + Res_value value; +}; + AssetManager2::AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration) : configuration_(configuration) { // Don't invalidate caches here as there's nothing cached yet. @@ -1523,12 +1529,6 @@ Theme::Theme(AssetManager2* asset_manager) : asset_manager_(asset_manager) { Theme::~Theme() = default; -struct Theme::Entry { - ApkAssetsCookie cookie; - uint32_t type_spec_flags; - Res_value value; -}; - base::expected Theme::ApplyStyle(uint32_t resid, bool force) { ATRACE_NAME("Theme::ApplyStyle"); -- cgit v1.2.3-59-g8ed1b