Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Implement SHORT_SERVICE ANR"

Browse Source
This commit is contained in:
Makoto Onuki
2022-11-28 22:28:55 +00:00
committed by Android (Google) Code Review
parent c58c817edf f273c10f59
commit 99c127730e
6 changed files with 178 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
core/java/com/android/internal/os/TimeoutRecord.java
View File

@@ -40,7 +40,8 @@ public class TimeoutRecord {
TimeoutKind.SERVICE_START,
TimeoutKind.SERVICE_EXEC,
TimeoutKind.CONTENT_PROVIDER,
TimeoutKind.APP_REGISTERED})
TimeoutKind.APP_REGISTERED,
TimeoutKind.SHORT_FGS_TIMEOUT})
@Retention(RetentionPolicy.SOURCE)
public @interface TimeoutKind {
@@ -51,6 +52,7 @@ public class TimeoutRecord {
int SERVICE_EXEC = 5;
int CONTENT_PROVIDER = 6;
int APP_REGISTERED = 7;
int SHORT_FGS_TIMEOUT = 8;
}
/** Kind of timeout, e.g. BROADCAST_RECEIVER, etc. */
@@ -144,4 +146,10 @@ public class TimeoutRecord {
public static TimeoutRecord forApp(@NonNull String reason) {
return TimeoutRecord.endingApproximatelyNow(TimeoutKind.APP_REGISTERED, reason);
}
/** Record for a "short foreground service" timeout. */
@NonNull
public static TimeoutRecord forShortFgsTimeout(String reason) {
return TimeoutRecord.endingNow(TimeoutKind.SHORT_FGS_TIMEOUT, reason);
}
}

85
services/core/java/com/android/server/am/ActiveServices.java
View File

@@ -228,7 +228,8 @@ public final class ActiveServices {
private static final boolean DEBUG_DELAYED_SERVICE = DEBUG_SERVICE;
private static final boolean DEBUG_DELAYED_STARTS = DEBUG_DELAYED_SERVICE;
private static final boolean DEBUG_SHORT_SERVICE = DEBUG_SERVICE;
// STOPSHIP(b/260012573) turn it off.
private static final boolean DEBUG_SHORT_SERVICE = true; // DEBUG_SERVICE;
private static final boolean LOG_SERVICE_START_STOP = DEBUG_SERVICE;
@@ -1912,6 +1913,36 @@ public final class ActiveServices {
"startForeground(SHORT_SERVICE) called on a service that's not"
+ " started.");
}
// If the service is already an FGS, and the type is changing, then we
// may need to do some extra work here.
if (r.isForeground && (r.foregroundServiceType != foregroundServiceType)) {
// TODO(short-service): Consider transitions:
// A. Short -> other types:
// Apply the BG restriction again. Don't just allow it.
// i.e. unless the app is in a situation where it's allowed to start
// a FGS, this transition shouldn't be allowed.
// ... But think about it more, there may be a case this should be
// allowed.
//
// If the transition is allowed, stop the timeout.
// If the transition is _not_ allowed... keep the timeout?
//
// B. Short -> Short:
// Allowed, but the timeout won't reset. The original timeout is used.
// C. Other -> short:
// This should always be allowed.
// A timeout should start.
// For now, let's just disallow transition from / to SHORT_SERVICE.
final boolean isNewTypeShortFgs =
foregroundServiceType == FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
if (r.isShortFgs() != isNewTypeShortFgs) {
// TODO(short-service): We should (probably) allow it.
throw new IllegalArgumentException(
"setForeground(): Changing foreground service type from / to "
+ " SHORT_SERVICE is now allowed");
}
}
// If a valid short-service (which has to be "started"), happens to
// also be bound, then we still _will_ apply a timeout, because it still has
@@ -1952,32 +1983,6 @@ public final class ActiveServices {
// on the same sarvice after it's created, regardless of whether
// stopForeground() has been called or not.
// TODO(short-service): Consider transitions:
// A. Short -> other types:
// Apply the BG restriction again. Don't just allow it.
// i.e. unless the app is in a situation where it's allowed to start
// a FGS, this transition shouldn't be allowed.
// ... But think about it more, there may be a case this should be
// allowed.
//
// If the transition is allowed, stop the timeout.
// If the transition is _not_ allowed... keep the timeout?
//
// B. Short -> Short:
// Allowed, but the timeout won't reset. The original timeout is used.
// C. Other -> short:
// This should always be allowed.
// A timeout should start.
// For now, let's just disallow transition from / to SHORT_SERVICE.
final boolean isNewTypeShortFgs =
foregroundServiceType == FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
if (r.isShortFgs() != isNewTypeShortFgs) {
// TODO(short-service): We should (probably) allow it.
throw new IllegalArgumentException("Changing FGS type from / to "
+ " SHORT_SERVICE is now allowed");
}
// The second or later time startForeground() is called after service is
// started. Check for app state again.
setFgsRestrictionLocked(r.serviceInfo.packageName, r.app.getPid(),
@@ -2948,6 +2953,9 @@ public final class ActiveServices {
* Stop the timeout for a ServiceRecord, if it's of a short-FGS.
*/
private void maybeStopShortFgsTimeoutLocked(ServiceRecord sr) {
if (!sr.isShortFgs()) {
return;
}
if (DEBUG_SHORT_SERVICE) {
Slog.i(TAG_SERVICE, "Stop short FGS timeout: " + sr);
}
@@ -2960,7 +2968,7 @@ public final class ActiveServices {
if (!sr.shouldTriggerShortFgsTimeout()) {
return;
}
Slog.i(TAG_SERVICE, "Short FGS timed out: " + sr);
Slog.e(TAG_SERVICE, "Short FGS timed out: " + sr);
try {
sr.app.getThread().scheduleTimeoutService(sr, sr.getShortFgsInfo().getStartId());
} catch (RemoteException e) {
@@ -2974,15 +2982,28 @@ public final class ActiveServices {
}
void onShortFgsAnrTimeout(ServiceRecord sr) {
// TODO(short-service): Implement ANR, and change the WTF to e().
// Also make sure to call waitingOnAMSLockStarted().
final String reason = "A foreground service of FOREGROUND_SERVICE_TYPE_SHORT_SERVICE"
+ " did not stop within a timeout: " + sr.getComponentName();
final TimeoutRecord tr = TimeoutRecord.forShortFgsTimeout(reason);
// TODO(short-service): TODO Add SHORT_FGS_TIMEOUT to AnrLatencyTracker
tr.mLatencyTracker.waitingOnAMSLockStarted();
synchronized (mAm) {
tr.mLatencyTracker.waitingOnAMSLockEnded();
if (!sr.shouldTriggerShortFgsAnr()) {
return;
}
}
Slog.wtf(TAG_SERVICE, "Short FGS ANR'ed (NOT IMPLEMENTED YET): " + sr);
final String message = "Short FGS ANR'ed: " + sr;
if (DEBUG_SHORT_SERVICE) {
Slog.wtf(TAG_SERVICE, message);
} else {
Slog.e(TAG_SERVICE, message);
}
mAm.appNotResponding(sr.app, tr);
}
}
private void updateAllowlistManagerLocked(ProcessServiceRecord psr) {

37
services/core/java/com/android/server/am/OomAdjuster.java
View File

@@ -1821,25 +1821,28 @@ public class OomAdjuster {
} else if (psr.hasForegroundServices()) {
// If we get here, hasNonShortForegroundServices() must be false.
// TODO(short-service): If all the short-fgs (there may be multiple) within the
// process is post proc-state-demote time, then we need to skip this part.
// ... Or, should we just ANR take care of timed-out short-FGS and shouldn't bother
// lowering the procstate / oom-adj...?? (likely not.)
// TODO(short-service): Proactively run OomAjudster when the grace period finish.
if (psr.areAllShortForegroundServicesProcstateTimedOut(now)) {
// All the short-FGSes within this process are timed out. Don't promote to FGS.
// TODO(short-service): Should we set some unique oom-adj to make it detectable,
// in a long trace?
} else {
// For short FGS.
adjType = "fg-service-short";
// We use MEDIUM_APP_ADJ + 1 so we can tell apart EJ
// (which uses MEDIUM_APP_ADJ + 1)
// from short-FGS.
// (We use +1 and +2, not +0 and +1, to be consistent with the following
// RECENT_FOREGROUND_APP_ADJ tweak)
newAdj = PERCEPTIBLE_MEDIUM_APP_ADJ + 1;
// For short FGS.
adjType = "fg-service-short";
// We use MEDIUM_APP_ADJ + 1 so we can tell apart EJ (which uses MEDIUM_APP_ADJ + 1)
// from short-FGS.
// (We use +1 and +2, not +0 and +1, to be consistent with the following
// RECENT_FOREGROUND_APP_ADJ tweak)
newAdj = PERCEPTIBLE_MEDIUM_APP_ADJ + 1;
// Short-FGS gets a below-BFGS procstate, so it can't start another FGS from it.
newProcState = PROCESS_STATE_IMPORTANT_FOREGROUND;
// Short-FGS gets a below-BFGS procstate, so it can't start another FGS from it.
newProcState = PROCESS_STATE_IMPORTANT_FOREGROUND;
// Same as EJ, we explicitly grant network access to short FGS,
// even when battery saver or data saver is enabled.
capabilityFromFGS |= PROCESS_CAPABILITY_NETWORK;
// Same as EJ, we explicitly grant network access to short FGS,
// even when battery saver or data saver is enabled.
capabilityFromFGS |= PROCESS_CAPABILITY_NETWORK;
}
}
if (adjType != null) {

30
services/core/java/com/android/server/am/ProcessServiceRecord.java
View File

@@ -175,6 +175,9 @@ final class ProcessServiceRecord {
}
}
/**
* @return true if this process has any foreground services (even timed-out short-FGS)
*/
boolean hasForegroundServices() {
return mHasForegroundServices;
}
@@ -224,6 +227,33 @@ final class ProcessServiceRecord {
return mFgServiceTypes != ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
}
/**
* @return if this process:
* - has at least one short-FGS
* - has no other types of FGS
* - and all the short-FGSes are procstate-timed out.
*/
boolean areAllShortForegroundServicesProcstateTimedOut(long nowUptime) {
if (!mHasForegroundServices) { // Process has no FGS?
return false;
}
if (hasNonShortForegroundServices()) { // Any non-short FGS running?
return false;
}
// Now we need to look at all short-FGS within the process and see if all of them are
// procstate-timed-out or not.
for (int i = mServices.size() - 1; i >= 0; i--) {
final ServiceRecord sr = mServices.valueAt(i);
if (!sr.isShortFgs() || !sr.hasShortFgsInfo()) {
continue;
}
if (sr.getShortFgsInfo().getProcStateDemoteTime() >= nowUptime) {
return false;
}
}
return true;
}
int getReportedForegroundServiceTypes() {
return mRepFgServiceTypes;
}

26
services/core/java/com/android/server/am/ServiceRecord.java
View File

@@ -690,6 +690,32 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
}
}
/** Used only for tests */
private ServiceRecord(ActivityManagerService ams) {
this.ams = ams;
name = null;
instanceName = null;
shortInstanceName = null;
definingPackageName = null;
definingUid = 0;
intent = null;
serviceInfo = null;
userId = 0;
packageName = null;
processName = null;
permission = null;
exported = false;
restarter = null;
createRealTime = 0;
isSdkSandbox = false;
sdkSandboxClientAppUid = 0;
sdkSandboxClientAppPackage = null;
}
public static ServiceRecord newEmptyInstanceForTest(ActivityManagerService ams) {
return new ServiceRecord(ams);
}
ServiceRecord(ActivityManagerService ams, ComponentName name,
ComponentName instanceName, String definingPackageName, int definingUid,
Intent.FilterComparison intent, ServiceInfo sInfo, boolean callerIsFg,

40
services/tests/mockingservicestests/src/com/android/server/am/MockingOomAdjusterTests.java
View File

@@ -439,14 +439,25 @@ public class MockingOomAdjusterTests {
@SuppressWarnings("GuardedBy")
@Test
public void testUpdateOomAdj_DoOne_FgService_ShortFgs() {
sService.mConstants.TOP_TO_FGS_GRACE_DURATION = 100_000;
sService.mConstants.mShortFgsProcStateExtraWaitDuration = 200_000;
ServiceRecord s = ServiceRecord.newEmptyInstanceForTest(sService);
s.startRequested = true;
s.isForeground = true;
s.foregroundServiceType = ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
s.setShortFgsInfo(SystemClock.uptimeMillis());
// SHORT_SERVICE FGS will get IMP_FG and a slightly different recent-adjustment.
{
ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,
MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));
app.mServices.startService(s);
app.mServices.setHasForegroundServices(true,
ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE, /* hasNoneType=*/false);
app.mState.setLastTopTime(SystemClock.uptimeMillis());
sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE);
sService.mOomAdjuster.updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_NONE);
assertProcStates(app, PROCESS_STATE_IMPORTANT_FOREGROUND,
@@ -461,9 +472,11 @@ public class MockingOomAdjusterTests {
MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));
app.mServices.setHasForegroundServices(true,
ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE, /* hasNoneType=*/false);
app.mServices.startService(s);
app.mState.setLastTopTime(SystemClock.uptimeMillis()
- sService.mConstants.TOP_TO_FGS_GRACE_DURATION);
sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE);
sService.mOomAdjuster.updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_NONE);
assertProcStates(app, PROCESS_STATE_IMPORTANT_FOREGROUND,
@@ -471,6 +484,33 @@ public class MockingOomAdjusterTests {
// Still should get network access.
assertTrue((app.mState.getSetCapability() & PROCESS_CAPABILITY_NETWORK) != 0);
}
// SHORT_SERVICE, timed out already.
s = ServiceRecord.newEmptyInstanceForTest(sService);
s.startRequested = true;
s.isForeground = true;
s.foregroundServiceType = ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE;
s.setShortFgsInfo(SystemClock.uptimeMillis()
- sService.mConstants.mShortFgsTimeoutDuration
- sService.mConstants.mShortFgsProcStateExtraWaitDuration);
{
ProcessRecord app = spy(makeDefaultProcessRecord(MOCKAPP_PID, MOCKAPP_UID,
MOCKAPP_PROCESSNAME, MOCKAPP_PACKAGENAME, true));
app.mServices.setHasForegroundServices(true,
ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE, /* hasNoneType=*/false);
app.mServices.startService(s);
app.mState.setLastTopTime(SystemClock.uptimeMillis()
- sService.mConstants.TOP_TO_FGS_GRACE_DURATION);
sService.mWakefulness.set(PowerManagerInternal.WAKEFULNESS_AWAKE);
sService.mOomAdjuster.updateOomAdjLocked(app, OomAdjuster.OOM_ADJ_REASON_NONE);
// Procstate should be lower than FGS. (It should be SERVICE)
assertEquals(app.mState.getSetProcState(), PROCESS_STATE_SERVICE);
// Shouldn't have the network capability now.
assertTrue((app.mState.getSetCapability() & PROCESS_CAPABILITY_NETWORK) == 0);
}
}
@SuppressWarnings("GuardedBy")