Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a57583df authored by Kweku Adams's avatar Kweku Adams
Browse files

Add app-level timeout & ANR limits.

If an app's jobs keep timing out, then the app potentially has a bug.
Timeouts resulted in backoff of the individual job that timed out,
but didn't affect other jobs. This meant that apps could bypass backoff
by scheduling one-off jobs instead of rescheduling using the return
parameter to onStopJob. Now, JobScheduler will track the number of job
timeouts across all of the app's jobs and reduce the app's max job time
if it exceeds a threshold. If the total number of timeouts exceeds
another limit, JobScheduler will consider the app buggy and
attempt to put the app in the RESTRICTED bucket.

Similarly, if an app keeps ANRing from JobScheduler's perspective, it
will consider the app buggy and attempt to put it in the RESTRICTED
bucket.

When an unexempted app is considered buggy, don't treat any of its jobs
as in the ACTIVE unless the app is current active.

Also, reduce the delay for forcing an app in the RESTRICTED bucket
(eg. when the app is buggy) to 1 hour. The normal bucket timeout
continues to be 8 days.

Bug: 284080732
Test: atest AppStandbyControllerTests
Test: atest FrameworksMockingServicesTests:JobSchedulerServiceTest
Test: atest FrameworksMockingServicesTests:JobStatusTest
Test: atest frameworks/base/services/tests/mockingservicestests/src/com/android/server/job
Test: atest frameworks/base/services/tests/servicestests/src/com/android/server/job
Change-Id: I8eb333a922658ba6137012dffd7a143cd990376d
parent 09c9df3f
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
@@ -59,6 +59,10 @@ public interface JobSchedulerInternal {
     */
    void reportAppUsage(String packageName, int userId);

    /** @return {@code true} if the app is considered buggy from JobScheduler's perspective. */
    boolean isAppConsideredBuggy(int callingUserId, @NonNull String callingPackageName,
            int timeoutBlameUserId, @NonNull String timeoutBlamePackageName);

    /**
     * @return {@code true} if the given notification is associated with any user-initiated jobs.
     */
+299 −18

File changed.

Preview size limit exceeded, changes collapsed.

+72 −3
Original line number Diff line number Diff line
@@ -1024,13 +1024,77 @@ public final class JobStatus {
        return UserHandle.getUserId(callingUid);
    }

    private boolean shouldBlameSourceForTimeout() {
        // If the system scheduled the job on behalf of an app, assume the app is the one
        // doing the work and blame the app directly. This is the case with things like
        // syncs via SyncManager.
        // If the system didn't schedule the job on behalf of an app, then
        // blame the app doing the actual work. Proxied jobs are a little tricky.
        // Proxied jobs scheduled by built-in system apps like DownloadManager may be fine
        // and we could consider exempting those jobs. For example, in DownloadManager's
        // case, all it does is download files and the code is vetted. A timeout likely
        // means it's downloading a large file, which isn't an error. For now, DownloadManager
        // is an exempted app, so this shouldn't be an issue.
        // However, proxied jobs coming from other system apps (such as those that can
        // be updated separately from an OTA) may not be fine and we would want to apply
        // this policy to those jobs/apps.
        // TODO(284512488): consider exempting DownloadManager or other system apps
        return UserHandle.isCore(callingUid);
    }

    /**
     * Returns the package name that should most likely be blamed for the job timing out.
     */
    public String getTimeoutBlamePackageName() {
        if (shouldBlameSourceForTimeout()) {
            return sourcePackageName;
        }
        return getServiceComponent().getPackageName();
    }

    /**
     * Returns the UID that should most likely be blamed for the job timing out.
     */
    public int getTimeoutBlameUid() {
        if (shouldBlameSourceForTimeout()) {
            return sourceUid;
        }
        return callingUid;
    }

    /**
     * Returns the userId that should most likely be blamed for the job timing out.
     */
    public int getTimeoutBlameUserId() {
        if (shouldBlameSourceForTimeout()) {
            return sourceUserId;
        }
        return UserHandle.getUserId(callingUid);
    }

    /**
     * Returns an appropriate standby bucket for the job, taking into account any standby
     * exemptions.
     */
    public int getEffectiveStandbyBucket() {
        final JobSchedulerInternal jsi = LocalServices.getService(JobSchedulerInternal.class);
        final boolean isBuggy = jsi.isAppConsideredBuggy(
                getUserId(), getServiceComponent().getPackageName(),
                getTimeoutBlameUserId(), getTimeoutBlamePackageName());

        final int actualBucket = getStandbyBucket();
        if (actualBucket == EXEMPTED_INDEX) {
            // EXEMPTED apps always have their jobs exempted, even if they're buggy, because the
            // user has explicitly told the system to avoid restricting the app for power reasons.
            if (isBuggy) {
                final String pkg;
                if (getServiceComponent().getPackageName().equals(sourcePackageName)) {
                    pkg = sourcePackageName;
                } else {
                    pkg = getServiceComponent().getPackageName() + "/" + sourcePackageName;
                }
                Slog.w(TAG, "Exempted app " + pkg + " considered buggy");
            }
            return actualBucket;
        }
        if (uidActive || getJob().isExemptedFromAppStandby()) {
@@ -1038,13 +1102,18 @@ public final class JobStatus {
            // like other ACTIVE apps.
            return ACTIVE_INDEX;
        }
        // If the app is considered buggy, but hasn't yet been put in the RESTRICTED bucket
        // (potentially because it's used frequently by the user), limit its effective bucket
        // so that it doesn't get to run as much as a normal ACTIVE app.
        final int highestBucket = isBuggy ? WORKING_INDEX : ACTIVE_INDEX;
        if (actualBucket != RESTRICTED_INDEX && actualBucket != NEVER_INDEX
                && mHasMediaBackupExemption) {
            // Cap it at WORKING_INDEX as media back up jobs are important to the user, and the
            // Treat it as if it's at least WORKING_INDEX since media backup jobs are important
            // to the user, and the
            // source package may not have been used directly in a while.
            return Math.min(WORKING_INDEX, actualBucket);
            return Math.max(highestBucket, Math.min(WORKING_INDEX, actualBucket));
        }
        return actualBucket;
        return Math.max(highestBucket, actualBucket);
    }

    /** Returns the real standby bucket of the job. */
+8 −0
Original line number Diff line number Diff line
@@ -773,6 +773,14 @@ public final class QuotaController extends StateController {
            // If quota is currently "free", then the job can run for the full amount of time,
            // regardless of bucket (hence using charging instead of isQuotaFreeLocked()).
            if (mService.isBatteryCharging()
                    // The top and foreground cases here were added because apps in those states
                    // aren't really restricted and the work could be something the user is
                    // waiting for. Now that user-initiated jobs are a defined concept, we may
                    // not need these exemptions as much. However, UIJs are currently limited
                    // (as of UDC) to data transfer work. There may be other work that could
                    // rely on this exception. Once we add more UIJ types, we can re-evaluate
                    // the need for these exceptions.
                    // TODO: re-evaluate the need for these exceptions
                    || mTopAppCache.get(jobStatus.getSourceUid())
                    || isTopStartedJobLocked(jobStatus)
                    || isUidInForeground(jobStatus.getSourceUid())) {
+1 −1
Original line number Diff line number Diff line
@@ -3025,7 +3025,7 @@ public class AppStandbyController
        public static final long DEFAULT_INITIAL_FOREGROUND_SERVICE_START_TIMEOUT =
                COMPRESS_TIME ? ONE_MINUTE : 30 * ONE_MINUTE;
        public static final long DEFAULT_AUTO_RESTRICTED_BUCKET_DELAY_MS =
                COMPRESS_TIME ? ONE_MINUTE : ONE_DAY;
                COMPRESS_TIME ? ONE_MINUTE : ONE_HOUR;
        public static final boolean DEFAULT_CROSS_PROFILE_APPS_SHARE_STANDBY_BUCKETS = true;
        public static final long DEFAULT_BROADCAST_RESPONSE_WINDOW_DURATION_MS =
                2 * ONE_MINUTE;
Loading