From 061f335321a5cc232f8ca9e0769aa392d8e2131b Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Wed, 5 May 2021 17:50:21 -0700 Subject: [PATCH] Avoid using stale sessions for cleanup alarm. Remove old EJ timing sessions before we schedule the next cleanup alarm. If we don't remove them, then we will continue to use stale sessions as the basis for the next cleanup alarm, which would eventually result in scheduling alarms in the past. Bug: 187351354 Test: atest FrameworksMockingServicesTest:QuotaControllerTest Change-Id: I1b968e0461efe9aad3e785760a3cd2f5d8d4de8b --- .../job/controllers/QuotaController.java | 69 ++++++++++++++----- .../job/controllers/QuotaControllerTest.java | 2 - 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java index 3322841dacb00..75bf8e7cfd229 100644 --- a/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java +++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/QuotaController.java @@ -1493,13 +1493,14 @@ public final class QuotaController extends StateController { /** Schedule a cleanup alarm if necessary and there isn't already one scheduled. */ @VisibleForTesting void maybeScheduleCleanupAlarmLocked() { - if (mNextCleanupTimeElapsed > sElapsedRealtimeClock.millis()) { + final long nowElapsed = sElapsedRealtimeClock.millis(); + if (mNextCleanupTimeElapsed > nowElapsed) { // There's already an alarm scheduled. Just stick with that one. There's no way we'll // end up scheduling an earlier alarm. if (DEBUG) { Slog.v(TAG, "Not scheduling cleanup since there's already one at " - + mNextCleanupTimeElapsed + " (in " + (mNextCleanupTimeElapsed - - sElapsedRealtimeClock.millis()) + "ms)"); + + mNextCleanupTimeElapsed + + " (in " + (mNextCleanupTimeElapsed - nowElapsed) + "ms)"); } return; } @@ -1521,7 +1522,7 @@ public final class QuotaController extends StateController { if (nextCleanupElapsed - mNextCleanupTimeElapsed <= 10 * MINUTE_IN_MILLIS) { // No need to clean up too often. Delay the alarm if the next cleanup would be too soon // after it. - nextCleanupElapsed += 10 * MINUTE_IN_MILLIS; + nextCleanupElapsed = mNextCleanupTimeElapsed + 10 * MINUTE_IN_MILLIS; } mNextCleanupTimeElapsed = nextCleanupElapsed; mAlarmManager.set(AlarmManager.ELAPSED_REALTIME, nextCleanupElapsed, ALARM_TAG_CLEANUP, @@ -2462,30 +2463,60 @@ public final class QuotaController extends StateController { } } - private final class DeleteTimingSessionsFunctor implements Consumer> { - private final Predicate mTooOld = new Predicate() { - public boolean test(TimingSession ts) { - return ts.endTimeElapsed <= sElapsedRealtimeClock.millis() - MAX_PERIOD_MS; - } - }; + private static final class TimingSessionTooOldPredicate implements Predicate { + private long mNowElapsed; + + private void updateNow() { + mNowElapsed = sElapsedRealtimeClock.millis(); + } @Override - public void accept(List sessions) { - if (sessions != null) { - // Remove everything older than MAX_PERIOD_MS time ago. - sessions.removeIf(mTooOld); - } + public boolean test(TimingSession ts) { + return ts.endTimeElapsed <= mNowElapsed - MAX_PERIOD_MS; } } - private final DeleteTimingSessionsFunctor mDeleteOldSessionsFunctor = - new DeleteTimingSessionsFunctor(); + private final TimingSessionTooOldPredicate mTimingSessionTooOld = + new TimingSessionTooOldPredicate(); + + private final Consumer> mDeleteOldSessionsFunctor = sessions -> { + if (sessions != null) { + // Remove everything older than MAX_PERIOD_MS time ago. + sessions.removeIf(mTimingSessionTooOld); + } + }; @VisibleForTesting void deleteObsoleteSessionsLocked() { + mTimingSessionTooOld.updateNow(); + + // Regular sessions mTimingSessions.forEach(mDeleteOldSessionsFunctor); - // Don't delete EJ timing sessions here. They'll be removed in - // getRemainingEJExecutionTimeLocked(). + + // EJ sessions + for (int uIdx = 0; uIdx < mEJTimingSessions.numMaps(); ++uIdx) { + final int userId = mEJTimingSessions.keyAt(uIdx); + for (int pIdx = 0; pIdx < mEJTimingSessions.numElementsForKey(userId); ++pIdx) { + final String packageName = mEJTimingSessions.keyAt(uIdx, pIdx); + final ShrinkableDebits debits = getEJDebitsLocked(userId, packageName); + final List sessions = mEJTimingSessions.get(userId, packageName); + if (sessions == null) { + continue; + } + + while (sessions.size() > 0) { + final TimingSession ts = sessions.get(0); + if (mTimingSessionTooOld.test(ts)) { + // Stale sessions may still be factored into tally. Remove them. + final long duration = ts.endTimeElapsed - ts.startTimeElapsed; + debits.transactLocked(-duration); + sessions.remove(0); + } else { + break; + } + } + } + } } private class QcHandler extends Handler { diff --git a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java index 64687908dc9b9..029930abe6b1d 100644 --- a/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/job/controllers/QuotaControllerTest.java @@ -68,7 +68,6 @@ import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.pm.ApplicationInfo; -import android.content.pm.IPackageManager; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; @@ -475,7 +474,6 @@ public class QuotaControllerTest { expectedRegular.add(thr); expectedRegular.add(two); expectedRegular.add(one); - expectedEJ.add(fiv); // EJ list should be unaffected expectedEJ.add(fou); expectedEJ.add(one); mQuotaController.saveTimingSession(0, "com.android.test", fiv, false);