diff options
author | 2024-08-05 11:59:27 -0700 | |
---|---|---|
committer | 2024-08-05 13:10:47 -0700 | |
commit | 3d13a4f71b75a4b9ab87156613cbd11ebbf9ad94 (patch) | |
tree | e01f71b841d63769cb86ca4cf72f176db60b986c | |
parent | 04f157c77b8adfc854135b9e788d9f75e1d3b87b (diff) |
[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
-rw-r--r-- | cmds/idmap2/idmap2d/Idmap2Service.cpp | 11 | ||||
-rw-r--r-- | cmds/idmap2/idmap2d/Idmap2Service.h | 17 | ||||
-rw-r--r-- | libs/androidfw/Idmap.cpp | 2 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/Idmap.h | 6 |
4 files changed, 16 insertions, 20 deletions
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp index f264125cfde5..6902d6db6751 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.cpp +++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp @@ -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}; } diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h index a69fa6119974..272ec6be3bac 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.h +++ b/cmds/idmap2/idmap2d/Idmap2Service.h @@ -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_ diff --git a/libs/androidfw/Idmap.cpp b/libs/androidfw/Idmap.cpp index 982419059ead..f066e4620675 100644 --- a/libs/androidfw/Idmap.cpp +++ b/libs/androidfw/Idmap.cpp @@ -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_; diff --git a/libs/androidfw/include/androidfw/Idmap.h b/libs/androidfw/include/androidfw/Idmap.h index c32a38ee9ec2..64b1f0c6ed03 100644 --- a/libs/androidfw/include/androidfw/Idmap.h +++ b/libs/androidfw/include/androidfw/Idmap.h @@ -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, - const OverlayDynamicRefTable* overlay_ref_table) const { + 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); } |