Merge "Hold wakelock earlier."

2021-06-14 20:37:46 +00:00
parent 5bddf9974d c55c7389d4
commit c026755287
5 changed files with 61 additions and 82 deletions
--- a/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobConcurrencyManager.java
@@ -804,28 +804,42 @@ class JobConcurrencyManager {
            @WorkType final int workType) {
        final List<StateController> controllers = mService.mControllers;
        final int numControllers = controllers.size();
-        for (int ic = 0; ic < numControllers; ic++) {
-            controllers.get(ic).prepareForExecutionLocked(jobStatus);
-        }
-        final PackageStats packageStats =
-                getPkgStatsLocked(jobStatus.getSourceUserId(), jobStatus.getSourcePackageName());
-        packageStats.adjustStagedCount(false, jobStatus.shouldTreatAsExpeditedJob());
-        if (!worker.executeRunnableJob(jobStatus, workType)) {
-            Slog.e(TAG, "Error executing " + jobStatus);
-            mWorkCountTracker.onStagedJobFailed(workType);
+        final PowerManager.WakeLock wl =
+                mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, jobStatus.getTag());
+        wl.setWorkSource(mService.deriveWorkSource(
+                jobStatus.getSourceUid(), jobStatus.getSourcePackageName()));
+        wl.setReferenceCounted(false);
+        // Since the quota controller will start counting from the time prepareForExecutionLocked()
+        // is called, hold a wakelock to make sure the CPU doesn't suspend between that call and
+        // when the service actually starts.
+        wl.acquire();
+        try {
            for (int ic = 0; ic < numControllers; ic++) {
-                controllers.get(ic).unprepareFromExecutionLocked(jobStatus);
+                controllers.get(ic).prepareForExecutionLocked(jobStatus);
            }
-        } else {
-            mRunningJobs.add(jobStatus);
-            mWorkCountTracker.onJobStarted(workType);
-            packageStats.adjustRunningCount(true, jobStatus.shouldTreatAsExpeditedJob());
-            mActivePkgStats.add(
-                    jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(), packageStats);
-        }
-        final List<JobStatus> pendingJobs = mService.mPendingJobs;
-        if (pendingJobs.remove(jobStatus)) {
-            mService.mJobPackageTracker.noteNonpending(jobStatus);
+            final PackageStats packageStats = getPkgStatsLocked(
+                    jobStatus.getSourceUserId(), jobStatus.getSourcePackageName());
+            packageStats.adjustStagedCount(false, jobStatus.shouldTreatAsExpeditedJob());
+            if (!worker.executeRunnableJob(jobStatus, workType)) {
+                Slog.e(TAG, "Error executing " + jobStatus);
+                mWorkCountTracker.onStagedJobFailed(workType);
+                for (int ic = 0; ic < numControllers; ic++) {
+                    controllers.get(ic).unprepareFromExecutionLocked(jobStatus);
+                }
+            } else {
+                mRunningJobs.add(jobStatus);
+                mWorkCountTracker.onJobStarted(workType);
+                packageStats.adjustRunningCount(true, jobStatus.shouldTreatAsExpeditedJob());
+                mActivePkgStats.add(
+                        jobStatus.getSourceUserId(), jobStatus.getSourcePackageName(),
+                        packageStats);
+            }
+            final List<JobStatus> pendingJobs = mService.mPendingJobs;
+            if (pendingJobs.remove(jobStatus)) {
+                mService.mJobPackageTracker.noteNonpending(jobStatus);
+            }
+        } finally {
+            wl.release();
        }
    }

--- a/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobSchedulerService.java
@@ -917,8 +917,18 @@ public class JobSchedulerService extends com.android.server.SystemService
        return mConstants;
    }

-    public boolean isChainedAttributionEnabled() {
-        return WorkSource.isChainedBatteryAttributionEnabled(getContext());
+    @NonNull
+    public WorkSource deriveWorkSource(int sourceUid, @Nullable String sourcePackageName) {
+        if (WorkSource.isChainedBatteryAttributionEnabled(getContext())) {
+            WorkSource ws = new WorkSource();
+            ws.createWorkChain()
+                    .addNode(sourceUid, sourcePackageName)
+                    .addNode(Process.SYSTEM_UID, "JobScheduler");
+            return ws;
+        } else {
+            return sourcePackageName == null
+                    ? new WorkSource(sourceUid) : new WorkSource(sourceUid, sourcePackageName);
+        }
    }

    @Nullable
--- a/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/JobServiceContext.java
@@ -42,7 +42,6 @@ import android.os.Message;
 import android.os.PowerManager;
 import android.os.RemoteException;
 import android.os.UserHandle;
-import android.os.WorkSource;
 import android.util.EventLog;
 import android.util.IndentingPrintWriter;
 import android.util.Slog;
@@ -109,6 +108,7 @@ public final class JobServiceContext implements ServiceConnection {
    private final Object mLock;
    private final IBatteryStats mBatteryStats;
    private final JobPackageTracker mJobPackageTracker;
+    private final PowerManager mPowerManager;
    private PowerManager.WakeLock mWakeLock;

    // Execution state.
@@ -205,6 +205,7 @@ public final class JobServiceContext implements ServiceConnection {
        mCallbackHandler = new JobServiceHandler(looper);
        mJobConcurrencyManager = concurrencyManager;
        mCompletedListener = service;
+        mPowerManager = mContext.getSystemService(PowerManager.class);
        mAvailable = true;
        mVerb = VERB_FINISHED;
        mPreferredUid = NO_PREFERRED_UID;
@@ -271,6 +272,12 @@ public final class JobServiceContext implements ServiceConnection {
            // it was inflated from disk with not-yet-coherent delay/deadline bounds.
            job.clearPersistedUtcTimes();

+            mWakeLock = mPowerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, job.getTag());
+            mWakeLock.setWorkSource(
+                    mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
+            mWakeLock.setReferenceCounted(false);
+            mWakeLock.acquire();
+
            mVerb = VERB_BINDING;
            scheduleOpTimeOutLocked();
            final Intent intent = new Intent().setComponent(job.getServiceComponent());
@@ -306,6 +313,7 @@ public final class JobServiceContext implements ServiceConnection {
                mRunningCallback = null;
                mParams = null;
                mExecutionStartTimeElapsed = 0L;
+                mWakeLock.release();
                mVerb = VERB_FINISHED;
                removeOpTimeOutLocked();
                return false;
@@ -495,42 +503,10 @@ public final class JobServiceContext implements ServiceConnection {
                return;
            }
            this.service = IJobService.Stub.asInterface(service);
-            final PowerManager pm =
-                    (PowerManager) mContext.getSystemService(Context.POWER_SERVICE);
-            PowerManager.WakeLock wl = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK,
-                    runningJob.getTag());
-            wl.setWorkSource(deriveWorkSource(runningJob));
-            wl.setReferenceCounted(false);
-            wl.acquire();
-
-            // We use a new wakelock instance per job.  In rare cases there is a race between
-            // teardown following job completion/cancellation and new job service spin-up
-            // such that if we simply assign mWakeLock to be the new instance, we orphan
-            // the currently-live lock instead of cleanly replacing it.  Watch for this and
-            // explicitly fast-forward the release if we're in that situation.
-            if (mWakeLock != null) {
-                Slog.w(TAG, "Bound new job " + runningJob + " but live wakelock " + mWakeLock
-                        + " tag=" + mWakeLock.getTag());
-                mWakeLock.release();
-            }
-            mWakeLock = wl;
            doServiceBoundLocked();
        }
    }

-    private WorkSource deriveWorkSource(JobStatus runningJob) {
-        final int jobUid = runningJob.getSourceUid();
-        if (WorkSource.isChainedBatteryAttributionEnabled(mContext)) {
-            WorkSource workSource = new WorkSource();
-            workSource.createWorkChain()
-                    .addNode(jobUid, null)
-                    .addNode(android.os.Process.SYSTEM_UID, "JobScheduler");
-            return workSource;
-        } else {
-            return new WorkSource(jobUid);
-        }
-    }
-
    /** If the client service crashes we reschedule this job and clean up. */
    @Override
    public void onServiceDisconnected(ComponentName name) {
--- a/apex/jobscheduler/service/java/com/android/server/job/controllers/TimeController.java
+++ b/apex/jobscheduler/service/java/com/android/server/job/controllers/TimeController.java
@@ -18,11 +18,9 @@ package com.android.server.job.controllers;

 import static com.android.server.job.JobSchedulerService.sElapsedRealtimeClock;

-import android.annotation.Nullable;
 import android.app.AlarmManager;
 import android.app.AlarmManager.OnAlarmListener;
 import android.content.Context;
-import android.os.Process;
 import android.os.UserHandle;
 import android.os.WorkSource;
 import android.util.IndentingPrintWriter;
@@ -62,8 +60,6 @@ public final class TimeController extends StateController {
    private long mNextDelayExpiredElapsedMillis;
    private volatile long mLastFiredDelayExpiredElapsedMillis;

-    private final boolean mChainedAttributionEnabled;
-
    private AlarmManager mAlarmService = null;
    /** List of tracked jobs, sorted asc. by deadline */
    private final List<JobStatus> mTrackedJobs = new LinkedList<>();
@@ -73,7 +69,6 @@ public final class TimeController extends StateController {

        mNextJobExpiredElapsedMillis = Long.MAX_VALUE;
        mNextDelayExpiredElapsedMillis = Long.MAX_VALUE;
-        mChainedAttributionEnabled = mService.isChainedAttributionEnabled();
    }

    /**
@@ -117,7 +112,8 @@ public final class TimeController extends StateController {
            it.add(job);

            job.setTrackingController(JobStatus.TRACKING_TIME);
-            WorkSource ws = deriveWorkSource(job.getSourceUid(), job.getSourcePackageName());
+            WorkSource ws =
+                    mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName());

            // Only update alarms if the job would be ready with the relevant timing constraint
            // satisfied.
@@ -165,7 +161,7 @@ public final class TimeController extends StateController {
            } else if (wouldBeReadyWithConstraintLocked(job, JobStatus.CONSTRAINT_DEADLINE)) {
                // This job's deadline is earlier than the current set alarm. Update the alarm.
                setDeadlineExpiredAlarmLocked(job.getLatestRunTimeElapsed(),
-                        deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
+                        mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
            }
        }
        if (job.hasTimingDelayConstraint()
@@ -177,7 +173,7 @@ public final class TimeController extends StateController {
                    && wouldBeReadyWithConstraintLocked(job, JobStatus.CONSTRAINT_TIMING_DELAY)) {
                // This job's delay is earlier than the current set alarm. Update the alarm.
                setDelayExpiredAlarmLocked(job.getEarliestRunTime(),
-                        deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
+                        mService.deriveWorkSource(job.getSourceUid(), job.getSourcePackageName()));
            }
        }
    }
@@ -248,7 +244,7 @@ public final class TimeController extends StateController {
                }
            }
            setDeadlineExpiredAlarmLocked(nextExpiryTime,
-                    deriveWorkSource(nextExpiryUid, nextExpiryPackageName));
+                    mService.deriveWorkSource(nextExpiryUid, nextExpiryPackageName));
        }
    }

@@ -312,19 +308,7 @@ public final class TimeController extends StateController {
                mStateChangedListener.onControllerStateChanged();
            }
            setDelayExpiredAlarmLocked(nextDelayTime,
-                    deriveWorkSource(nextDelayUid, nextDelayPackageName));
-        }
-    }
-
-    private WorkSource deriveWorkSource(int uid, @Nullable String packageName) {
-        if (mChainedAttributionEnabled) {
-            WorkSource ws = new WorkSource();
-            ws.createWorkChain()
-                    .addNode(uid, packageName)
-                    .addNode(Process.SYSTEM_UID, "JobScheduler");
-            return ws;
-        } else {
-            return packageName == null ? new WorkSource(uid) : new WorkSource(uid, packageName);
+                    mService.deriveWorkSource(nextDelayUid, nextDelayPackageName));
        }
    }

--- a/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/job/JobSchedulerServiceTest.java
@@ -90,11 +90,6 @@ public class JobSchedulerServiceTest {
            super(context);
            mAppStateTracker = mock(AppStateTrackerImpl.class);
        }
-
-        @Override
-        public boolean isChainedAttributionEnabled() {
-            return false;
-        }
    }

    @Before