Work on issue #18201239: ANRs in com.google.process.gapps:

Reason: Executing service com.google.android.syncadapters.contacts
/.SyncHighResPhotoIntentService

Make the code more robust when destroying services, so that if
the nesting count gets out of sync we don't just hang.

Change-Id: If117d5ef242e7c148fd9576bd89a1a092583d6ad

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index b5c0e90..f2be45c 100644 (file)
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -168,6 +168,13 @@ public final class ActivityThread {
     private static final int LOG_ON_PAUSE_CALLED = 30021;
     private static final int LOG_ON_RESUME_CALLED = 30022;
 
+    /** Type for IActivityManager.serviceDoneExecuting: anonymous operation */
+    public static final int SERVICE_DONE_EXECUTING_ANON = 0;
+    /** Type for IActivityManager.serviceDoneExecuting: done with an onStart call */
+    public static final int SERVICE_DONE_EXECUTING_START = 1;
+    /** Type for IActivityManager.serviceDoneExecuting: done stopping (destroying) service */
+    public static final int SERVICE_DONE_EXECUTING_STOP = 2;
+
     private ContextImpl mSystemContext;
 
     static IPackageManager sPackageManager;
@@ -2758,7 +2765,7 @@ public final class ActivityThread {
             mServices.put(data.token, service);
             try {
                 ActivityManagerNative.getDefault().serviceDoneExecuting(
-                        data.token, 0, 0, 0);
+                        data.token, SERVICE_DONE_EXECUTING_ANON, 0, 0);
             } catch (RemoteException e) {
                 // nothing to do.
             }
@@ -2787,7 +2794,7 @@ public final class ActivityThread {
                     } else {
                         s.onRebind(data.intent);
                         ActivityManagerNative.getDefault().serviceDoneExecuting(
-                                data.token, 0, 0, 0);
+                                data.token, SERVICE_DONE_EXECUTING_ANON, 0, 0);
                     }
                     ensureJitEnabled();
                 } catch (RemoteException ex) {
@@ -2815,7 +2822,7 @@ public final class ActivityThread {
                                 data.token, data.intent, doRebind);
                     } else {
                         ActivityManagerNative.getDefault().serviceDoneExecuting(
-                                data.token, 0, 0, 0);
+                                data.token, SERVICE_DONE_EXECUTING_ANON, 0, 0);
                     }
                 } catch (RemoteException ex) {
                 }
@@ -2897,7 +2904,7 @@ public final class ActivityThread {
 
                 try {
                     ActivityManagerNative.getDefault().serviceDoneExecuting(
-                            data.token, 1, data.startId, res);
+                            data.token, SERVICE_DONE_EXECUTING_START, data.startId, res);
                 } catch (RemoteException e) {
                     // nothing to do.
                 }
@@ -2928,7 +2935,7 @@ public final class ActivityThread {
 
                 try {
                     ActivityManagerNative.getDefault().serviceDoneExecuting(
-                            token, 0, 0, 0);
+                            token, SERVICE_DONE_EXECUTING_STOP, 0, 0);
                 } catch (RemoteException e) {
                     // nothing to do.
                     Slog.i(TAG, "handleStopService: unable to execute serviceDoneExecuting for "

diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 6202ba0..6d28382 100755 (executable)
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 
+import android.app.ActivityThread;
 import android.os.Build;
 import android.os.DeadObjectException;
 import android.os.Handler;
@@ -1692,6 +1693,7 @@ public final class ActiveServices {
                 try {
                     bumpServiceExecutingLocked(r, false, "destroy");
                     mDestroyingServices.add(r);
+                    r.destroying = true;
                     mAm.updateOomAdjLocked(r.app);
                     r.app.thread.scheduleStopService(r);
                 } catch (Exception e) {
@@ -1813,7 +1815,7 @@ public final class ActiveServices {
     void serviceDoneExecutingLocked(ServiceRecord r, int type, int startId, int res) {
         boolean inDestroying = mDestroyingServices.contains(r);
         if (r != null) {
-            if (type == 1) {
+            if (type == ActivityThread.SERVICE_DONE_EXECUTING_START) {
                 // This is a call from a service start...  take care of
                 // book-keeping.
                 r.callStart = true;
@@ -1862,6 +1864,20 @@ public final class ActiveServices {
                 if (res == Service.START_STICKY_COMPATIBILITY) {
                     r.callStart = false;
                 }
+            } else if (type == ActivityThread.SERVICE_DONE_EXECUTING_STOP) {
+                // This is the final call from destroying the service...  we should
+                // actually be getting rid of the service at this point.  Do some
+                // validation of its state, and ensure it will be fully removed.
+                if (!inDestroying) {
+                    // Not sure what else to do with this...  if it is not actually in the
+                    // destroying list, we don't need to make sure to remove it from it.
+                    Slog.wtfStack(TAG, "Service done with onDestroy, but not inDestroying: "
+                            + r);
+                } else if (r.executeNesting != 1) {
+                    Slog.wtfStack(TAG, "Service done with onDestroy, but executeNesting="
+                            + r.executeNesting + ": " + r);
+                    r.executeNesting = 1;
+                }
             }
             final long origId = Binder.clearCallingIdentity();
             serviceDoneExecutingLocked(r, inDestroying, inDestroying);

diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java
index 8381c4a..1cb1bb8 100644 (file)
--- a/services/core/java/com/android/server/am/ServiceRecord.java
+++ b/services/core/java/com/android/server/am/ServiceRecord.java
@@ -105,6 +105,7 @@ final class ServiceRecord extends Binder {
     long restartDelay;      // delay until next restart attempt.
     long restartTime;       // time of last restart.
     long nextRestartTime;   // time when restartDelay will expire.
+    boolean destroying;     // set when we have started destroying the service
     long destroyTime;       // time at which destory was initiated.
 
     String stringName;      // caching of toString
@@ -251,9 +252,11 @@ final class ServiceRecord extends Binder {
                     TimeUtils.formatDuration(executingStart, now, pw);
                     pw.println();
         }
-        if (destroyTime != 0) {
-            pw.print(" destroyed=");
-            TimeUtils.formatDuration(destroyTime, now, pw);
+        if (destroying || destroyTime != 0) {
+            pw.print(prefix); pw.print("destroying="); pw.print(destroying);
+                    pw.print(" destroyTime=");
+                    TimeUtils.formatDuration(destroyTime, now, pw);
+                    pw.println();
         }
         if (crashCount != 0 || restartCount != 0
                 || restartDelay != 0 || nextRestartTime != 0) {