diff options
| -rw-r--r-- | services/core/java/com/android/server/job/JobStore.java | 61 | ||||
| -rw-r--r-- | services/tests/servicestests/src/com/android/server/job/JobSetTest.java | 146 |
2 files changed, 187 insertions, 20 deletions
diff --git a/services/core/java/com/android/server/job/JobStore.java b/services/core/java/com/android/server/job/JobStore.java index a24a4ac3823b..778cc2ce8334 100644 --- a/services/core/java/com/android/server/job/JobStore.java +++ b/services/core/java/com/android/server/job/JobStore.java @@ -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) { diff --git a/services/tests/servicestests/src/com/android/server/job/JobSetTest.java b/services/tests/servicestests/src/com/android/server/job/JobSetTest.java new file mode 100644 index 000000000000..83bd9fc2f648 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/job/JobSetTest.java @@ -0,0 +1,146 @@ +/* + * 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); + } +} |