Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3d13a4f7 authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi
Browse files

[res] Make sure cached container is retained off cache 

The object in the cache may get removed by a different thread,
so getting it from the cache needs to also take (shared)
ownership instead of relying just on the cache itself.

This CL makes the cache hols shared_ptr<> and getting it
increments the ref counter, so the object won't go away
anymore

+ fix a few small issues in Idmap - const return types
  and bad formatting

Bug: 332234677
Flag: EXEMPT bugfix
Test: build + boot
Change-Id: I8e666e380a58b45142ddbd196dd684e5874fd2a6
parent 04f157c7

cmds/idmap2/idmap2d/Idmap2Service.cpp

+8 −3
Original line number Diff line number Diff line
@@ -78,6 +78,11 @@ PolicyBitmask ConvertAidlArgToPolicyBitmask(int32_t arg) { 


namespace android::os {



template <typename T>

const T* Idmap2Service::GetPointer(const OwningPtr<T>& ptr) {

    return std::visit([](auto&& ptr) { return ptr.get(); }, ptr);

}



Status Idmap2Service::getIdmapPath(const std::string& overlay_path,

                                   int32_t user_id ATTRIBUTE_UNUSED, std::string* _aidl_return) {

  assert(_aidl_return);
@@ -224,7 +229,7 @@ idmap2::Result<Idmap2Service::TargetResourceContainerPtr> Idmap2Service::GetTarg 
      if (is_framework ||

        (item.dev == st.st_dev && item.inode == st.st_ino && item.size == st.st_size

          && item.mtime.tv_sec == st.st_mtim.tv_sec && item.mtime.tv_nsec == st.st_mtim.tv_nsec)) {

        return {item.apk.get()};

        return {item.apk};

      }

      container_cache_.erase(cache_it);

    }
@@ -238,14 +243,14 @@ idmap2::Result<Idmap2Service::TargetResourceContainerPtr> Idmap2Service::GetTarg 
    return {std::move(*target)};

  }



  const auto res = target->get();

  auto res = std::shared_ptr(std::move(*target));

  std::lock_guard lock(container_cache_mutex_);

  container_cache_.emplace(target_path, CachedContainer {

    .dev = dev_t(st.st_dev),

    .inode = ino_t(st.st_ino),

    .size = st.st_size,

    .mtime = st.st_mtim,

    .apk = std::move(*target)

    .apk = res

  });

  return {res};

}

cmds/idmap2/idmap2d/Idmap2Service.h

+4 −13
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 { 
    ino_t inode;

    int64_t size;

    struct timespec mtime;

    std::unique_ptr<idmap2::TargetResourceContainer> apk;

    std::shared_ptr<idmap2::TargetResourceContainer> apk;

  };

  std::unordered_map<std::string, CachedContainer> container_cache_;

  std::mutex container_cache_mutex_;
@@ -95,24 +95,15 @@ class Idmap2Service : public BinderService<Idmap2Service>, public BnIdmap2 { 
  std::mutex frro_iter_mutex_;



  template <typename T>

  using MaybeUniquePtr = std::variant<std::unique_ptr<T>, T*>;

  using OwningPtr = std::variant<std::unique_ptr<T>, std::shared_ptr<T>>;



  using TargetResourceContainerPtr = MaybeUniquePtr<idmap2::TargetResourceContainer>;

  using TargetResourceContainerPtr = OwningPtr<idmap2::TargetResourceContainer>;

  idmap2::Result<TargetResourceContainerPtr> GetTargetContainer(const std::string& target_path);



  template <typename T>

  WARN_UNUSED static const T* GetPointer(const MaybeUniquePtr<T>& ptr);

  WARN_UNUSED static const T* GetPointer(const OwningPtr<T>& ptr);

};



template <typename T>

const T* Idmap2Service::GetPointer(const MaybeUniquePtr<T>& ptr) {

  auto u = std::get_if<T*>(&ptr);

  if (u != nullptr) {

    return *u;

  }

  return std::get<std::unique_ptr<T>>(ptr).get();

}



}  // namespace android::os



#endif  // IDMAP2_IDMAP2D_IDMAP2SERVICE_H_

libs/androidfw/Idmap.cpp

+1 −1
Original line number Diff line number Diff line
@@ -121,7 +121,7 @@ OverlayDynamicRefTable::OverlayDynamicRefTable(const Idmap_data_header* data_hea 
                                               uint8_t target_assigned_package_id)

    : data_header_(data_header),

      entries_(entries),

      target_assigned_package_id_(target_assigned_package_id) { };

      target_assigned_package_id_(target_assigned_package_id) {}



status_t OverlayDynamicRefTable::lookupResourceId(uint32_t* resId) const {

  const Idmap_overlay_entry* first_entry = entries_;

libs/androidfw/include/androidfw/Idmap.h

+3 −3
Original line number Diff line number Diff line
@@ -171,14 +171,14 @@ class LoadedIdmap { 
  }



  // Returns a mapping from target resource ids to overlay values.

  const IdmapResMap GetTargetResourcesMap(uint8_t target_assigned_package_id,

  IdmapResMap GetTargetResourcesMap(uint8_t target_assigned_package_id,

                                    const OverlayDynamicRefTable* overlay_ref_table) const {

    return IdmapResMap(data_header_, target_entries_, target_inline_entries_, inline_entry_values_,

                       configurations_, target_assigned_package_id, overlay_ref_table);

  }



  // Returns a dynamic reference table for a loaded overlay package.

  const OverlayDynamicRefTable GetOverlayDynamicRefTable(uint8_t target_assigned_package_id) const {

  OverlayDynamicRefTable GetOverlayDynamicRefTable(uint8_t target_assigned_package_id) const {

    return OverlayDynamicRefTable(data_header_, overlay_entries_, target_assigned_package_id);

  }