From 75fc5258b73b4b9b079a9383420a1d6b88575d72 Mon Sep 17 00:00:00 2001 From: Matthew Williams Date: Tue, 2 Sep 2014 16:17:53 -0700 Subject: [PATCH] Add timeout when waiting to bind to JobService BUG: 17322886 bindService() to an invalid service might never actually result in onServiceConnected being called , for e.g. if the client service doesn't actually implement JobService. This wastes an execution slot as we end up waiting forever. Also made the javadocs clearer for the JobScheduler class. Change-Id: Ie15ebbe18c0b7579f2ab77dd46428d354ef632c3 --- core/java/android/app/job/JobScheduler.java | 13 +++- .../server/job/JobSchedulerService.java | 23 ++++-- .../android/server/job/JobServiceContext.java | 73 +++++++++---------- 3 files changed, 64 insertions(+), 45 deletions(-) diff --git a/core/java/android/app/job/JobScheduler.java b/core/java/android/app/job/JobScheduler.java index ca7022d310412..5edc2a0efd157 100644 --- a/core/java/android/app/job/JobScheduler.java +++ b/core/java/android/app/job/JobScheduler.java @@ -21,14 +21,23 @@ import java.util.List; import android.content.Context; /** - * Class for scheduling various types of jobs with the scheduling framework on the device. + * This is an API for scheduling various types of jobs against the framework that will be executed + * in your application's own process. + *

* See {@link android.app.job.JobInfo} for more description of the types of jobs that can be run - * and how to construct them. + * and how to construct them. You will construct these JobInfo objects and pass them to the + * JobScheduler with {@link #schedule(JobInfo)}. When the criteria declared are met, the + * system will execute this job on your application's {@link android.app.job.JobService}. + * You identify which JobService is meant to execute the logic for your job when you create the + * JobInfo with {@link android.app.job.JobInfo.Builder#Builder(int, android.content.ComponentName)}. + *

+ *

* The framework will be intelligent about when you receive your callbacks, and attempt to batch * and defer them as much as possible. Typically if you don't specify a deadline on your job, it * can be run at any moment depending on the current state of the JobScheduler's internal queue, * however it might be deferred as long as until the next time the device is connected to a power * source. + *

*

You do not * instantiate this class directly; instead, retrieve it through * {@link android.content.Context#getSystemService diff --git a/services/core/java/com/android/server/job/JobSchedulerService.java b/services/core/java/com/android/server/job/JobSchedulerService.java index c3bc306974a55..30154d716f7fc 100644 --- a/services/core/java/com/android/server/job/JobSchedulerService.java +++ b/services/core/java/com/android/server/job/JobSchedulerService.java @@ -199,9 +199,6 @@ public class JobSchedulerService extends com.android.server.SystemService List jobsForUser = mJobs.getJobsByUser(userHandle); for (int i=0; i jobsForUid = mJobs.getJobsByUid(uid); for (int i=0; i jobs = mJobs.getJobs(); + if (DEBUG) { + Slog.d(TAG, "queuing all ready jobs for execution:"); + } for (int i=0; i it = mPendingJobs.iterator(); + if (DEBUG) { + Slog.d(TAG, "pending queue: " + mPendingJobs.size() + " jobs."); + } while (it.hasNext()) { JobStatus nextPending = it.next(); JobServiceContext availableContext = null; diff --git a/services/core/java/com/android/server/job/JobServiceContext.java b/services/core/java/com/android/server/job/JobServiceContext.java index 52979117dc6fb..344c57b3b93cf 100644 --- a/services/core/java/com/android/server/job/JobServiceContext.java +++ b/services/core/java/com/android/server/job/JobServiceContext.java @@ -157,6 +157,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne mExecutionStartTimeElapsed = SystemClock.elapsedRealtime(); mVerb = VERB_BINDING; + scheduleOpTimeOut(); final Intent intent = new Intent().setComponent(job.getServiceComponent()); boolean binding = mContext.bindServiceAsUser(intent, this, Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND, @@ -253,8 +254,6 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne return; } this.service = IJobService.Stub.asInterface(service); - // Remove all timeouts. - mCallbackHandler.removeMessages(MSG_TIMEOUT); final PowerManager pm = (PowerManager) mContext.getSystemService(Context.POWER_SERVICE); mWakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, mRunningJob.getTag()); @@ -299,6 +298,7 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne public void handleMessage(Message message) { switch (message.what) { case MSG_SERVICE_BOUND: + removeMessages(MSG_TIMEOUT); handleServiceBoundH(); break; case MSG_CALLBACK: @@ -337,6 +337,9 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne /** Start the job on the service. */ private void handleServiceBoundH() { + if (DEBUG) { + Slog.d(TAG, "MSG_SERVICE_BOUND for " + mRunningJob.toShortString()); + } if (mVerb != VERB_BINDING) { Slog.e(TAG, "Sending onStartJob for a job that isn't pending. " + VERB_STRINGS[mVerb]); @@ -456,42 +459,36 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne /** Process MSG_TIMEOUT here. */ private void handleOpTimeoutH() { - if (JobSchedulerService.DEBUG) { - Slog.d(TAG, "MSG_TIMEOUT of " + - mRunningJob.getServiceComponent().getShortClassName() + " : " - + mParams.getJobId()); - } - - final int jobId = mParams.getJobId(); switch (mVerb) { + case VERB_BINDING: + Slog.e(TAG, "Time-out while trying to bind " + mRunningJob.toShortString() + + ", dropping."); + closeAndCleanupJobH(false /* needsReschedule */); + break; case VERB_STARTING: // Client unresponsive - wedged or failed to respond in time. We don't really // know what happened so let's log it and notify the JobScheduler // FINISHED/NO-RETRY. Slog.e(TAG, "No response from client for onStartJob '" + - mRunningJob.getServiceComponent().getShortClassName() + "' jId: " - + jobId); + mRunningJob.toShortString()); closeAndCleanupJobH(false /* needsReschedule */); break; case VERB_STOPPING: // At least we got somewhere, so fail but ask the JobScheduler to reschedule. Slog.e(TAG, "No response from client for onStopJob, '" + - mRunningJob.getServiceComponent().getShortClassName() + "' jId: " - + jobId); + mRunningJob.toShortString()); closeAndCleanupJobH(true /* needsReschedule */); break; case VERB_EXECUTING: // Not an error - client ran out of time. Slog.i(TAG, "Client timed out while executing (no jobFinished received)." + - " sending onStop. " + - mRunningJob.getServiceComponent().getShortClassName() + "' jId: " - + jobId); + " sending onStop. " + mRunningJob.toShortString()); sendStopMessageH(); break; default: - Slog.e(TAG, "Handling timeout for an unknown active job state: " - + mRunningJob); - return; + Slog.e(TAG, "Handling timeout for an invalid job state: " + + mRunningJob.toShortString() + ", dropping."); + closeAndCleanupJobH(false /* needsReschedule */); } } @@ -530,7 +527,9 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne } catch (RemoteException e) { // Whatever. } - mWakeLock.release(); + if (mWakeLock != null) { + mWakeLock.release(); + } mContext.unbindService(JobServiceContext.this); mWakeLock = null; mRunningJob = null; @@ -547,25 +546,25 @@ public class JobServiceContext extends IJobCallback.Stub implements ServiceConne removeMessages(MSG_SHUTDOWN_EXECUTION); mCompletedListener.onJobCompleted(completedJob, reschedule); } + } - /** - * Called when sending a message to the client, over whose execution we have no control. If - * we haven't received a response in a certain amount of time, we want to give up and carry - * on with life. - */ - private void scheduleOpTimeOut() { - mCallbackHandler.removeMessages(MSG_TIMEOUT); + /** + * Called when sending a message to the client, over whose execution we have no control. If + * we haven't received a response in a certain amount of time, we want to give up and carry + * on with life. + */ + private void scheduleOpTimeOut() { + mCallbackHandler.removeMessages(MSG_TIMEOUT); - final long timeoutMillis = (mVerb == VERB_EXECUTING) ? - EXECUTING_TIMESLICE_MILLIS : OP_TIMEOUT_MILLIS; - if (DEBUG) { - Slog.d(TAG, "Scheduling time out for '" + - mRunningJob.getServiceComponent().getShortClassName() + "' jId: " + - mParams.getJobId() + ", in " + (timeoutMillis / 1000) + " s"); - } - Message m = mCallbackHandler.obtainMessage(MSG_TIMEOUT); - mCallbackHandler.sendMessageDelayed(m, timeoutMillis); - mTimeoutElapsed = SystemClock.elapsedRealtime() + timeoutMillis; + final long timeoutMillis = (mVerb == VERB_EXECUTING) ? + EXECUTING_TIMESLICE_MILLIS : OP_TIMEOUT_MILLIS; + if (DEBUG) { + Slog.d(TAG, "Scheduling time out for '" + + mRunningJob.getServiceComponent().getShortClassName() + "' jId: " + + mParams.getJobId() + ", in " + (timeoutMillis / 1000) + " s"); } + Message m = mCallbackHandler.obtainMessage(MSG_TIMEOUT); + mCallbackHandler.sendMessageDelayed(m, timeoutMillis); + mTimeoutElapsed = SystemClock.elapsedRealtime() + timeoutMillis; } }