[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
diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp
index f264125..6902d6d 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.cpp
+++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp
@@ -78,6 +78,11 @@
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 @@
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 @@
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 a69fa61..272ec6b 100644
--- a/cmds/idmap2/idmap2d/Idmap2Service.h
+++ b/cmds/idmap2/idmap2d/Idmap2Service.h
@@ -85,7 +85,7 @@
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 @@
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_