Merge "Fixed removeJobsOfNonUsers to keep data consistent"

This commit is contained in:
TreeHugger Robot
2018-01-30 03:13:25 +00:00
committed by Android (Google) Code Review
2 changed files with 187 additions and 20 deletions

View File

@@ -62,6 +62,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
@@ -85,7 +86,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
@@ -1000,10 +1001,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>>();
@@ -1046,7 +1048,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) {
@@ -1075,27 +1083,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) {

View File

@@ -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);
}
}