Fix issue #37306208: CTS: ServiceTest#testForegroundService...
..._removeNotificationOnStop failure
We weren't removing the foreground timeout when a service is
brought down and still waiting for the app to make it foreground.
Instead, turn this in to an immediate crash of the app when we
detect it while bringing down the service. Also make various
paths more robust against trying to do things with a ServiceRecord
that isn't currently in ActiveServices.
Test: bit CtsAppTestCases:ServiceTest
Change-Id: I06f4ac48fb983483569677ab1ac6e5b0b681c4ae
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index c77820b..a3c691c 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -761,7 +761,7 @@
}
}
if (r.fgRequired) {
- if (DEBUG_BACKGROUND_CHECK) {
+ if (DEBUG_SERVICE || DEBUG_BACKGROUND_CHECK) {
Slog.i(TAG, "Service called startForeground() as required: " + r);
}
r.fgRequired = false;
@@ -1339,16 +1339,19 @@
final ComponentName comp = service.getComponent();
if (comp != null) {
r = smap.mServicesByName.get(comp);
+ if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by component: " + r);
}
if (r == null && !isBindExternal) {
Intent.FilterComparison filter = new Intent.FilterComparison(service);
r = smap.mServicesByIntent.get(filter);
+ if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by intent: " + r);
}
if (r != null && (r.serviceInfo.flags & ServiceInfo.FLAG_EXTERNAL_SERVICE) != 0
&& !callingPackage.equals(r.packageName)) {
// If an external service is running within its own package, other packages
// should not bind to that instance.
r = null;
+ if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Whoops, can't use existing external service");
}
if (r == null) {
try {
@@ -1408,12 +1411,14 @@
sInfo.applicationInfo = mAm.getAppInfoForUser(sInfo.applicationInfo, userId);
}
r = smap.mServicesByName.get(name);
+ if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE,
+ "Retrieved via pm by intent: " + r);
if (r == null && createIfNeeded) {
- Intent.FilterComparison filter
+ final Intent.FilterComparison filter
= new Intent.FilterComparison(service.cloneFilter());
- ServiceRestarter res = new ServiceRestarter();
- BatteryStatsImpl.Uid.Pkg.Serv ss = null;
- BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();
+ final ServiceRestarter res = new ServiceRestarter();
+ final BatteryStatsImpl.Uid.Pkg.Serv ss;
+ final BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();
synchronized (stats) {
ss = stats.getServiceStatsLocked(
sInfo.applicationInfo.uid, sInfo.packageName,
@@ -1426,12 +1431,14 @@
// Make sure this component isn't in the pending list.
for (int i=mPendingServices.size()-1; i>=0; i--) {
- ServiceRecord pr = mPendingServices.get(i);
+ final ServiceRecord pr = mPendingServices.get(i);
if (pr.serviceInfo.applicationInfo.uid == sInfo.applicationInfo.uid
&& pr.name.equals(name)) {
+ if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Remove pending: " + pr);
mPendingServices.remove(i);
}
}
+ if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Retrieve created new service: " + r);
}
} catch (RemoteException ex) {
// pm is in same process, this will never happen.
@@ -2119,7 +2126,28 @@
}
}
- if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent);
+ // Check to see if the service had been started as foreground, but being
+ // brought down before actually showing a notification. That is not allowed.
+ if (r.fgRequired) {
+ Slog.w(TAG_SERVICE, "Bringing down service while still waiting for start foreground: "
+ + r);
+ r.fgRequired = false;
+ r.fgWaiting = false;
+ mAm.mHandler.removeMessages(
+ ActivityManagerService.SERVICE_FOREGROUND_TIMEOUT_MSG, r);
+ if (r.app != null) {
+ Message msg = mAm.mHandler.obtainMessage(
+ ActivityManagerService.SERVICE_FOREGROUND_CRASH_MSG);
+ msg.obj = r.app;
+ mAm.mHandler.sendMessage(msg);
+ }
+ }
+
+ if (DEBUG_SERVICE) {
+ RuntimeException here = new RuntimeException();
+ here.fillInStackTrace();
+ Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent, here);
+ }
r.destroyTime = SystemClock.uptimeMillis();
if (LOG_SERVICE_START_STOP) {
EventLogTags.writeAmDestroyService(
@@ -2127,7 +2155,14 @@
}
final ServiceMap smap = getServiceMapLocked(r.userId);
- smap.mServicesByName.remove(r.name);
+ ServiceRecord found = smap.mServicesByName.remove(r.name);
+ if (found != r) {
+ // This is not actually the service we think is running... this should not happen,
+ // but if it does, fail hard.
+ smap.mServicesByName.put(r.name, found);
+ throw new IllegalStateException("Bringing down " + r + " but actually running "
+ + found);
+ }
smap.mServicesByIntent.remove(r.intent);
r.totalRestartCount = 0;
unscheduleServiceRestartLocked(r, 0, true);
@@ -2967,7 +3002,7 @@
void serviceForegroundTimeout(ServiceRecord r) {
ProcessRecord app;
synchronized (mAm) {
- if (!r.fgRequired) {
+ if (!r.fgRequired || r.destroying) {
return;
}
@@ -2985,6 +3020,11 @@
}
}
+ void serviceForegroundCrash(ProcessRecord app) {
+ mAm.crashApplication(app.uid, app.pid, app.info.packageName, app.userId,
+ "Context.startForegroundService() did not then call Service.startForeground()");
+ }
+
void scheduleServiceTimeoutLocked(ProcessRecord proc) {
if (proc.executingServices.size() == 0 || proc.thread == null) {
return;
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index fa936c2..be30e4b 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -1703,6 +1703,7 @@
static final int SERVICE_FOREGROUND_TIMEOUT_MSG = 66;
static final int DISPATCH_PENDING_INTENT_CANCEL_MSG = 67;
static final int PUSH_TEMP_WHITELIST_UI_MSG = 68;
+ static final int SERVICE_FOREGROUND_CRASH_MSG = 69;
static final int START_USER_SWITCH_FG_MSG = 712;
static final int FIRST_ACTIVITY_STACK_MSG = 100;
@@ -1974,6 +1975,9 @@
case SERVICE_FOREGROUND_TIMEOUT_MSG: {
mServices.serviceForegroundTimeout((ServiceRecord)msg.obj);
} break;
+ case SERVICE_FOREGROUND_CRASH_MSG: {
+ mServices.serviceForegroundCrash((ProcessRecord)msg.obj);
+ } break;
case DISPATCH_PENDING_INTENT_CANCEL_MSG: {
RemoteCallbackList<IResultReceiver> callbacks
= (RemoteCallbackList<IResultReceiver>)msg.obj;