Retry crashed bound foreground service with some delay
After quick crashes, back-off for 30 minutes and try again.
Keep backing-off up to 10 times and then give up.
Bug: 63075467
Test: runtest -x frameworks/base/tests/ServiceCrashTest
Change-Id: I3819aefac5fd48b49a70b1765e07696f2ad61328
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index 756e274..e0cde72 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -1926,6 +1926,9 @@
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 @@
// 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
- &ApplicationInfo.FLAG_PERSISTENT) == 0) {
+ if (allowRestart && sr.crashCount >= mAm.mConstants.BOUND_SERVICE_MAX_CRASH_RETRY
+ && (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,
diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java
index 2cd2603..b52db87 100644
--- a/services/core/java/com/android/server/am/ActivityManagerConstants.java
+++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java
@@ -32,6 +32,7 @@
* 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 @@
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 @@
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 @@
// 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 @@
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();
}
}
diff --git a/services/core/java/com/android/server/am/AppErrorDialog.java b/services/core/java/com/android/server/am/AppErrorDialog.java
index 51ce30e..5412266 100644
--- a/services/core/java/com/android/server/am/AppErrorDialog.java
+++ b/services/core/java/com/android/server/am/AppErrorDialog.java
@@ -40,7 +40,6 @@
private final ProcessRecord mProc;
private final boolean mRepeating;
private final boolean mIsRestartable;
-
private CharSequence mName;
static int CANT_SHOW = -1;
@@ -110,17 +109,16 @@
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(),
diff --git a/services/core/java/com/android/server/am/AppErrors.java b/services/core/java/com/android/server/am/AppErrors.java
index fc03db1..a842724 100644
--- a/services/core/java/com/android/server/am/AppErrors.java
+++ b/services/core/java/com/android/server/am/AppErrors.java
@@ -55,7 +55,6 @@
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 @@
boolean handleAppCrashLocked(ProcessRecord app, String reason,
String shortMsg, String longMsg, String stackTrace, AppErrorDialog.Data data) {
- long now = SystemClock.uptimeMillis();
- boolean showBackground = Settings.Secure.getInt(mContext.getContentResolver(),
+ final long now = SystemClock.uptimeMillis();
+ 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 @@
// 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 @@
}
}
- boolean procIsBoundForeground =
- (app.curProcState == ActivityManager.PROCESS_STATE_BOUND_FOREGROUND_SERVICE);
- // 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 (data != null && tryAgain) {
+ data.isRestartableForService = true;
}
// If the crashing process is what we consider to be the "home process" and it has been
@@ -690,7 +702,7 @@
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);
}
diff --git a/tests/ServiceCrashTest/Android.mk b/tests/ServiceCrashTest/Android.mk
new file mode 100644
index 0000000..d1f8456
--- /dev/null
+++ b/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))
diff --git a/tests/ServiceCrashTest/AndroidManifest.xml b/tests/ServiceCrashTest/AndroidManifest.xml
new file mode 100644
index 0000000..387c8b8
--- /dev/null
+++ b/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>
diff --git a/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/CrashingService.java b/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/CrashingService.java
new file mode 100644
index 0000000..8593bce
--- /dev/null
+++ b/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;
+ }
+}
\ No newline at end of file
diff --git a/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/MainActivity.java b/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/MainActivity.java
new file mode 100644
index 0000000..8fffecc
--- /dev/null
+++ b/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();
+ }
+}
diff --git a/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/ServiceCrashTest.java b/tests/ServiceCrashTest/src/com/android/tests/servicecrashtest/ServiceCrashTest.java
new file mode 100644
index 0000000..fb0fa4b
--- /dev/null
+++ b/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) {
+ }
+ }
+}