Fix memory leak.

BatteryController only starts tracking jobs that have a power constraint
and stops tracking them if it had previously marked them as being
tracked by the controller. Adding all top-started jobs to its list of
top-started jobs meant that the top-started list would slowly accumulate
jobs that didn't have a power constraint since the controller would
never attempt to remove them from the list.

Bug: 239797636
Test: atest CtsJobSchedulerTestCases
Change-Id: I084586c13772c6942ba841647458f72a941cc087
This commit is contained in:
Kweku Adams
2022-07-21 21:53:23 +00:00
parent 84ce5affa7
commit 7be14890c5
2 changed files with 82 additions and 0 deletions

View File

@@ -34,6 +34,7 @@ import android.util.Slog;
import android.util.proto.ProtoOutputStream;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.server.JobSchedulerBackgroundThread;
import com.android.server.LocalServices;
import com.android.server.job.JobSchedulerService;
@@ -100,6 +101,10 @@ public final class BatteryController extends RestrictingController {
@Override
@GuardedBy("mLock")
public void prepareForExecutionLocked(JobStatus jobStatus) {
if (!jobStatus.hasPowerConstraint()) {
// Ignore all jobs the controller wouldn't be tracking.
return;
}
if (DEBUG) {
Slog.d(TAG, "Prepping for " + jobStatus.toShortString());
}
@@ -259,6 +264,16 @@ public final class BatteryController extends RestrictingController {
}
}
@VisibleForTesting
ArraySet<JobStatus> getTrackedJobs() {
return mTrackedTasks;
}
@VisibleForTesting
ArraySet<JobStatus> getTopStartedJobs() {
return mTopStartedJobs;
}
@Override
public void dumpControllerStateLocked(IndentingPrintWriter pw,
Predicate<JobStatus> predicate) {

View File

@@ -312,4 +312,71 @@ public class BatteryControllerTest {
assertFalse(jobLateFg.isConstraintSatisfied(JobStatus.CONSTRAINT_CHARGING));
assertFalse(jobLateFgLow.isConstraintSatisfied(JobStatus.CONSTRAINT_CHARGING));
}
@Test
public void testControllerOnlyTracksPowerJobs() {
JobStatus batteryJob = createJobStatus("testControllerOnlyTracksPowerJobs",
SOURCE_PACKAGE, mSourceUid,
createBaseJobInfoBuilder(1).setRequiresBatteryNotLow(true).build());
JobStatus chargingJob = createJobStatus("testControllerOnlyTracksPowerJobs",
SOURCE_PACKAGE, mSourceUid,
createBaseJobInfoBuilder(2).setRequiresCharging(true).build());
JobStatus bothPowerJob = createJobStatus("testControllerOnlyTracksPowerJobs",
SOURCE_PACKAGE, mSourceUid,
createBaseJobInfoBuilder(3)
.setRequiresCharging(true)
.setRequiresBatteryNotLow(true)
.build());
JobStatus unrelatedJob = createJobStatus("testControllerOnlyTracksPowerJobs",
SOURCE_PACKAGE, mSourceUid, createBaseJobInfoBuilder(4).build());
// Follow the lifecycle of tracking
// Start tracking
trackJobs(batteryJob, chargingJob, bothPowerJob, unrelatedJob);
final ArraySet<JobStatus> trackedJobs = mBatteryController.getTrackedJobs();
final ArraySet<JobStatus> topStartedJobs = mBatteryController.getTopStartedJobs();
assertTrue(trackedJobs.contains(batteryJob));
assertTrue(trackedJobs.contains(chargingJob));
assertTrue(trackedJobs.contains(bothPowerJob));
assertFalse(trackedJobs.contains(unrelatedJob));
assertFalse(topStartedJobs.contains(batteryJob));
assertFalse(topStartedJobs.contains(chargingJob));
assertFalse(topStartedJobs.contains(bothPowerJob));
assertFalse(topStartedJobs.contains(unrelatedJob));
// Procstate change shouldn't affect anything
setUidBias(mSourceUid, JobInfo.BIAS_TOP_APP);
assertTrue(trackedJobs.contains(batteryJob));
assertTrue(trackedJobs.contains(chargingJob));
assertTrue(trackedJobs.contains(bothPowerJob));
assertFalse(trackedJobs.contains(unrelatedJob));
assertFalse(topStartedJobs.contains(batteryJob));
assertFalse(topStartedJobs.contains(chargingJob));
assertFalse(topStartedJobs.contains(bothPowerJob));
assertFalse(topStartedJobs.contains(unrelatedJob));
// Job starts running
mBatteryController.prepareForExecutionLocked(batteryJob);
mBatteryController.prepareForExecutionLocked(chargingJob);
mBatteryController.prepareForExecutionLocked(bothPowerJob);
mBatteryController.prepareForExecutionLocked(unrelatedJob);
assertTrue(topStartedJobs.contains(batteryJob));
assertTrue(topStartedJobs.contains(chargingJob));
assertTrue(topStartedJobs.contains(bothPowerJob));
assertFalse(topStartedJobs.contains(unrelatedJob));
// Job cleanup
mBatteryController.maybeStopTrackingJobLocked(batteryJob, null, false);
mBatteryController.maybeStopTrackingJobLocked(chargingJob, null, false);
mBatteryController.maybeStopTrackingJobLocked(bothPowerJob, null, false);
mBatteryController.maybeStopTrackingJobLocked(unrelatedJob, null, false);
assertFalse(trackedJobs.contains(batteryJob));
assertFalse(trackedJobs.contains(chargingJob));
assertFalse(trackedJobs.contains(bothPowerJob));
assertFalse(trackedJobs.contains(unrelatedJob));
assertFalse(topStartedJobs.contains(batteryJob));
assertFalse(topStartedJobs.contains(chargingJob));
assertFalse(topStartedJobs.contains(bothPowerJob));
assertFalse(topStartedJobs.contains(unrelatedJob));
}
}