Merge "Fix async behavior and address possible race conditions." into sc-dev

This commit is contained in:
TreeHugger Robot
2021-03-30 18:45:50 +00:00
committed by Android (Google) Code Review
2 changed files with 65 additions and 51 deletions

View File

@@ -746,11 +746,14 @@ public class JobSchedulerService extends com.android.server.SystemService
Slog.d(TAG, "Removing jobs for package " + pkgName
+ " in user " + userId);
}
// By the time we get here, the process should have already
// been stopped, so the app wouldn't get the stop reason,
// so just put USER instead of UNINSTALL or DISABLED.
cancelJobsForPackageAndUid(pkgName, pkgUid,
JobParameters.STOP_REASON_USER, "app disabled");
synchronized (mLock) {
// By the time we get here, the process should have
// already been stopped, so the app wouldn't get the
// stop reason, so just put USER instead of UNINSTALL
// or DISABLED.
cancelJobsForPackageAndUidLocked(pkgName, pkgUid,
JobParameters.STOP_REASON_USER, "app disabled");
}
}
} catch (RemoteException|IllegalArgumentException e) {
/*
@@ -791,9 +794,9 @@ public class JobSchedulerService extends com.android.server.SystemService
// By the time we get here, the process should have already
// been stopped, so the app wouldn't get the stop reason,
// so just put USER instead of UNINSTALL or DISABLED.
cancelJobsForPackageAndUid(pkgName, uidRemoved,
JobParameters.STOP_REASON_USER, "app uninstalled");
synchronized (mLock) {
cancelJobsForPackageAndUidLocked(pkgName, uidRemoved,
JobParameters.STOP_REASON_USER, "app uninstalled");
for (int c = 0; c < mControllers.size(); ++c) {
mControllers.get(c).onAppRemovedLocked(pkgName, pkgUid);
}
@@ -812,8 +815,8 @@ public class JobSchedulerService extends com.android.server.SystemService
if (DEBUG) {
Slog.d(TAG, "Removing jobs for user: " + userId);
}
cancelJobsForUser(userId);
synchronized (mLock) {
cancelJobsForUserLocked(userId);
for (int c = 0; c < mControllers.size(); ++c) {
mControllers.get(c).onUserRemovedLocked(userId);
}
@@ -844,8 +847,10 @@ public class JobSchedulerService extends com.android.server.SystemService
if (DEBUG) {
Slog.d(TAG, "Removing jobs for pkg " + pkgName + " at uid " + pkgUid);
}
cancelJobsForPackageAndUid(pkgName, pkgUid,
JobParameters.STOP_REASON_USER, "app force stopped");
synchronized (mLock) {
cancelJobsForPackageAndUidLocked(pkgName, pkgUid,
JobParameters.STOP_REASON_USER, "app force stopped");
}
}
}
}
@@ -1106,16 +1111,14 @@ public class JobSchedulerService extends com.android.server.SystemService
}
}
void cancelJobsForUser(int userHandle) {
synchronized (mLock) {
final List<JobStatus> jobsForUser = mJobs.getJobsByUser(userHandle);
for (int i=0; i<jobsForUser.size(); i++) {
JobStatus toRemove = jobsForUser.get(i);
// By the time we get here, the process should have already been stopped, so the
// app wouldn't get the stop reason, so just put USER instead of UNINSTALL.
cancelJobImplLocked(toRemove, null, JobParameters.STOP_REASON_USER,
"user removed");
}
private void cancelJobsForUserLocked(int userHandle) {
final List<JobStatus> jobsForUser = mJobs.getJobsByUser(userHandle);
for (int i = 0; i < jobsForUser.size(); i++) {
JobStatus toRemove = jobsForUser.get(i);
// By the time we get here, the process should have already been stopped, so the
// app wouldn't get the stop reason, so just put USER instead of UNINSTALL.
cancelJobImplLocked(toRemove, null, JobParameters.STOP_REASON_USER,
"user removed");
}
}
@@ -1126,19 +1129,17 @@ public class JobSchedulerService extends com.android.server.SystemService
}
}
void cancelJobsForPackageAndUid(String pkgName, int uid, @JobParameters.StopReason int reason,
String debugReason) {
private void cancelJobsForPackageAndUidLocked(String pkgName, int uid,
@JobParameters.StopReason int reason, String debugReason) {
if ("android".equals(pkgName)) {
Slog.wtfStack(TAG, "Can't cancel all jobs for system package");
return;
}
synchronized (mLock) {
final List<JobStatus> jobsForUid = mJobs.getJobsByUid(uid);
for (int i = jobsForUid.size() - 1; i >= 0; i--) {
final JobStatus job = jobsForUid.get(i);
if (job.getSourcePackageName().equals(pkgName)) {
cancelJobImplLocked(job, null, reason, debugReason);
}
final List<JobStatus> jobsForUid = mJobs.getJobsByUid(uid);
for (int i = jobsForUid.size() - 1; i >= 0; i--) {
final JobStatus job = jobsForUid.get(i);
if (job.getSourcePackageName().equals(pkgName)) {
cancelJobImplLocked(job, null, reason, debugReason);
}
}
}

View File

@@ -226,9 +226,22 @@ public final class ConnectivityController extends RestrictingController implemen
jobs.remove(jobStatus);
}
UidStats us = mUidStats.get(jobStatus.getSourceUid());
us.numReadyWithConnectivity--;
if (jobStatus.madeActive != 0) {
us.numRunning--;
if (us == null) {
// This shouldn't be happening. We create a UidStats object for the app when the
// first job is scheduled in maybeStartTrackingJobLocked() and only ever drop the
// object if the app is uninstalled or the user is removed. That means that if we
// end up in this situation, onAppRemovedLocked() or onUserRemovedLocked() was
// called before maybeStopTrackingJobLocked(), which is the reverse order of what
// JobSchedulerService does (JSS calls maybeStopTrackingJobLocked() for all jobs
// before calling onAppRemovedLocked() or onUserRemovedLocked()).
Slog.wtfStack(TAG,
"UidStats was null after job for " + jobStatus.getSourcePackageName()
+ " was registered");
} else {
us.numReadyWithConnectivity--;
if (jobStatus.madeActive != 0) {
us.numRunning--;
}
}
maybeRevokeStandbyExceptionLocked(jobStatus);
maybeAdjustRegisteredCallbacksLocked();
@@ -773,22 +786,22 @@ public final class ConnectivityController extends RestrictingController implemen
* @param filterNetwork only update jobs that would use this
* {@link Network}, or {@code null} to update all tracked jobs.
*/
private void updateTrackedJobs(int filterUid, Network filterNetwork) {
synchronized (mLock) {
boolean changed = false;
if (filterUid == -1) {
for (int i = mTrackedJobs.size() - 1; i >= 0; i--) {
changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i), filterNetwork);
}
} else {
changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid), filterNetwork);
}
if (changed) {
mStateChangedListener.onControllerStateChanged();
@GuardedBy("mLock")
private void updateTrackedJobsLocked(int filterUid, Network filterNetwork) {
boolean changed = false;
if (filterUid == -1) {
for (int i = mTrackedJobs.size() - 1; i >= 0; i--) {
changed |= updateTrackedJobsLocked(mTrackedJobs.valueAt(i), filterNetwork);
}
} else {
changed = updateTrackedJobsLocked(mTrackedJobs.get(filterUid), filterNetwork);
}
if (changed) {
mStateChangedListener.onControllerStateChanged();
}
}
@GuardedBy("mLock")
private boolean updateTrackedJobsLocked(ArraySet<JobStatus> jobs, Network filterNetwork) {
if (jobs == null || jobs.size() == 0) {
return false;
@@ -875,9 +888,9 @@ public final class ConnectivityController extends RestrictingController implemen
}
synchronized (mLock) {
mAvailableNetworks.put(network, capabilities);
updateTrackedJobsLocked(-1, network);
maybeAdjustRegisteredCallbacksLocked();
}
updateTrackedJobs(-1, network);
maybeAdjustRegisteredCallbacksLocked();
}
@Override
@@ -893,9 +906,9 @@ public final class ConnectivityController extends RestrictingController implemen
callback.mDefaultNetwork = null;
}
}
updateTrackedJobsLocked(-1, network);
maybeAdjustRegisteredCallbacksLocked();
}
updateTrackedJobs(-1, network);
maybeAdjustRegisteredCallbacksLocked();
}
};
@@ -909,7 +922,7 @@ public final class ConnectivityController extends RestrictingController implemen
synchronized (mLock) {
switch (msg.what) {
case MSG_REEVALUATE_JOBS:
updateTrackedJobs(-1, null);
updateTrackedJobsLocked(-1, null);
break;
}
}
@@ -949,8 +962,8 @@ public final class ConnectivityController extends RestrictingController implemen
synchronized (mLock) {
mDefaultNetwork = network;
mBlocked = blocked;
updateTrackedJobsLocked(mUid, network);
}
updateTrackedJobs(mUid, network);
}
// Network transitions have some complicated behavior that JS doesn't handle very well.
@@ -993,8 +1006,8 @@ public final class ConnectivityController extends RestrictingController implemen
if (Objects.equals(mDefaultNetwork, network)) {
mDefaultNetwork = null;
}
updateTrackedJobsLocked(mUid, network);
}
updateTrackedJobs(mUid, network);
}
private void dumpLocked(IndentingPrintWriter pw) {