summaryrefslogtreecommitdiff
path: root/tools
diff options
context:
space:
mode:
author Yurii Zubrytskyi <zyy@google.com> 2023-04-13 02:32:45 -0700
committer Yurii Zubrytskyi <zyy@google.com> 2023-04-19 10:41:15 -0700
commitc357f719ed3734aee05418808699a48dd150a1a5 (patch)
tree8b0026b5cad06a6814debfb9265765e4d3e4fb08 /tools
parent77224899f2cc319443f00905e781432dbc2ceaba (diff)
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 Change-Id: I495fd9e012fe370a1f725dbb0265b4ee1be8d805
Diffstat (limited to 'tools')
-rw-r--r--tools/aapt2/cmd/Link_test.cpp6
-rw-r--r--tools/aapt2/process/SymbolTable.cpp12
-rw-r--r--tools/aapt2/process/SymbolTable.h2
3 files changed, 7 insertions, 13 deletions
diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp
index e629bafbdd25..7096f5cc54e3 100644
--- a/tools/aapt2/cmd/Link_test.cpp
+++ b/tools/aapt2/cmd/Link_test.cpp
@@ -575,7 +575,7 @@ TEST_F(LinkTest, StagedAndroidApi) {
android::AssetManager2 am;
auto android_asset = android::ApkAssets::Load(android_apk);
ASSERT_THAT(android_asset, NotNull());
- ASSERT_TRUE(am.SetApkAssets({android_asset.get()}));
+ ASSERT_TRUE(am.SetApkAssets({android_asset}));
auto result = am.GetResourceId("android:attr/finalized_res");
ASSERT_TRUE(result.has_value());
@@ -631,7 +631,7 @@ TEST_F(LinkTest, FinalizedAndroidApi) {
auto app_against_non_final = android::ApkAssets::Load(app_apk);
ASSERT_THAT(android_asset, NotNull());
ASSERT_THAT(app_against_non_final, NotNull());
- ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_non_final.get()}));
+ ASSERT_TRUE(am.SetApkAssets({android_asset, app_against_non_final}));
auto result = am.GetResourceId("android:attr/finalized_res");
ASSERT_TRUE(result.has_value());
@@ -667,7 +667,7 @@ TEST_F(LinkTest, FinalizedAndroidApi) {
auto app_against_final = android::ApkAssets::Load(app_apk_respin);
ASSERT_THAT(app_against_final, NotNull());
- ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_final.get()}));
+ ASSERT_TRUE(am.SetApkAssets({android_asset, app_against_final}));
{
auto style = am.GetBag(0x7f020000);
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index bca62da447b0..b75458a7b8b6 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -220,15 +220,9 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName(
bool AssetManagerSymbolSource::AddAssetPath(StringPiece path) {
TRACE_CALL();
- if (std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path.data())) {
+ if (auto apk = ApkAssets::Load(path.data())) {
apk_assets_.push_back(std::move(apk));
-
- std::vector<const ApkAssets*> apk_assets;
- for (const std::unique_ptr<const ApkAssets>& apk_asset : apk_assets_) {
- apk_assets.push_back(apk_asset.get());
- }
-
- asset_manager_.SetApkAssets(apk_assets);
+ asset_manager_.SetApkAssets(apk_assets_);
return true;
}
return false;
@@ -251,7 +245,7 @@ bool AssetManagerSymbolSource::IsPackageDynamic(uint32_t packageId,
return true;
}
- for (const std::unique_ptr<const ApkAssets>& assets : apk_assets_) {
+ for (auto&& assets : apk_assets_) {
for (const std::unique_ptr<const android::LoadedPackage>& loaded_package
: assets->GetLoadedArsc()->GetPackages()) {
if (package_name == loaded_package->GetPackageName() && loaded_package->IsDynamic()) {
diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h
index b09ff702ca58..36eb0bab6046 100644
--- a/tools/aapt2/process/SymbolTable.h
+++ b/tools/aapt2/process/SymbolTable.h
@@ -207,8 +207,8 @@ class AssetManagerSymbolSource : public ISymbolSource {
}
private:
+ std::vector<android::AssetManager2::ApkAssetsPtr> apk_assets_;
android::AssetManager2 asset_manager_;
- std::vector<std::unique_ptr<const android::ApkAssets>> apk_assets_;
DISALLOW_COPY_AND_ASSIGN(AssetManagerSymbolSource);
};