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

Commit 1bd36ec1 authored by Kweku Adams's avatar Kweku Adams
Browse files

Only clear UID-based structures when UID is fully gone. 

Check to make sure that there are no remaining packages in a UID before
we remove data structures based on UID-only keys.

Bug: 184098842
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job
Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job
Test: atest CtsJobSchedulerTestCases
Change-Id: Ia38130c1b6fcbf6c3851fe6890726bcab71e15b1
parent 293c2796

apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java

+58 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import static android.content.pm.PackageManager.COMPONENT_ENABLED_STATE_DISABLED 
import static android.text.format.DateUtils.MINUTE_IN_MILLIS;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.annotation.UserIdInt;

import android.app.Activity;

import android.app.ActivityManager;
@@ -67,15 +68,18 @@ import android.os.WorkSource; 
import android.provider.DeviceConfig;

import android.text.format.DateUtils;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.IndentingPrintWriter;

import android.util.Log;

import android.util.Slog;

import android.util.SparseArray;

import android.util.SparseIntArray;

import android.util.SparseSetArray;

import android.util.TimeUtils;

import android.util.proto.ProtoOutputStream;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.app.IBatteryStats;

import com.android.internal.util.ArrayUtils;
@@ -316,6 +320,9 @@ public class JobSchedulerService extends com.android.server.SystemService 
     */

    final ArrayMap<String, Boolean> mDebuggableApps = new ArrayMap<>();



    /** Cached mapping of UIDs (for all users) to a list of packages in the UID. */

    private final SparseSetArray<String> mUidToPackageCache = new SparseSetArray<>();



    /**

     * Named indices into standby bucket arrays, for clarity in referring to

     * specific buckets' bookkeeping.
@@ -785,12 +792,20 @@ public class JobSchedulerService extends com.android.server.SystemService 
                } else {

                    Slog.w(TAG, "PACKAGE_CHANGED for " + pkgName + " / uid " + pkgUid);

                }

            } else if (Intent.ACTION_PACKAGE_ADDED.equals(action)) {

                if (!intent.getBooleanExtra(Intent.EXTRA_REPLACING, false)) {

                    final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);

                    synchronized (mLock) {

                        mUidToPackageCache.remove(uid);

                    }

                }

            } else if (Intent.ACTION_PACKAGE_FULLY_REMOVED.equals(action)) {

                int uidRemoved = intent.getIntExtra(Intent.EXTRA_UID, -1);

                if (DEBUG) {

                    Slog.d(TAG, "Removing jobs for uid: " + uidRemoved);

                }

                synchronized (mLock) {

                    mUidToPackageCache.remove(uidRemoved);

                    // There's no guarantee that the process has been stopped by the time we

                    // get here, but since this is generally a user-initiated action, it should

                    // be fine to just put USER instead of UNINSTALL or DISABLED.
@@ -815,6 +830,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
                    Slog.d(TAG, "Removing jobs for user: " + userId);

                }

                synchronized (mLock) {

                    mUidToPackageCache.clear();

                    cancelJobsForUserLocked(userId);

                    for (int c = 0; c < mControllers.size(); ++c) {

                        mControllers.get(c).onUserRemovedLocked(userId);
@@ -904,6 +920,27 @@ public class JobSchedulerService extends com.android.server.SystemService 
        return WorkSource.isChainedBatteryAttributionEnabled(getContext());

    }



    @Nullable

    @GuardedBy("mLock")

    public ArraySet<String> getPackagesForUidLocked(final int uid) {

        ArraySet<String> packages = mUidToPackageCache.get(uid);

        if (packages == null) {

            try {

                String[] pkgs = AppGlobals.getPackageManager()

                        .getPackagesForUid(uid);

                if (pkgs != null) {

                    for (String pkg : pkgs) {

                        mUidToPackageCache.add(uid, pkg);

                    }

                    packages = mUidToPackageCache.get(uid);

                }

            } catch (RemoteException e) {

                // Shouldn't happen.

            }

        }

        return packages;

    }



    @Override

    public void onUserUnlocked(@NonNull TargetUser user) {

        synchronized (mLock) {
@@ -1484,6 +1521,7 @@ public class JobSchedulerService extends com.android.server.SystemService 
            // Register br for package removals and user removals.

            final IntentFilter filter = new IntentFilter();

            filter.addAction(Intent.ACTION_PACKAGE_FULLY_REMOVED);

            filter.addAction(Intent.ACTION_PACKAGE_ADDED);

            filter.addAction(Intent.ACTION_PACKAGE_CHANGED);

            filter.addAction(Intent.ACTION_PACKAGE_RESTARTED);

            filter.addAction(Intent.ACTION_QUERY_PACKAGE_RESTART);
@@ -3285,6 +3323,26 @@ public class JobSchedulerService extends com.android.server.SystemService 
                pw.decreaseIndent();

            }



            boolean uidMapPrinted = false;

            for (int i = 0; i < mUidToPackageCache.size(); ++i) {

                final int uid = mUidToPackageCache.keyAt(i);

                if (filterUid != -1 && filterUid != uid) {

                    continue;

                }

                if (!uidMapPrinted) {

                    uidMapPrinted = true;

                    pw.println();

                    pw.println("Cached UID->package map:");

                    pw.increaseIndent();

                }

                pw.print(uid);

                pw.print(": ");

                pw.println(mUidToPackageCache.get(uid));

            }

            if (uidMapPrinted) {

                pw.decreaseIndent();

            }



            boolean backingPrinted = false;

            for (int i = 0; i < mBackingUpUids.size(); i++) {

                int uid = mBackingUpUids.keyAt(i);

apex/jobscheduler/service/java/com/android/server/job/controllers/ConnectivityController.java

+9 −5
Original line number Diff line number Diff line
@@ -452,12 +452,16 @@ public final class ConnectivityController extends RestrictingController implemen 
    @GuardedBy("mLock")

    @Override

    public void onAppRemovedLocked(String pkgName, int uid) {

        if (mService.getPackagesForUidLocked(uid) == null) {

            // All packages in the UID have been removed. It's safe to remove things based on

            // UID alone.

            mTrackedJobs.delete(uid);

            UidStats uidStats = mUidStats.removeReturnOld(uid);

            unregisterDefaultNetworkCallbackLocked(uid, sElapsedRealtimeClock.millis());

            mSortedStats.remove(uidStats);

            registerPendingUidCallbacksLocked();

        }

    }



    @GuardedBy("mLock")

    @Override

apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java

+15 −80
Original line number Diff line number Diff line
@@ -340,9 +340,6 @@ public final class QuotaController extends StateController { 
    /** List of UIDs currently in the foreground. */

    private final SparseBooleanArray mForegroundUids = new SparseBooleanArray();



    /** Cached mapping of UIDs (for all users) to a list of packages in the UID. */

    private final SparseSetArray<String> mUidToPackageCache = new SparseSetArray<>();



    /**

     * List of jobs that started while the UID was in the TOP state. There will be no more than

     * 16 ({@link JobSchedulerService#MAX_JOB_CONTEXTS_COUNT}) running at once, so an ArraySet is
@@ -449,22 +446,6 @@ public final class QuotaController extends StateController { 
        }

    }



    private final BroadcastReceiver mPackageAddedReceiver = new BroadcastReceiver() {

        @Override

        public void onReceive(Context context, Intent intent) {

            if (intent == null) {

                return;

            }

            if (intent.getBooleanExtra(Intent.EXTRA_REPLACING, false)) {

                return;

            }

            final int uid = intent.getIntExtra(Intent.EXTRA_UID, -1);

            synchronized (mLock) {

                mUidToPackageCache.remove(uid);

            }

        }

    };



    /**

     * The rolling window size for each standby bucket. Within each window, an app will have 10

     * minutes to run its jobs.
@@ -611,9 +592,6 @@ public final class QuotaController extends StateController { 
        mBackgroundJobsController = backgroundJobsController;

        mConnectivityController = connectivityController;



        final IntentFilter filter = new IntentFilter(Intent.ACTION_PACKAGE_ADDED);

        mContext.registerReceiverAsUser(mPackageAddedReceiver, UserHandle.ALL, filter, null, null);



        // Set up the app standby bucketing tracker

        AppStandbyInternal appStandby = LocalServices.getService(AppStandbyInternal.class);

        appStandby.addListener(new StandbyTracker());
@@ -730,14 +708,16 @@ public final class QuotaController extends StateController { 
            return;

        }

        clearAppStatsLocked(UserHandle.getUserId(uid), packageName);

        if (mService.getPackagesForUidLocked(uid) == null) {

            // All packages in the UID have been removed. It's safe to remove things based on

            // UID alone.

            mForegroundUids.delete(uid);

        mUidToPackageCache.remove(uid);

            mTempAllowlistCache.delete(uid);

            mTempAllowlistGraceCache.delete(uid);

            mTopAppCache.delete(uid);

        mTopAppTrackers.delete(uid);

            mTopAppGraceCache.delete(uid);

        }

    }



    @Override

    public void onUserAddedLocked(int userId) {
@@ -754,8 +734,8 @@ public final class QuotaController extends StateController { 
        mInQuotaAlarmListener.removeAlarmsLocked(userId);

        mExecutionStatsCache.delete(userId);

        mEJStats.delete(userId);

        mUidToPackageCache.clear();

        mSystemInstallers.remove(userId);

        mTopAppTrackers.delete(userId);

    }



    /** Drop all historical stats and stop tracking any active sessions for the specified app. */
@@ -780,6 +760,7 @@ public final class QuotaController extends StateController { 
        mInQuotaAlarmListener.removeAlarmLocked(userId, packageName);

        mExecutionStatsCache.delete(userId, packageName);

        mEJStats.delete(userId, packageName);

        mTopAppTrackers.delete(userId, packageName);

    }



    private void cacheInstallerPackagesLocked(int userId) {
@@ -2450,7 +2431,7 @@ public final class QuotaController extends StateController { 
            synchronized (mLock) {

                final long nowElapsed = sElapsedRealtimeClock.millis();

                mTempAllowlistCache.put(uid, true);

                final ArraySet<String> packages = getPackagesForUidLocked(uid);

                final ArraySet<String> packages = mService.getPackagesForUidLocked(uid);

                if (packages != null) {

                    final int userId = UserHandle.getUserId(uid);

                    for (int i = packages.size() - 1; i >= 0; --i) {
@@ -2505,26 +2486,6 @@ public final class QuotaController extends StateController { 
        // getRemainingEJExecutionTimeLocked().

    }



    @Nullable

    private ArraySet<String> getPackagesForUidLocked(final int uid) {

        ArraySet<String> packages = mUidToPackageCache.get(uid);

        if (packages == null) {

            try {

                String[] pkgs = AppGlobals.getPackageManager()

                        .getPackagesForUid(uid);

                if (pkgs != null) {

                    for (String pkg : pkgs) {

                        mUidToPackageCache.add(uid, pkg);

                    }

                    packages = mUidToPackageCache.get(uid);

                }

            } catch (RemoteException e) {

                // Shouldn't happen.

            }

        }

        return packages;

    }



    private class QcHandler extends Handler {



        QcHandler(Looper looper) {
@@ -2655,7 +2616,8 @@ public final class QuotaController extends StateController { 
                            // Update Timers first.

                            if (mPkgTimers.indexOfKey(userId) >= 0

                                    || mEJPkgTimers.indexOfKey(userId) >= 0) {

                                final ArraySet<String> packages = getPackagesForUidLocked(uid);

                                final ArraySet<String> packages =

                                        mService.getPackagesForUidLocked(uid);

                                if (packages != null) {

                                    for (int i = packages.size() - 1; i >= 0; --i) {

                                        Timer t = mEJPkgTimers.get(userId, packages.valueAt(i));
@@ -2740,7 +2702,7 @@ public final class QuotaController extends StateController { 
                            }

                            mTempAllowlistGraceCache.delete(uid);

                            mTopAppGraceCache.delete(uid);

                            final ArraySet<String> packages = getPackagesForUidLocked(uid);

                            final ArraySet<String> packages = mService.getPackagesForUidLocked(uid);

                            if (packages != null) {

                                final int userId = UserHandle.getUserId(uid);

                                for (int i = packages.size() - 1; i >= 0; --i) {
@@ -4109,17 +4071,6 @@ public final class QuotaController extends StateController { 
        pw.println(mTempAllowlistGraceCache.toString());

        pw.println();



        pw.println("Cached UID->package map:");

        pw.increaseIndent();

        for (int i = 0; i < mUidToPackageCache.size(); ++i) {

            final int uid = mUidToPackageCache.keyAt(i);

            pw.print(uid);

            pw.print(": ");

            pw.println(mUidToPackageCache.get(uid));

        }

        pw.decreaseIndent();

        pw.println();



        pw.println("Special apps:");

        pw.increaseIndent();

        pw.print("System installers", mSystemInstallers.toString());
@@ -4278,22 +4229,6 @@ public final class QuotaController extends StateController { 
                    mForegroundUids.keyAt(i));

        }



        for (int i = 0; i < mUidToPackageCache.size(); ++i) {

            final long upToken = proto.start(

                    StateControllerProto.QuotaController.UID_TO_PACKAGE_CACHE);



            final int uid = mUidToPackageCache.keyAt(i);

            ArraySet<String> packages = mUidToPackageCache.get(uid);



            proto.write(StateControllerProto.QuotaController.UidPackageMapping.UID, uid);

            for (int j = 0; j < packages.size(); ++j) {

                proto.write(StateControllerProto.QuotaController.UidPackageMapping.PACKAGE_NAMES,

                        packages.valueAt(j));

            }



            proto.end(upToken);

        }



        mTrackedJobs.forEach((jobs) -> {

            for (int j = 0; j < jobs.size(); j++) {

                final JobStatus js = jobs.valueAt(j);

services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java

+8 −6
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ import android.os.RemoteException; 
import android.os.SystemClock;

import android.platform.test.annotations.LargeTest;

import android.provider.DeviceConfig;

import android.util.ArraySet;

import android.util.SparseBooleanArray;



import androidx.test.runner.AndroidJUnit4;
@@ -254,6 +255,9 @@ public class QuotaControllerTest { 
                    any());

            mUidObserver = uidObserverCaptor.getValue();

            mSourceUid = AppGlobals.getPackageManager().getPackageUid(SOURCE_PACKAGE, 0, 0);

            // Need to do this since we're using a mock JS and not a real object.

            doReturn(new ArraySet<>(new String[]{SOURCE_PACKAGE}))

                    .when(mJobSchedulerService).getPackagesForUidLocked(mSourceUid);

        } catch (RemoteException e) {

            fail(e.getMessage());

        }
@@ -1803,12 +1807,10 @@ public class QuotaControllerTest { 
        setStandbyBucket(ACTIVE_INDEX, fgStateChanger);

        setProcessState(ActivityManager.PROCESS_STATE_BACKUP, fgChangerUid);



        IPackageManager packageManager = AppGlobals.getPackageManager();

        spyOn(packageManager);

        doReturn(new String[]{unaffectedPkgName})

                .when(packageManager).getPackagesForUid(unaffectedUid);

        doReturn(new String[]{fgChangerPkgName})

                .when(packageManager).getPackagesForUid(fgChangerUid);

        doReturn(new ArraySet<>(new String[]{unaffectedPkgName}))

                .when(mJobSchedulerService).getPackagesForUidLocked(unaffectedUid);

        doReturn(new ArraySet<>(new String[]{fgChangerPkgName}))

                .when(mJobSchedulerService).getPackagesForUidLocked(fgChangerUid);



        synchronized (mQuotaController.mLock) {

            mQuotaController.maybeStartTrackingJobLocked(unaffected, null);