Merge "Finer-grained locking for size operations."
diff --git a/cmds/installd/CacheItem.cpp b/cmds/installd/CacheItem.cpp
index 6caf149..0349b0f 100644
--- a/cmds/installd/CacheItem.cpp
+++ b/cmds/installd/CacheItem.cpp
@@ -72,7 +72,7 @@
FTS *fts;
FTSENT *p;
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
PLOG(WARNING) << "Failed to fts_open " << path;
return -1;
}
diff --git a/cmds/installd/CacheTracker.cpp b/cmds/installd/CacheTracker.cpp
index 9377836..fada699 100644
--- a/cmds/installd/CacheTracker.cpp
+++ b/cmds/installd/CacheTracker.cpp
@@ -83,7 +83,7 @@
FTS *fts;
FTSENT *p;
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
PLOG(WARNING) << "Failed to fts_open " << path;
return;
}
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 71595f0..e486589 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -203,14 +203,20 @@
out << "installd is happy!" << endl;
- out << endl << "Devices with quota support:" << endl;
- for (const auto& n : mQuotaDevices) {
- out << " " << n.first << " = " << n.second << endl;
+ {
+ std::lock_guard<std::recursive_mutex> lock(mQuotaDevicesLock);
+ out << endl << "Devices with quota support:" << endl;
+ for (const auto& n : mQuotaDevices) {
+ out << " " << n.first << " = " << n.second << endl;
+ }
}
- out << endl << "Per-UID cache quotas:" << endl;
- for (const auto& n : mCacheQuotas) {
- out << " " << n.first << " = " << n.second << endl;
+ {
+ std::lock_guard<std::recursive_mutex> lock(mCacheQuotasLock);
+ out << endl << "Per-UID cache quotas:" << endl;
+ for (const auto& n : mCacheQuotas) {
+ out << " " << n.first << " = " << n.second << endl;
+ }
}
out << endl;
@@ -753,9 +759,6 @@
CHECK_ARGUMENT_UUID(uuid);
std::lock_guard<std::recursive_mutex> lock(mLock);
- // TODO: remove this once framework is more robust
- invalidateMounts();
-
const char* uuid_ = uuid ? uuid->c_str() : nullptr;
auto data_path = create_data_path(uuid_);
auto device = findQuotaDeviceForUuid(uuid);
@@ -787,7 +790,7 @@
(char*) create_data_user_de_path(uuid_, user).c_str(),
nullptr
};
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
return error("Failed to fts_open");
}
while ((p = fts_read(fts)) != NULL) {
@@ -800,7 +803,10 @@
auto tracker = std::shared_ptr<CacheTracker>(new CacheTracker(
multiuser_get_user_id(uid), multiuser_get_app_id(uid), device));
tracker->addDataPath(p->fts_path);
- tracker->cacheQuota = mCacheQuotas[uid];
+ {
+ std::lock_guard<std::recursive_mutex> lock(mCacheQuotasLock);
+ tracker->cacheQuota = mCacheQuotas[uid];
+ }
if (tracker->cacheQuota == 0) {
LOG(WARNING) << "UID " << uid << " has no cache quota; assuming 64MB";
tracker->cacheQuota = 67108864;
@@ -1120,7 +1126,7 @@
FTS *fts;
FTSENT *p;
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
PLOG(ERROR) << "Failed to fts_open " << path;
return;
}
@@ -1159,7 +1165,8 @@
for (auto packageName : packageNames) {
CHECK_ARGUMENT_PACKAGE_NAME(packageName);
}
- std::lock_guard<std::recursive_mutex> lock(mLock);
+ // NOTE: Locking is relaxed on this method, since it's limited to
+ // read-only measurements without mutation.
// When modifying this logic, always verify using tests:
// runtest -x frameworks/base/services/tests/servicestests/src/com/android/server/pm/InstallerTest.java -m testGetAppSize
@@ -1274,7 +1281,8 @@
std::vector<int64_t>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
CHECK_ARGUMENT_UUID(uuid);
- std::lock_guard<std::recursive_mutex> lock(mLock);
+ // NOTE: Locking is relaxed on this method, since it's limited to
+ // read-only measurements without mutation.
// When modifying this logic, always verify using tests:
// runtest -x frameworks/base/services/tests/servicestests/src/com/android/server/pm/InstallerTest.java -m testGetUserSize
@@ -1420,7 +1428,8 @@
int32_t userId, int32_t flags, std::vector<int64_t>* _aidl_return) {
ENFORCE_UID(AID_SYSTEM);
CHECK_ARGUMENT_UUID(uuid);
- std::lock_guard<std::recursive_mutex> lock(mLock);
+ // NOTE: Locking is relaxed on this method, since it's limited to
+ // read-only measurements without mutation.
// When modifying this logic, always verify using tests:
// runtest -x frameworks/base/services/tests/servicestests/src/com/android/server/pm/InstallerTest.java -m testGetExternalSize
@@ -1486,7 +1495,7 @@
FTSENT *p;
auto path = create_data_media_path(uuid_, userId);
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
return error("Failed to fts_open " + path);
}
while ((p = fts_read(fts)) != NULL) {
@@ -1533,7 +1542,7 @@
int32_t userId, int32_t appId, int64_t cacheQuota) {
ENFORCE_UID(AID_SYSTEM);
CHECK_ARGUMENT_UUID(uuid);
- std::lock_guard<std::recursive_mutex> lock(mLock);
+ std::lock_guard<std::recursive_mutex> lock(mCacheQuotasLock);
int32_t uid = multiuser_get_uid(userId, appId);
mCacheQuotas[uid] = cacheQuota;
@@ -1986,7 +1995,7 @@
binder::Status InstalldNativeService::invalidateMounts() {
ENFORCE_UID(AID_SYSTEM);
- std::lock_guard<std::recursive_mutex> lock(mLock);
+ std::lock_guard<std::recursive_mutex> lock(mQuotaDevicesLock);
mQuotaDevices.clear();
@@ -2017,6 +2026,7 @@
std::string InstalldNativeService::findQuotaDeviceForUuid(
const std::unique_ptr<std::string>& uuid) {
+ std::lock_guard<std::recursive_mutex> lock(mQuotaDevicesLock);
auto path = create_data_path(uuid ? uuid->c_str() : nullptr);
return mQuotaDevices[path];
}
diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h
index feb2219..7ad8687 100644
--- a/cmds/installd/InstalldNativeService.h
+++ b/cmds/installd/InstalldNativeService.h
@@ -117,6 +117,9 @@
private:
std::recursive_mutex mLock;
+ std::recursive_mutex mQuotaDevicesLock;
+ std::recursive_mutex mCacheQuotasLock;
+
/* Map from mount point to underlying device node */
std::unordered_map<std::string, std::string> mQuotaDevices;
/* Map from UID to cache quota size */
diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp
index e5b243a..5a08c32 100644
--- a/cmds/installd/utils.cpp
+++ b/cmds/installd/utils.cpp
@@ -284,7 +284,7 @@
FTSENT *p;
int64_t matchedSize = 0;
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
if (errno != ENOENT) {
PLOG(ERROR) << "Failed to fts_open " << path;
}
@@ -1440,7 +1440,7 @@
FTS *fts;
FTSENT *p;
char *argv[] = { (char*) path.c_str(), nullptr };
- if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_XDEV, NULL))) {
+ if (!(fts = fts_open(argv, FTS_PHYSICAL | FTS_NOCHDIR | FTS_XDEV, NULL))) {
PLOG(ERROR) << "Failed to fts_open " << path;
return -1;
}