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

1 files changed, 26 insertions, 32 deletions

diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp
index 15aaae25f754..f0c639574a9f 100644
--- a/libs/androidfw/ApkAssets.cpp
+++ b/libs/androidfw/ApkAssets.cpp
@@ -27,39 +27,34 @@ using base::unique_fd;
 
 constexpr const char* kResourcesArsc = "resources.arsc";
 
-ApkAssets::ApkAssets(std::unique_ptr<Asset> resources_asset,
+ApkAssets::ApkAssets(PrivateConstructorUtil, std::unique_ptr<Asset> resources_asset,
                      std::unique_ptr<LoadedArsc> loaded_arsc,
-                     std::unique_ptr<AssetsProvider> assets,
-                     package_property_t property_flags,
-                     std::unique_ptr<Asset> idmap_asset,
-                     std::unique_ptr<LoadedIdmap> loaded_idmap)
+                     std::unique_ptr<AssetsProvider> assets, package_property_t property_flags,
+                     std::unique_ptr<Asset> idmap_asset, std::unique_ptr<LoadedIdmap> loaded_idmap)
     : resources_asset_(std::move(resources_asset)),
       loaded_arsc_(std::move(loaded_arsc)),
       assets_provider_(std::move(assets)),
       property_flags_(property_flags),
       idmap_asset_(std::move(idmap_asset)),
-      loaded_idmap_(std::move(loaded_idmap)) {}
+      loaded_idmap_(std::move(loaded_idmap)) {
+}
 
-std::unique_ptr<ApkAssets> ApkAssets::Load(const std::string& path, package_property_t flags) {
+ApkAssetsPtr ApkAssets::Load(const std::string& path, package_property_t flags) {
   return Load(ZipAssetsProvider::Create(path, flags), flags);
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::LoadFromFd(base::unique_fd fd,
-                                                 const std::string& debug_name,
-                                                 package_property_t flags,
-                                                 off64_t offset,
-                                                 off64_t len) {
+ApkAssetsPtr ApkAssets::LoadFromFd(base::unique_fd fd, const std::string& debug_name,
+                                   package_property_t flags, off64_t offset, off64_t len) {
   return Load(ZipAssetsProvider::Create(std::move(fd), debug_name, offset, len), flags);
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::Load(std::unique_ptr<AssetsProvider> assets,
-                                           package_property_t flags) {
+ApkAssetsPtr ApkAssets::Load(std::unique_ptr<AssetsProvider> assets, package_property_t flags) {
   return LoadImpl(std::move(assets), flags, nullptr /* idmap_asset */, nullptr /* loaded_idmap */);
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset,
-                                                std::unique_ptr<AssetsProvider> assets,
-                                                package_property_t flags) {
+ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset,
+                                  std::unique_ptr<AssetsProvider> assets,
+                                  package_property_t flags) {
   if (resources_asset == nullptr) {
     return {};
   }
@@ -67,8 +62,7 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadTable(std::unique_ptr<Asset> resources
                   nullptr /* loaded_idmap */);
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::LoadOverlay(const std::string& idmap_path,
-                                                  package_property_t flags) {
+ApkAssetsPtr ApkAssets::LoadOverlay(const std::string& idmap_path, package_property_t flags) {
   CHECK((flags & PROPERTY_LOADER) == 0U) << "Cannot load RROs through loaders";
   auto idmap_asset = AssetsProvider::CreateAssetFromFile(idmap_path);
   if (idmap_asset == nullptr) {
@@ -103,10 +97,10 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadOverlay(const std::string& idmap_path,
                   std::move(loaded_idmap));
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> assets,
-                                               package_property_t property_flags,
-                                               std::unique_ptr<Asset> idmap_asset,
-                                               std::unique_ptr<LoadedIdmap> loaded_idmap) {
+ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> assets,
+                                 package_property_t property_flags,
+                                 std::unique_ptr<Asset> idmap_asset,
+                                 std::unique_ptr<LoadedIdmap> loaded_idmap) {
   if (assets == nullptr) {
     return {};
   }
@@ -125,11 +119,11 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> a
                   std::move(idmap_asset), std::move(loaded_idmap));
 }
 
-std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_asset,
-                                               std::unique_ptr<AssetsProvider> assets,
-                                               package_property_t property_flags,
-                                               std::unique_ptr<Asset> idmap_asset,
-                                               std::unique_ptr<LoadedIdmap> loaded_idmap) {
+ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_asset,
+                                 std::unique_ptr<AssetsProvider> assets,
+                                 package_property_t property_flags,
+                                 std::unique_ptr<Asset> idmap_asset,
+                                 std::unique_ptr<LoadedIdmap> loaded_idmap) {
   if (assets == nullptr ) {
     return {};
   }
@@ -155,10 +149,9 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_
     return {};
   }
 
-  return std::unique_ptr<ApkAssets>(new ApkAssets(std::move(resources_asset),
-                                                  std::move(loaded_arsc), std::move(assets),
-                                                  property_flags, std::move(idmap_asset),
-                                                  std::move(loaded_idmap)));
+  return ApkAssetsPtr::make(PrivateConstructorUtil{}, std::move(resources_asset),
+                            std::move(loaded_arsc), std::move(assets), property_flags,
+                            std::move(idmap_asset), std::move(loaded_idmap));
 }
 
 std::optional<std::string_view> ApkAssets::GetPath() const {
@@ -174,4 +167,5 @@ bool ApkAssets::IsUpToDate() const {
   return IsLoader() || ((!loaded_idmap_ || loaded_idmap_->IsUpToDate())
                         && assets_provider_->IsUpToDate());
 }
+
 }  // namespace android