summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Yurii Zubrytskyi <zyy@google.com> 2024-08-05 11:59:27 -0700
committer Yurii Zubrytskyi <zyy@google.com> 2024-08-05 13:10:47 -0700
commit3d13a4f71b75a4b9ab87156613cbd11ebbf9ad94 (patch)
treee01f71b841d63769cb86ca4cf72f176db60b986c
parent04f157c77b8adfc854135b9e788d9f75e1d3b87b (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.cpp11
-rw-r--r--cmds/idmap2/idmap2d/Idmap2Service.h17
-rw-r--r--libs/androidfw/Idmap.cpp2
-rw-r--r--libs/androidfw/include/androidfw/Idmap.h6
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);
}