Fix ownership of objects returned by VintfObject::Get*
VintfObject::Get* is not thread-safe; the raw pointers
returned to clients can be invalidated by another thread
that calls the same VintfObject::Get* with skipCache = true.
Fix this by sharing ownership to callers as well.
VintfObject::Get* now returns a shared_ptr, and clients
can destroy the shared_ptr once they finish reading it.
Test: libvintf_test
Test: vintf_object_test
Test: `adb shell vintf` shows compatible
Bug: 65166415
Change-Id: I931cf0864a8dba3d83fe41f715b94e1a21b266eb
diff --git a/VintfObject.cpp b/VintfObject.cpp
index f785e6b..0f590b6 100644
--- a/VintfObject.cpp
+++ b/VintfObject.cpp
@@ -28,20 +28,14 @@
namespace vintf {
template <typename T>
-struct LockedUniquePtr {
- std::unique_ptr<T> object;
+struct LockedSharedPtr {
+ std::shared_ptr<T> object;
std::mutex mutex;
};
-static LockedUniquePtr<HalManifest> gDeviceManifest;
-static LockedUniquePtr<HalManifest> gFrameworkManifest;
-static LockedUniquePtr<CompatibilityMatrix> gDeviceMatrix;
-static LockedUniquePtr<CompatibilityMatrix> gFrameworkMatrix;
-static LockedUniquePtr<RuntimeInfo> gDeviceRuntimeInfo;
-
template <typename T, typename F>
-static const T *Get(
- LockedUniquePtr<T> *ptr,
+static std::shared_ptr<const T> Get(
+ LockedSharedPtr<T> *ptr,
bool skipCache,
const F &fetchAllInformation) {
std::unique_lock<std::mutex> _lock(ptr->mutex);
@@ -51,18 +45,20 @@
ptr->object = nullptr; // frees the old object
}
}
- return ptr->object.get();
+ return ptr->object;
}
// static
-const HalManifest *VintfObject::GetDeviceHalManifest(bool skipCache) {
+std::shared_ptr<const HalManifest> VintfObject::GetDeviceHalManifest(bool skipCache) {
+ static LockedSharedPtr<HalManifest> gDeviceManifest;
return Get(&gDeviceManifest, skipCache,
std::bind(&HalManifest::fetchAllInformation, std::placeholders::_1,
"/vendor/manifest.xml"));
}
// static
-const HalManifest *VintfObject::GetFrameworkHalManifest(bool skipCache) {
+std::shared_ptr<const HalManifest> VintfObject::GetFrameworkHalManifest(bool skipCache) {
+ static LockedSharedPtr<HalManifest> gFrameworkManifest;
return Get(&gFrameworkManifest, skipCache,
std::bind(&HalManifest::fetchAllInformation, std::placeholders::_1,
"/system/manifest.xml"));
@@ -70,21 +66,24 @@
// static
-const CompatibilityMatrix *VintfObject::GetDeviceCompatibilityMatrix(bool skipCache) {
+std::shared_ptr<const CompatibilityMatrix> VintfObject::GetDeviceCompatibilityMatrix(bool skipCache) {
+ static LockedSharedPtr<CompatibilityMatrix> gDeviceMatrix;
return Get(&gDeviceMatrix, skipCache,
std::bind(&CompatibilityMatrix::fetchAllInformation, std::placeholders::_1,
"/vendor/compatibility_matrix.xml"));
}
// static
-const CompatibilityMatrix *VintfObject::GetFrameworkCompatibilityMatrix(bool skipCache) {
+std::shared_ptr<const CompatibilityMatrix> VintfObject::GetFrameworkCompatibilityMatrix(bool skipCache) {
+ static LockedSharedPtr<CompatibilityMatrix> gFrameworkMatrix;
return Get(&gFrameworkMatrix, skipCache,
std::bind(&CompatibilityMatrix::fetchAllInformation, std::placeholders::_1,
"/system/compatibility_matrix.xml"));
}
// static
-const RuntimeInfo *VintfObject::GetRuntimeInfo(bool skipCache) {
+std::shared_ptr<const RuntimeInfo> VintfObject::GetRuntimeInfo(bool skipCache) {
+ static LockedSharedPtr<RuntimeInfo> gDeviceRuntimeInfo;
return Get(&gDeviceRuntimeInfo, skipCache,
std::bind(&RuntimeInfo::fetchAllInformation, std::placeholders::_1));
}
@@ -110,8 +109,8 @@
template<typename T>
static ParseStatus tryParse(const std::string &xml, const XmlConverter<T> &parse,
- std::unique_ptr<T> *fwk, std::unique_ptr<T> *dev) {
- std::unique_ptr<T> ret = std::make_unique<T>();
+ std::shared_ptr<T> *fwk, std::shared_ptr<T> *dev) {
+ std::shared_ptr<T> ret = std::make_shared<T>();
if (!parse(ret.get(), xml)) {
return ParseStatus::PARSE_ERROR;
}
@@ -130,9 +129,9 @@
}
template<typename T, typename GetFunction>
-static status_t getMissing(const T *pkg, bool mount,
+static status_t getMissing(const std::shared_ptr<T>& pkg, bool mount,
std::function<status_t(void)> mountFunction,
- const T **updated,
+ std::shared_ptr<const T>* updated,
GetFunction getFunction) {
if (pkg != nullptr) {
*updated = pkg;
@@ -152,8 +151,8 @@
struct PackageInfo {
struct Pair {
- std::unique_ptr<HalManifest> manifest;
- std::unique_ptr<CompatibilityMatrix> matrix;
+ std::shared_ptr<HalManifest> manifest;
+ std::shared_ptr<CompatibilityMatrix> matrix;
};
Pair dev;
Pair fwk;
@@ -161,12 +160,12 @@
struct UpdatedInfo {
struct Pair {
- const HalManifest *manifest;
- const CompatibilityMatrix *matrix;
+ std::shared_ptr<const HalManifest> manifest;
+ std::shared_ptr<const CompatibilityMatrix> matrix;
};
Pair dev;
Pair fwk;
- const RuntimeInfo *runtimeInfo;
+ std::shared_ptr<const RuntimeInfo> runtimeInfo;
};
// Checks given compatibility info against info on the device. If no
@@ -207,23 +206,23 @@
auto mountSystem = [&mounter] { return mounter.mountSystem(); };
auto mountVendor = [&mounter] { return mounter.mountVendor(); };
if ((status = getMissing(
- pkg.fwk.manifest.get(), mount, mountSystem, &updated.fwk.manifest,
+ pkg.fwk.manifest, mount, mountSystem, &updated.fwk.manifest,
std::bind(VintfObject::GetFrameworkHalManifest, true /* skipCache */))) != OK) {
return status;
}
if ((status = getMissing(
- pkg.dev.manifest.get(), mount, mountVendor, &updated.dev.manifest,
+ pkg.dev.manifest, mount, mountVendor, &updated.dev.manifest,
std::bind(VintfObject::GetDeviceHalManifest, true /* skipCache */))) != OK) {
return status;
}
if ((status = getMissing(
- pkg.fwk.matrix.get(), mount, mountSystem, &updated.fwk.matrix,
+ pkg.fwk.matrix, mount, mountSystem, &updated.fwk.matrix,
std::bind(VintfObject::GetFrameworkCompatibilityMatrix, true /* skipCache */))) !=
OK) {
return status;
}
if ((status = getMissing(
- pkg.dev.matrix.get(), mount, mountVendor, &updated.dev.matrix,
+ pkg.dev.matrix, mount, mountVendor, &updated.dev.matrix,
std::bind(VintfObject::GetDeviceCompatibilityMatrix, true /* skipCache */))) != OK) {
return status;
}