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

Commit 7b21bb5b authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Fixed removeJobsOfNonUsers to keep data consistent 

removeJobsOfNonUsers() was removing different jobs from the maps mJobs and
mJobsPerSourceUid. Also modified it to remove jobs from both the caller
and the source user id. Added a test for JobStore.JobSet

Test: atest com.android.server.job.JobSetTest

Fixes: 72459151
Change-Id: I95021b6370b68ad9e8a94dd4ebcad28905ee3c8b
parent 618e8602

services/core/java/com/android/server/job/JobStore.java

+41 −20
Original line number Diff line number Diff line
@@ -61,6 +61,7 @@ import java.nio.charset.StandardCharsets; 
import java.util.ArrayList;

import java.util.List;

import java.util.Set;

import java.util.function.Predicate;



/**

 * Maintains the master list of jobs that the job scheduler is tracking. These jobs are compared by
@@ -84,7 +85,7 @@ public final class JobStore { 
    private static final int MAX_OPS_BEFORE_WRITE = 1;



    final Object mLock;

    final JobSet mJobSet; // per-caller-uid tracking

    final JobSet mJobSet; // per-caller-uid and per-source-uid tracking

    final Context mContext;



    // Bookkeeping around incorrect boot-time system clock
@@ -998,10 +999,11 @@ public final class JobStore { 
    }



    static final class JobSet {

        // Key is the getUid() originator of the jobs in each sheaf

        private SparseArray<ArraySet<JobStatus>> mJobs;

        // Same data but with the key as getSourceUid() of the jobs in each sheaf

        private SparseArray<ArraySet<JobStatus>> mJobsPerSourceUid;

        @VisibleForTesting // Key is the getUid() originator of the jobs in each sheaf

        final SparseArray<ArraySet<JobStatus>> mJobs;



        @VisibleForTesting // Same data but with the key as getSourceUid() of the jobs in each sheaf

        final SparseArray<ArraySet<JobStatus>> mJobsPerSourceUid;



        public JobSet() {

            mJobs = new SparseArray<ArraySet<JobStatus>>();
@@ -1044,7 +1046,13 @@ public final class JobStore { 
                jobsForSourceUid = new ArraySet<>();

                mJobsPerSourceUid.put(sourceUid, jobsForSourceUid);

            }

            return jobs.add(job) && jobsForSourceUid.add(job);

            final boolean added = jobs.add(job);

            final boolean addedInSource = jobsForSourceUid.add(job);

            if (added != addedInSource) {

                Slog.wtf(TAG, "mJobs and mJobsPerSourceUid mismatch; caller= " + added

                        + " source= " + addedInSource);

            }

            return added || addedInSource;

        }



        public boolean remove(JobStatus job) {
@@ -1073,27 +1081,40 @@ public final class JobStore { 


        /**

         * Removes the jobs of all users not specified by the whitelist of user ids.

         * The jobs scheduled by non existent users will not be removed if they were

         * This will remove jobs scheduled *by* non-existent users as well as jobs scheduled *for*

         * non-existent users

         */

        public void removeJobsOfNonUsers(int[] whitelist) {

            for (int jobSetIndex = mJobsPerSourceUid.size() - 1; jobSetIndex >= 0; jobSetIndex--) {

                final int jobUserId = UserHandle.getUserId(mJobsPerSourceUid.keyAt(jobSetIndex));

                if (!ArrayUtils.contains(whitelist, jobUserId)) {

                    mJobsPerSourceUid.removeAt(jobSetIndex);

                }

        public void removeJobsOfNonUsers(final int[] whitelist) {

            final Predicate<JobStatus> noSourceUser =

                    job -> !ArrayUtils.contains(whitelist, job.getSourceUserId());

            final Predicate<JobStatus> noCallingUser =

                    job -> !ArrayUtils.contains(whitelist, job.getUserId());

            removeAll(noSourceUser.or(noCallingUser));

        }



        private void removeAll(Predicate<JobStatus> predicate) {

            for (int jobSetIndex = mJobs.size() - 1; jobSetIndex >= 0; jobSetIndex--) {

                final ArraySet<JobStatus> jobsForUid = mJobs.valueAt(jobSetIndex);

                for (int jobIndex = jobsForUid.size() - 1; jobIndex >= 0; jobIndex--) {

                    final int jobUserId = jobsForUid.valueAt(jobIndex).getUserId();

                    if (!ArrayUtils.contains(whitelist, jobUserId)) {

                        jobsForUid.removeAt(jobIndex);

                final ArraySet<JobStatus> jobs = mJobs.valueAt(jobSetIndex);

                for (int jobIndex = jobs.size() - 1; jobIndex >= 0; jobIndex--) {

                    if (predicate.test(jobs.valueAt(jobIndex))) {

                        jobs.removeAt(jobIndex);

                    }

                }

                if (jobsForUid.size() == 0) {

                if (jobs.size() == 0) {

                    mJobs.removeAt(jobSetIndex);

                }

            }

            for (int jobSetIndex = mJobsPerSourceUid.size() - 1; jobSetIndex >= 0; jobSetIndex--) {

                final ArraySet<JobStatus> jobs = mJobsPerSourceUid.valueAt(jobSetIndex);

                for (int jobIndex = jobs.size() - 1; jobIndex >= 0; jobIndex--) {

                    if (predicate.test(jobs.valueAt(jobIndex))) {

                        jobs.removeAt(jobIndex);

                    }

                }

                if (jobs.size() == 0) {

                    mJobsPerSourceUid.removeAt(jobSetIndex);

                }

            }

        }



        public boolean contains(JobStatus job) {

services/tests/servicestests/src/com/android/server/job/JobSetTest.java

0 → 100644
+146 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2018 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License

 */



package com.android.server.job;



import static org.junit.Assert.assertTrue;

import static org.junit.Assert.fail;

import static org.junit.Assume.assumeFalse;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;



import android.app.job.JobInfo;

import android.content.ComponentName;

import android.content.Context;

import android.content.pm.PackageManagerInternal;

import android.os.Build;

import android.os.UserHandle;

import android.support.test.InstrumentationRegistry;

import android.support.test.filters.SmallTest;

import android.support.test.runner.AndroidJUnit4;

import android.util.ArraySet;

import android.util.Log;

import android.util.SparseArray;



import com.android.server.LocalServices;

import com.android.server.job.controllers.JobStatus;



import org.junit.After;

import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;



@RunWith(AndroidJUnit4.class)

@SmallTest

public class JobSetTest {

    private static final String TAG = JobSetTest.class.getSimpleName();

    private static final int SECONDARY_USER_ID_1 = 12;

    private static final int SECONDARY_USER_ID_2 = 13;



    private Context mContext;

    private ComponentName mComponent;

    private JobStore.JobSet mJobSet;



    @Before

    public void setUp() throws Exception {

        mContext = InstrumentationRegistry.getTargetContext();

        mComponent = new ComponentName(mContext, JobStoreTest.class);

        mJobSet = new JobStore.JobSet();

        final PackageManagerInternal pm = mock(PackageManagerInternal.class);

        when(pm.getPackageTargetSdkVersion(anyString()))

                .thenReturn(Build.VERSION_CODES.CUR_DEVELOPMENT);

        LocalServices.addService(PackageManagerInternal.class, pm);

        assumeFalse("Test cannot run in user " + mContext.getUserId(),

                mContext.getUserId() == SECONDARY_USER_ID_1

                        || mContext.getUserId() == SECONDARY_USER_ID_2);

    }



    private JobStatus getJobStatusWithCallinUid(int jobId, int callingUid) {

        final JobInfo jobInfo = new JobInfo.Builder(jobId, mComponent)

                .setPeriodic(10)

                .setRequiresCharging(true)

                .build();

        return JobStatus.createFromJobInfo(jobInfo, callingUid, mContext.getPackageName(),

                mContext.getUserId(), "Test");

    }



    @Test

    public void testBothMapsHaveSameJobs() {

        final int callingUid1 = UserHandle.getUid(SECONDARY_USER_ID_1, 1);

        final int callingUid2 = UserHandle.getUid(SECONDARY_USER_ID_2, 1);

        final JobStatus testJob1 = getJobStatusWithCallinUid(1, callingUid1);

        final JobStatus testJob2 = getJobStatusWithCallinUid(2, callingUid2);

        mJobSet.add(testJob1);

        mJobSet.add(testJob2);

        for (int i = 11; i <= 20; i++) {

            mJobSet.add(getJobStatusWithCallinUid(i, (i%2 == 0) ? callingUid2 : callingUid1));

        }

        assertHaveSameJobs(mJobSet.mJobsPerSourceUid, mJobSet.mJobs);

        mJobSet.remove(testJob1);

        mJobSet.remove(testJob2);

        assertHaveSameJobs(mJobSet.mJobsPerSourceUid, mJobSet.mJobs);

        mJobSet.removeJobsOfNonUsers(new int[] {mContext.getUserId(), SECONDARY_USER_ID_1});

        assertHaveSameJobs(mJobSet.mJobsPerSourceUid, mJobSet.mJobs);

        mJobSet.removeJobsOfNonUsers(new int[] {mContext.getUserId()});

        assertTrue("mJobs should be empty", mJobSet.mJobs.size() == 0);

        assertTrue("mJobsPerSourceUid should be empty", mJobSet.mJobsPerSourceUid.size() == 0);

    }



    private static void assertHaveSameJobs(SparseArray<ArraySet<JobStatus>> map1,

            SparseArray<ArraySet<JobStatus>> map2) {

        final ArraySet<JobStatus> set1 = new ArraySet<>();

        final ArraySet<JobStatus> set2 = new ArraySet<>();

        int size1 = 0;

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

            final ArraySet<JobStatus> jobs = map1.valueAt(i);

            if (jobs == null) return;

            size1 += jobs.size();

            set1.addAll(jobs);

        }

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

            final ArraySet<JobStatus> jobs = map2.valueAt(i);

            if (jobs == null) return;

            size1 -= jobs.size();

            set2.addAll(jobs);

        }

        if (size1 != 0 || !set1.equals(set2)) {

            dump("map1", map1);

            dump("map2", map2);

            fail("Both maps have different sets of jobs");

        }

    }



    private static void dump(String prefix, SparseArray<ArraySet<JobStatus>> jobMap) {

        final StringBuilder str = new StringBuilder();

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

            final ArraySet<JobStatus> jobs = jobMap.valueAt(i);

            if (jobs == null) return;

            str.append("[Key: " + jobMap.keyAt(i) + ", Value: {");

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

                final JobStatus job = jobs.valueAt(j);

                str.append("(s=" + job.getSourceUid() + ", c=" + job.getUid() + "), ");

            }

            str.append("}], ");

        }

        Log.d(TAG, prefix + ": " + str.toString());

    }



    @After

    public void tearDown() throws Exception {

        LocalServices.removeServiceForTest(PackageManagerInternal.class);

    }

}