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

Commit 703cf10c authored by Hui Yu's avatar Hui Yu
Browse files

Synchronize on the SparseArray object in PendingTempAllowlists.

The field mPendingTempAllowlist in PendingTempAllowlists is a
SparseArray.

In ActivityManagerService, the lock protection for PendingTempAllowlists
object is:
 @CompositeRWLock({"this", "mProcLock"})
    final PendingTempAllowlists mPendingTempAllowlist

Which means the mPendingTempAllowlist object can be read when either
"this" lock or "mProcLock" lock is held. But the read-operation of
SparseArray such as indexOfKey() and size() etc, actually mutate the
SparseArray by calling SparseArray.gc(). This makes @CompositeRWLock not
to be compatible with SparseArray.

Since we can not make SparseArray thread-safe, also we want to maintain
the semantic of @CompositeRWLock, we can make
PendingTempAllowlists thread-safe at least.

Bug: 193788840
Test: Regression test.
Change-Id: Ie1c239ad27d1fd6b76676951b470605513848b20
parent da94cf36
Loading
Loading
Loading
Loading
+8 −6
Original line number Diff line number Diff line
@@ -5823,7 +5823,7 @@ public class ActivityManagerService extends IActivityManager.Stub
        return Arrays.binarySearch(allowlist, appId) >= 0
                || Arrays.binarySearch(mDeviceIdleTempAllowlist, appId) >= 0
                || mPendingTempAllowlist.indexOfKey(uid) >= 0;
                || mPendingTempAllowlist.get(uid) != null;
    }
    /**
@@ -15122,6 +15122,7 @@ public class ActivityManagerService extends IActivityManager.Stub
        // First copy out the pending changes...  we need to leave them in the map for now,
        // in case someone needs to check what is coming up while we don't have the lock held.
        synchronized (this) {
            synchronized (mProcLock) {
                N = mPendingTempAllowlist.size();
                list = new PendingTempAllowlist[N];
@@ -15129,6 +15130,7 @@ public class ActivityManagerService extends IActivityManager.Stub
                    list[i] = mPendingTempAllowlist.valueAt(i);
                }
            }
        }
        // Now safely dispatch changes to device idle controller.  Skip this if we're early
        // in boot and the controller hasn't yet been brought online:  we do not apply
+22 −7
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package com.android.server.am;

import static android.os.Process.INVALID_UID;

import android.util.SparseArray;

/** Allowlists of uids to temporarily bypass Power Save mode. */
@@ -31,29 +33,42 @@ final class PendingTempAllowlists {
    }

    void put(int uid, ActivityManagerService.PendingTempAllowlist value) {
        synchronized (mPendingTempAllowlist) {
            mPendingTempAllowlist.put(uid, value);
        }
        mService.mAtmInternal.onUidAddedToPendingTempAllowlist(uid, value.tag);
    }

    void removeAt(int index) {
        final int uid = mPendingTempAllowlist.keyAt(index);
        int uid = INVALID_UID;
        synchronized (mPendingTempAllowlist) {
            uid = mPendingTempAllowlist.keyAt(index);
            mPendingTempAllowlist.removeAt(index);
        }
        mService.mAtmInternal.onUidRemovedFromPendingTempAllowlist(uid);
    }

    ActivityManagerService.PendingTempAllowlist get(int uid) {
        synchronized (mPendingTempAllowlist) {
            return mPendingTempAllowlist.get(uid);
        }
    }

    int size() {
        synchronized (mPendingTempAllowlist) {
            return mPendingTempAllowlist.size();
        }
    }

    ActivityManagerService.PendingTempAllowlist valueAt(int index) {
        synchronized (mPendingTempAllowlist) {
            return mPendingTempAllowlist.valueAt(index);
        }
    }

    int indexOfKey(int key) {
        synchronized (mPendingTempAllowlist) {
            return mPendingTempAllowlist.indexOfKey(key);
        }
    }
}