Merge "Retry crashed bound foreground service with some delay" into oc-mr1-dev

services/core/java/com/android/server/am/ActiveServices.java

@@ -1926,6 +1926,9 @@ public final class ActiveServices {
             if (r.restartDelay == 0) {
                 r.restartCount++;
                 r.restartDelay = minDuration;
+            } else if (r.crashCount > 1) {
+                r.restartDelay = mAm.mConstants.BOUND_SERVICE_CRASH_RESTART_DURATION
+                        * (r.crashCount - 1);
             } else {
                 // If it has been a "reasonably long time" since the service
                 // was started, then reset our restart duration back to
@@ -3129,8 +3132,9 @@ public final class ActiveServices {
 
             // Any services running in the application may need to be placed
             // back in the pending list.
-            if (allowRestart && sr.crashCount >= 2 && (sr.serviceInfo.applicationInfo.flags
+            if (allowRestart && sr.crashCount >= mAm.mConstants.BOUND_SERVICE_MAX_CRASH_RETRY
-                    &ApplicationInfo.FLAG_PERSISTENT) == 0) {
+                    && (sr.serviceInfo.applicationInfo.flags
+                        &ApplicationInfo.FLAG_PERSISTENT) == 0) {
                 Slog.w(TAG, "Service crashed " + sr.crashCount
                         + " times, stopping: " + sr);
                 EventLog.writeEvent(EventLogTags.AM_SERVICE_CRASHED_TOO_MUCH,

services/core/java/com/android/server/am/ActivityManagerConstants.java

@@ -32,6 +32,7 @@ import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_POWER_QUICK
  * Settings constants that can modify the activity manager's behavior.
  */
 final class ActivityManagerConstants extends ContentObserver {
+
     // Key names stored in the settings value.
     private static final String KEY_MAX_CACHED_PROCESSES = "max_cached_processes";
     private static final String KEY_BACKGROUND_SETTLE_TIME = "background_settle_time";
@@ -63,6 +64,8 @@ final class ActivityManagerConstants extends ContentObserver {
     static final String KEY_SERVICE_MIN_RESTART_TIME_BETWEEN = "service_min_restart_time_between";
     static final String KEY_MAX_SERVICE_INACTIVITY = "service_max_inactivity";
     static final String KEY_BG_START_TIMEOUT = "service_bg_start_timeout";
+    static final String KEY_BOUND_SERVICE_CRASH_RESTART_DURATION = "service_crash_restart_duration";
+    static final String KEY_BOUND_SERVICE_CRASH_MAX_RETRY = "service_crash_max_retry";
 
     private static final int DEFAULT_MAX_CACHED_PROCESSES = 32;
     private static final long DEFAULT_BACKGROUND_SETTLE_TIME = 60*1000;
@@ -88,6 +91,9 @@ final class ActivityManagerConstants extends ContentObserver {
     private static final long DEFAULT_SERVICE_MIN_RESTART_TIME_BETWEEN = 10*1000;
     private static final long DEFAULT_MAX_SERVICE_INACTIVITY = 30*60*1000;
     private static final long DEFAULT_BG_START_TIMEOUT = 15*1000;
+    private static final long DEFAULT_BOUND_SERVICE_CRASH_RESTART_DURATION = 30*60_000;
+    private static final int DEFAULT_BOUND_SERVICE_CRASH_MAX_RETRY = 16;
+
 
     // Maximum number of cached processes we will allow.
     public int MAX_CACHED_PROCESSES = DEFAULT_MAX_CACHED_PROCESSES;
@@ -190,6 +196,12 @@ final class ActivityManagerConstants extends ContentObserver {
     // allowing the next pending start to run.
     public long BG_START_TIMEOUT = DEFAULT_BG_START_TIMEOUT;
 
+    // Initial backoff delay for retrying bound foreground services
+    public long BOUND_SERVICE_CRASH_RESTART_DURATION = DEFAULT_BOUND_SERVICE_CRASH_RESTART_DURATION;
+
+    // Maximum number of retries for bound foreground services that crash soon after start
+    public long BOUND_SERVICE_MAX_CRASH_RETRY = DEFAULT_BOUND_SERVICE_CRASH_MAX_RETRY;
+
     private final ActivityManagerService mService;
     private ContentResolver mResolver;
     private final KeyValueListParser mParser = new KeyValueListParser(',');
@@ -308,6 +320,12 @@ final class ActivityManagerConstants extends ContentObserver {
                     DEFAULT_MAX_SERVICE_INACTIVITY);
             BG_START_TIMEOUT = mParser.getLong(KEY_BG_START_TIMEOUT,
                     DEFAULT_BG_START_TIMEOUT);
+            BOUND_SERVICE_CRASH_RESTART_DURATION = mParser.getLong(
+                KEY_BOUND_SERVICE_CRASH_RESTART_DURATION,
+                DEFAULT_BOUND_SERVICE_CRASH_RESTART_DURATION);
+            BOUND_SERVICE_MAX_CRASH_RETRY = mParser.getInt(KEY_BOUND_SERVICE_CRASH_MAX_RETRY,
+                DEFAULT_BOUND_SERVICE_CRASH_MAX_RETRY);
+
             updateMaxCachedProcesses();
         }
     }

services/core/java/com/android/server/am/AppErrorDialog.java

@@ -40,7 +40,6 @@ final class AppErrorDialog extends BaseErrorDialog implements View.OnClickListen
     private final ProcessRecord mProc;
     private final boolean mRepeating;
     private final boolean mIsRestartable;
-
     private CharSequence mName;
 
     static int CANT_SHOW = -1;
@@ -110,17 +109,16 @@ final class AppErrorDialog extends BaseErrorDialog implements View.OnClickListen
         LayoutInflater.from(context).inflate(
                 com.android.internal.R.layout.app_error_dialog, frame, true);
 
-        boolean hasRestart = !mRepeating && mIsRestartable;
         final boolean hasReceiver = mProc.errorReportReceiver != null;
 
         final TextView restart = findViewById(com.android.internal.R.id.aerr_restart);
         restart.setOnClickListener(this);
-        restart.setVisibility(hasRestart ? View.VISIBLE : View.GONE);
+        restart.setVisibility(mIsRestartable ? View.VISIBLE : View.GONE);
         final TextView report = findViewById(com.android.internal.R.id.aerr_report);
         report.setOnClickListener(this);
         report.setVisibility(hasReceiver ? View.VISIBLE : View.GONE);
         final TextView close = findViewById(com.android.internal.R.id.aerr_close);
-        close.setVisibility(!hasRestart ? View.VISIBLE : View.GONE);
+        close.setVisibility(mRepeating ? View.VISIBLE : View.GONE);
         close.setOnClickListener(this);
 
         boolean showMute = !Build.IS_USER && Settings.Global.getInt(context.getContentResolver(),

services/core/java/com/android/server/am/AppErrors.java

@@ -55,7 +55,6 @@ import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Set;
 
 import static com.android.server.Watchdog.NATIVE_STACKS_OF_INTEREST;
@@ -593,20 +592,46 @@ class AppErrors {
 
     boolean handleAppCrashLocked(ProcessRecord app, String reason,
             String shortMsg, String longMsg, String stackTrace, AppErrorDialog.Data data) {
-        long now = SystemClock.uptimeMillis();
+        final long now = SystemClock.uptimeMillis();
-        boolean showBackground = Settings.Secure.getInt(mContext.getContentResolver(),
+        final boolean showBackground = Settings.Secure.getInt(mContext.getContentResolver(),
                 Settings.Secure.ANR_SHOW_BACKGROUND, 0) != 0;
 
+        final boolean procIsBoundForeground =
+            (app.curProcState == ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE);
+
         Long crashTime;
         Long crashTimePersistent;
+        boolean tryAgain = false;
+
         if (!app.isolated) {
             crashTime = mProcessCrashTimes.get(app.info.processName, app.uid);
             crashTimePersistent = mProcessCrashTimesPersistent.get(app.info.processName, app.uid);
         } else {
             crashTime = crashTimePersistent = null;
         }
-        if (crashTime != null && now < crashTime+ProcessList.MIN_CRASH_INTERVAL) {
+
-            // This process loses!
+        // Bump up the crash count of any services currently running in the proc.
+        for (int i = app.services.size() - 1; i >= 0; i--) {
+            // Any services running in the application need to be placed
+            // back in the pending list.
+            ServiceRecord sr = app.services.valueAt(i);
+            // If the service was restarted a while ago, then reset crash count, else increment it.
+            if (now > sr.restartTime + ProcessList.MIN_CRASH_INTERVAL) {
+                sr.crashCount = 1;
+            } else {
+                sr.crashCount++;
+            }
+            // Allow restarting for started or bound foreground services that are crashing.
+            // This includes wallpapers.
+            if (sr.crashCount < mService.mConstants.BOUND_SERVICE_MAX_CRASH_RETRY
+                    && (sr.isForeground || procIsBoundForeground)) {
+                tryAgain = true;
+            }
+        }
+
+        if (crashTime != null && now < crashTime + ProcessList.MIN_CRASH_INTERVAL) {
+            // The process crashed again very quickly. If it was a bound foreground service, let's
+            // try to restart again in a while, otherwise the process loses!
             Slog.w(TAG, "Process " + app.info.processName
                     + " has crashed too many times: killing!");
             EventLog.writeEvent(EventLogTags.AM_PROCESS_CRASHED_TOO_MUCH,
@@ -631,7 +656,7 @@ class AppErrors {
                 // Don't let services in this process be restarted and potentially
                 // annoy the user repeatedly.  Unless it is persistent, since those
                 // processes run critical code.
-                mService.removeProcessLocked(app, false, false, "crash");
+                mService.removeProcessLocked(app, false, tryAgain, "crash");
                 mService.mStackSupervisor.resumeFocusedStackTopActivityLocked();
                 if (!showBackground) {
                     return false;
@@ -650,21 +675,8 @@ class AppErrors {
             }
         }
 
-        boolean procIsBoundForeground =
+        if (data != null && tryAgain) {
-                (app.curProcState == ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE);
+            data.isRestartableForService = true;
-        // Bump up the crash count of any services currently running in the proc.
-        for (int i=app.services.size()-1; i>=0; i--) {
-            // Any services running in the application need to be placed
-            // back in the pending list.
-            ServiceRecord sr = app.services.valueAt(i);
-            sr.crashCount++;
-
-            // Allow restarting for started or bound foreground services that are crashing the
-            // first time. This includes wallpapers.
-            if ((data != null) && (sr.crashCount <= 1)
-                    && (sr.isForeground || procIsBoundForeground)) {
-                data.isRestartableForService = true;
-            }
         }
 
         // If the crashing process is what we consider to be the "home process" and it has been
@@ -690,7 +702,7 @@ class AppErrors {
 
         if (!app.isolated) {
             // XXX Can't keep track of crash times for isolated processes,
-            // because they don't have a perisistent identity.
+            // because they don't have a persistent identity.
             mProcessCrashTimes.put(app.info.processName, app.uid, now);
             mProcessCrashTimesPersistent.put(app.info.processName, app.uid, now);
         }

tests/ServiceCrashTest/Android.mk

@@ -0,0 +1,19 @@
+LOCAL_PATH:= $(call my-dir)
+include $(CLEAR_VARS)
+
+LOCAL_MODULE_TAGS := tests
+
+# Only compile source java files in this apk.
+LOCAL_SRC_FILES := $(call all-java-files-under, src)
+
+LOCAL_PACKAGE_NAME := ServiceCrashTest
+
+LOCAL_CERTIFICATE := platform
+LOCAL_JAVA_LIBRARIES := legacy-android-test
+
+LOCAL_STATIC_JAVA_LIBRARIES := compatibility-device-util android-support-test
+
+include $(BUILD_PACKAGE)
+
+# Use the following include to make our test apk.
+include $(call all-makefiles-under,$(LOCAL_PATH))

tests/ServiceCrashTest/AndroidManifest.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+        package="com.android.tests.servicecrashtest">
+
+    <application android:label="Service Crash Test">
+        <uses-library android:name="android.test.runner" />
+
+        <service android:name=".CrashingService"
+                android:process=":badservice" />
+
+        <activity android:name=".MainActivity" >
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+            </intent-filter>
+        </activity>
+    </application>
+
+    <instrumentation android:label="Test bound service crash restart"
+            android:name="android.test.InstrumentationTestRunner"
+            android:targetPackage="com.android.tests.servicecrashtest" />
+
+</manifest>

tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/CrashingService.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.tests.servicecrashtest;
+
+import android.app.Service;
+import android.content.Intent;
+import android.os.Handler;
+import android.os.HandlerThread;
+import android.os.IBinder;
+import android.os.Looper;
+import android.os.Message;
+import android.os.Process;
+import android.widget.Toast;
+
+public class CrashingService extends Service {
+    private ServiceHandler mServiceHandler;
+
+    static long CRASH_DELAY = 1000;
+
+    // Handler that receives messages from the thread
+    private final class ServiceHandler extends Handler {
+        public ServiceHandler(Looper looper) {
+            super(looper);
+        }
+
+        @Override
+        public void handleMessage(Message msg) {
+            throw new RuntimeException("Crashing!");
+        }
+    }
+
+    @Override
+    public void onCreate() {
+        mServiceHandler = new ServiceHandler(Looper.getMainLooper());
+        Toast.makeText(this, "service starting", Toast.LENGTH_SHORT).show();
+
+        Message msg = mServiceHandler.obtainMessage();
+        mServiceHandler.sendMessageDelayed(msg, CRASH_DELAY);
+    }
+
+    @Override
+    public int onStartCommand(Intent intent, int flags, int startId) {
+        // If we get killed, after returning from here, restart
+        return START_STICKY;
+    }
+
+    @Override
+    public IBinder onBind(Intent intent) {
+        // We don't provide binding, so return null
+        return null;
+    }
+}

tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/MainActivity.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.tests.servicecrashtest;
+
+import android.app.Activity;
+import android.app.Service;
+import android.content.ComponentName;
+import android.content.Intent;
+import android.content.ServiceConnection;
+import android.os.Bundle;
+import android.os.IBinder;
+import android.util.Log;
+import android.widget.TextView;
+
+import java.util.concurrent.CountDownLatch;
+
+public class MainActivity extends Activity {
+
+    private static final String TAG = "ServiceCrashTest";
+
+    static final CountDownLatch sBindingDiedLatch = new CountDownLatch(1);
+
+    private ServiceConnection mServiceConnection = new ServiceConnection() {
+
+        @Override
+        public void onServiceConnected(ComponentName name, IBinder service) {
+            Log.i(TAG, "Service connected");
+        }
+
+        @Override
+        public void onServiceDisconnected(ComponentName name) {
+            Log.i(TAG, "Service disconnected");
+        }
+
+        @Override
+        public void onBindingDied(ComponentName componentName) {
+            Log.i(TAG, "Binding died");
+            sBindingDiedLatch.countDown();
+        }
+    };
+
+    @Override
+    public void onCreate(Bundle savedInstance) {
+        super.onCreate(savedInstance);
+
+        setContentView(new TextView(this));
+    }
+
+    public void onResume() {
+        Intent intent = new Intent();
+        intent.setClass(this, CrashingService.class);
+        bindService(intent, mServiceConnection, Service.BIND_AUTO_CREATE);
+
+        super.onResume();
+    }
+}

tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/ServiceCrashTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.tests.servicecrashtest;
+
+import android.app.UiAutomation;
+import android.content.Context;
+import android.content.Intent;
+import android.os.RemoteException;
+import android.provider.Settings;
+import android.test.InstrumentationTestCase;
+
+import com.android.compatibility.common.util.SystemUtil;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+public class ServiceCrashTest extends InstrumentationTestCase {
+
+    private static final String TAG = ServiceCrashTest.class.getSimpleName();
+
+    private String mResetConstants = "foo=bar";
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mResetConstants = Settings.Global.getString(
+                getInstrumentation().getContext().getContentResolver(),
+                Settings.Global.ACTIVITY_MANAGER_CONSTANTS);
+        setAMConstants("service_crash_restart_duration=5000,service_crash_max_retry=4");
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        // Reset the activity manager constants
+        setAMConstants(mResetConstants);
+        super.tearDown();
+    }
+
+    private void setAMConstants(String value) throws IOException {
+        // Set the activity manager constants
+        if (value == null) {
+            SystemUtil.runShellCommand(getInstrumentation(),
+                    "settings delete global activity_manager_constants");
+        } else {
+            SystemUtil.runShellCommand(getInstrumentation(), "settings put global "
+                    + "activity_manager_constants " + value);
+        }
+    }
+
+    public void testCrashQuickly() throws RemoteException {
+        Context ctx = getInstrumentation().getContext();
+        // Start the activity, which will bind the crashing service
+        Intent intent = new Intent();
+        intent.setAction(Intent.ACTION_MAIN);
+        intent.setClass(ctx, MainActivity.class);
+        ctx.startActivity(intent);
+        try {
+            assertTrue(MainActivity.sBindingDiedLatch.await(200, TimeUnit.SECONDS));
+        } catch (InterruptedException ie) {
+        }
+    }
+}