Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b26786d6 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Finer-grained locking for size operations. 

Disk space measurements are read-only and don't perform mutations,
so other installd operations shouldn't block them.

If there's an ongoing parallel operation (such as a dexopt) that
could race and skew the results, that's no different than an actively
running app changing it's disk usage during the measurement.

This change also allows measurements to happen in parallel, so we can
no longer rely on getcwd() being stable, which means all fts(3) users
now need to use FTS_NOCHDIR.

Bug: 36032444, 35706513
Test: runtest -x frameworks/base/services/tests/servicestests/src/com/android/server/pm/InstallerTest.java
Change-Id: I67d303d3ecce148052d41444cef67381b1d34ab0
parent 3e3474ea

cmds/installd/CacheItem.cpp

+1 −1
Original line number Diff line number Diff line
@@ -72,7 +72,7 @@ int CacheItem::purge() { 
        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;

        }

cmds/installd/CacheTracker.cpp

+1 −1
Original line number Diff line number Diff line
@@ -83,7 +83,7 @@ void CacheTracker::loadItemsFrom(const std::string& path) { 
    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;

    }

cmds/installd/InstalldNativeService.cpp

+28 −18
Original line number Diff line number Diff line
@@ -203,15 +203,21 @@ status_t InstalldNativeService::dump(int fd, const Vector<String16> & /* args */ 


    out << "installd is happy!" << 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;

        }

    }



    {

        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;

    out.flush();
@@ -755,9 +761,6 @@ binder::Status InstalldNativeService::freeCache(const std::unique_ptr<std::strin 
    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);
@@ -789,7 +792,7 @@ binder::Status InstalldNativeService::freeCache(const std::unique_ptr<std::strin 
                    (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) {
@@ -802,7 +805,10 @@ binder::Status InstalldNativeService::freeCache(const std::unique_ptr<std::strin 
                        auto tracker = std::shared_ptr<CacheTracker>(new CacheTracker(

                                multiuser_get_user_id(uid), multiuser_get_app_id(uid), device));

                        tracker->addDataPath(p->fts_path);

                        {

                            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;
@@ -1122,7 +1128,7 @@ static void collectManualExternalStatsForUser(const std::string& path, struct st 
    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;

    }
@@ -1161,7 +1167,8 @@ binder::Status InstalldNativeService::getAppSize(const std::unique_ptr<std::stri 
    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
@@ -1276,7 +1283,8 @@ binder::Status InstalldNativeService::getUserSize(const std::unique_ptr<std::str 
        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
@@ -1422,7 +1430,8 @@ binder::Status InstalldNativeService::getExternalSize(const std::unique_ptr<std: 
        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
@@ -1488,7 +1497,7 @@ binder::Status InstalldNativeService::getExternalSize(const std::unique_ptr<std: 
        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) {
@@ -1535,7 +1544,7 @@ binder::Status InstalldNativeService::setAppQuota(const std::unique_ptr<std::str 
        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;
@@ -1988,7 +1997,7 @@ binder::Status InstalldNativeService::reconcileSecondaryDexFile( 


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();
@@ -2019,6 +2028,7 @@ binder::Status InstalldNativeService::invalidateMounts() { 


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];

}

cmds/installd/InstalldNativeService.h

+3 −0
Original line number Diff line number Diff line
@@ -117,6 +117,9 @@ public: 
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 */

cmds/installd/utils.cpp

+2 −2
Original line number Diff line number Diff line
@@ -284,7 +284,7 @@ int calculate_tree_size(const std::string& path, int64_t* size, 
    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 @@ int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t ta 
    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;

    }