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

diff --git a/core/java/android/app/job/JobScheduler.java b/core/java/android/app/job/JobScheduler.java
index ca7022d..5edc2a0 100644 (file)
--- 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.
+ * <p>
  * 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)}.
+ * </p>
+ * <p>
  * 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.
+ * </p>
  * <p>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 c3bc306..30154d7 100644 (file)
--- 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<JobStatus> jobsForUser = mJobs.getJobsByUser(userHandle);
             for (int i=0; i<jobsForUser.size(); i++) {
                 JobStatus toRemove = jobsForUser.get(i);
-                if (DEBUG) {
-                    Slog.d(TAG, "Cancelling: " + toRemove);
-                }
                 cancelJobLocked(toRemove);
             }
         }
@@ -219,9 +216,6 @@ public class JobSchedulerService extends com.android.server.SystemService
             List<JobStatus> jobsForUid = mJobs.getJobsByUid(uid);
             for (int i=0; i<jobsForUid.size(); i++) {
                 JobStatus toRemove = jobsForUid.get(i);
-                if (DEBUG) {
-                    Slog.d(TAG, "Cancelling: " + toRemove);
-                }
                 cancelJobLocked(toRemove);
             }
         }
@@ -547,14 +541,28 @@ public class JobSchedulerService extends com.android.server.SystemService
         private void queueReadyJobsForExecutionH() {
             synchronized (mJobs) {
                 ArraySet<JobStatus> jobs = mJobs.getJobs();
+                if (DEBUG) {
+                    Slog.d(TAG, "queuing all ready jobs for execution:");
+                }
                 for (int i=0; i<jobs.size(); i++) {
                     JobStatus job = jobs.valueAt(i);
                     if (isReadyToBeExecutedLocked(job)) {
+                        if (DEBUG) {
+                            Slog.d(TAG, "    queued " + job.toShortString());
+                        }
                         mPendingJobs.add(job);
                     } else if (isReadyToBeCancelledLocked(job)) {
                         stopJobOnServiceContextLocked(job);
                     }
                 }
+                if (DEBUG) {
+                    final int queuedJobs = mPendingJobs.size();
+                    if (queuedJobs == 0) {
+                        Slog.d(TAG, "No jobs pending.");
+                    } else {
+                        Slog.d(TAG, queuedJobs + " jobs queued.");
+                    }
+                }
             }
         }
 
@@ -657,6 +665,9 @@ public class JobSchedulerService extends com.android.server.SystemService
         private void maybeRunPendingJobsH() {
             synchronized (mJobs) {
                 Iterator<JobStatus> 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 5297911..344c57b 100644 (file)
--- 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;
     }
 }