diff options
5 files changed, 304 insertions, 27 deletions
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index dd3020a9a2ad..114ed86953ef 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -33,6 +33,7 @@ import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST; import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_NONE; +import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE; import static android.os.PowerExemptionManager.REASON_ACTIVE_DEVICE_ADMIN; import static android.os.PowerExemptionManager.REASON_ACTIVITY_STARTER; import static android.os.PowerExemptionManager.REASON_ACTIVITY_VISIBILITY_GRACE_PERIOD; @@ -1286,6 +1287,8 @@ public final class ActiveServices { return; } + maybeStopShortFgsTimeoutLocked(service); + final int uid = service.appInfo.uid; final String packageName = service.name.getPackageName(); final String serviceName = service.name.getClassName(); @@ -1469,6 +1472,8 @@ public final class ActiveServices { } } + maybeStopShortFgsTimeoutLocked(r); + final int uid = r.appInfo.uid; final String packageName = r.name.getPackageName(); final String serviceName = r.name.getClassName(); @@ -1836,6 +1841,14 @@ public final class ActiveServices { + String.format("0x%08X", manifestType) + " in service element of manifest file"); } + if ((foregroundServiceType & FOREGROUND_SERVICE_TYPE_SHORT_SERVICE) != 0 + && foregroundServiceType != FOREGROUND_SERVICE_TYPE_SHORT_SERVICE) { + Slog.w(TAG_SERVICE, "startForeground(): FOREGROUND_SERVICE_TYPE_SHORT_SERVICE" + + " is combined with other types. SHORT_SERVICE will be ignored."); + // In this case, the service will be handled as a non-short, regular FGS + // anyway, so we just remove the SHORT_SERVICE type. + foregroundServiceType &= ~FOREGROUND_SERVICE_TYPE_SHORT_SERVICE; + } } boolean alreadyStartedOp = false; @@ -1886,14 +1899,21 @@ public final class ActiveServices { int fgsTypeCheckCode = FGS_TYPE_POLICY_CHECK_UNKNOWN; if (!ignoreForeground) { - // TODO(short-service): There's a known long-standing bug that allows - // a abound service to become "foreground" if setForeground() is called - // (without actually "starting" it). - // Unfortunately we can't just "fix" it because some apps are relying on it, - // but this will cause a problem to short-fgs, so we should disallow it if - // this happens and the type is SHORT_SERVICE. - // - // OTOH, if a valid short-service (which has to be "started"), happens to + if (foregroundServiceType == FOREGROUND_SERVICE_TYPE_SHORT_SERVICE + && !r.startRequested) { + // There's a long standing bug that allows a bound service to become + // a foreground service *even when it's not started*. + // Unfortunately, there are apps relying on this behavior, so we can't just + // suddenly disallow it. + // However, this would be very problematic if used with a short-FGS, so we + // explicitly disallow this combination. + // TODO(short-service): Change to another exception type? + throw new IllegalStateException( + "startForeground(SHORT_SERVICE) called on a service that's not" + + " started."); + } + + // 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 // to be stopped. if (r.mStartForegroundCount == 0) { @@ -1944,12 +1964,20 @@ public final class ActiveServices { // If the transition is _not_ allowed... keep the timeout? // // B. Short -> Short: - // This should be the same as case A - // If this is allowed, the new timeout should start. + // 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(), @@ -2106,8 +2134,11 @@ public final class ActiveServices { mAm.notifyPackageUse(r.serviceInfo.packageName, PackageManager.NOTIFY_PACKAGE_USE_FOREGROUND_SERVICE); - // TODO(short-service): Start counting a timeout. - + // Note, we'll get here if setForeground(SHORT_SERVICE) is called on a + // already short-fgs. + // In that case, because ShortFgsInfo is already set, this method + // will be noop. + maybeStartShortFgsTimeoutAndUpdateShortFgsInfoLocked(r); } else { if (DEBUG_FOREGROUND_SERVICE) { Slog.d(TAG, "Suppressing startForeground() for FAS " + r); @@ -2141,7 +2172,7 @@ public final class ActiveServices { decActiveForegroundAppLocked(smap, r); } - // TODO(short-service): Stop the timeout. (any better place to do it?) + maybeStopShortFgsTimeoutLocked(r); // Adjust notification handling before setting isForeground to false, because // that state is relevant to the notification policy side. @@ -2886,6 +2917,74 @@ public final class ActiveServices { psr.setHasReportedForegroundServices(anyForeground); } + void unscheduleShortFgsTimeoutLocked(ServiceRecord sr) { + mAm.mHandler.removeMessages(ActivityManagerService.SERVICE_SHORT_FGS_ANR_TIMEOUT_MSG, sr); + mAm.mHandler.removeMessages(ActivityManagerService.SERVICE_SHORT_FGS_TIMEOUT_MSG, sr); + } + + /** + * If {@code sr} is of a short-fgs, start a short-FGS timeout. + */ + private void maybeStartShortFgsTimeoutAndUpdateShortFgsInfoLocked(ServiceRecord sr) { + if (!sr.isShortFgs()) { + return; + } + if (DEBUG_SHORT_SERVICE) { + Slog.i(TAG_SERVICE, "Short FGS started: " + sr); + } + if (sr.hasShortFgsInfo()) { + sr.getShortFgsInfo().update(); + } else { + sr.setShortFgsInfo(SystemClock.uptimeMillis()); + } + unscheduleShortFgsTimeoutLocked(sr); // Do it just in case + + final Message msg = mAm.mHandler.obtainMessage( + ActivityManagerService.SERVICE_SHORT_FGS_TIMEOUT_MSG, sr); + mAm.mHandler.sendMessageAtTime(msg, sr.getShortFgsInfo().getTimeoutTime()); + } + + /** + * Stop the timeout for a ServiceRecord, if it's of a short-FGS. + */ + private void maybeStopShortFgsTimeoutLocked(ServiceRecord sr) { + if (DEBUG_SHORT_SERVICE) { + Slog.i(TAG_SERVICE, "Stop short FGS timeout: " + sr); + } + sr.clearShortFgsInfo(); + unscheduleShortFgsTimeoutLocked(sr); + } + + void onShortFgsTimeout(ServiceRecord sr) { + synchronized (mAm) { + if (!sr.shouldTriggerShortFgsTimeout()) { + return; + } + Slog.i(TAG_SERVICE, "Short FGS timed out: " + sr); + try { + sr.app.getThread().scheduleTimeoutService(sr, sr.getShortFgsInfo().getStartId()); + } catch (RemoteException e) { + // TODO(short-service): Anything to do here? + } + // Schedule the ANR timeout. + final Message msg = mAm.mHandler.obtainMessage( + ActivityManagerService.SERVICE_SHORT_FGS_ANR_TIMEOUT_MSG, sr); + mAm.mHandler.sendMessageAtTime(msg, sr.getShortFgsInfo().getAnrTime()); + } + } + + void onShortFgsAnrTimeout(ServiceRecord sr) { + // TODO(short-service): Implement ANR, and change the WTF to e(). + // Also make sure to call waitingOnAMSLockStarted(). + synchronized (mAm) { + if (!sr.shouldTriggerShortFgsAnr()) { + return; + } + } + + Slog.wtf(TAG_SERVICE, "Short FGS ANR'ed (NOT IMPLEMENTED YET): " + sr); + } + private void updateAllowlistManagerLocked(ProcessServiceRecord psr) { psr.mAllowlistManager = false; for (int i = psr.numberOfRunningServices() - 1; i >= 0; i--) { @@ -2897,6 +2996,7 @@ public final class ActiveServices { } } + // TODO(short-service): Hmm what is it? Should we stop the timeout here? private void stopServiceAndUpdateAllowlistManagerLocked(ServiceRecord service) { final ProcessServiceRecord psr = service.app.mServices; psr.stopService(service); @@ -4178,7 +4278,7 @@ public final class ActiveServices { /** * Reschedule service restarts based on if the extra delays are enabled or not. * - * @param prevEnable The previous state of whether or not it's enabled. + * @param prevEnabled The previous state of whether or not it's enabled. * @param curEnabled The current state of whether or not it's enabled. * @param now The uptimeMillis */ @@ -4863,13 +4963,6 @@ public final class ActiveServices { Slog.i(TAG, "Bring down service for " + debugReason + " :" + r.toString()); } - // TODO(short-service): Hmm, when the app stops a short-fgs, we should stop the timeout - // here. - // However we have a couple if's here and if these conditions are met, we stop here - // without bringing down the service. - // We need to make sure this can't be used (somehow) to keep having a short-FGS running - // while having the timeout stopped. - if (isServiceNeededLocked(r, knowConn, hasConn)) { return; } @@ -4886,6 +4979,13 @@ public final class ActiveServices { //Slog.i(TAG, "Bring down service:"); //r.dump(" "); + if (r.isShortFgs()) { + // FGS can be stopped without the app calling stopService() or stopSelf(), + // due to force-app-standby, or from Task Manager. + Slog.w(TAG_SERVICE, "Short FGS brought down without stopping: " + r); + maybeStopShortFgsTimeoutLocked(r); + } + // Report to all of the connections that the service is no longer // available. ArrayMap<IBinder, ArrayList<ConnectionRecord>> connections = r.getConnections(); diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 046403d409af..2d69667da575 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -954,7 +954,7 @@ final class ActivityManagerConstants extends ContentObserver { static final long DEFAULT_SHORT_FGS_TIMEOUT_DURATION = 60_000; /** @see #KEY_SHORT_FGS_TIMEOUT_DURATION */ - public static volatile long mShortFgsTimeoutDuration = DEFAULT_SHORT_FGS_TIMEOUT_DURATION; + public volatile long mShortFgsTimeoutDuration = DEFAULT_SHORT_FGS_TIMEOUT_DURATION; /** * If a "short service" doesn't finish within this after the timeout ( @@ -967,7 +967,7 @@ final class ActivityManagerConstants extends ContentObserver { static final long DEFAULT_SHORT_FGS_PROC_STATE_EXTRA_WAIT_DURATION = 5_000; /** @see #KEY_SHORT_FGS_PROC_STATE_EXTRA_WAIT_DURATION */ - public static volatile long mShortFgsProcStateExtraWaitDuration = + public volatile long mShortFgsProcStateExtraWaitDuration = DEFAULT_SHORT_FGS_PROC_STATE_EXTRA_WAIT_DURATION; /** @@ -983,7 +983,7 @@ final class ActivityManagerConstants extends ContentObserver { static final long DEFAULT_SHORT_FGS_ANR_EXTRA_WAIT_DURATION = 10_000; /** @see #KEY_SHORT_FGS_ANR_EXTRA_WAIT_DURATION */ - public static volatile long mShortFgsAnrExtraWaitDuration = + public volatile long mShortFgsAnrExtraWaitDuration = DEFAULT_SHORT_FGS_ANR_EXTRA_WAIT_DURATION; private final OnPropertiesChangedListener mOnDeviceConfigChangedListener = diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index c779ea9b4916..81bde919d8c1 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1563,6 +1563,8 @@ public class ActivityManagerService extends IActivityManager.Stub static final int WAIT_FOR_CONTENT_PROVIDER_TIMEOUT_MSG = 73; static final int DISPATCH_SENDING_BROADCAST_EVENT = 74; static final int DISPATCH_BINDING_SERVICE_EVENT = 75; + static final int SERVICE_SHORT_FGS_TIMEOUT_MSG = 76; + static final int SERVICE_SHORT_FGS_ANR_TIMEOUT_MSG = 77; static final int FIRST_BROADCAST_QUEUE_MSG = 200; @@ -1897,6 +1899,12 @@ public class ActivityManagerService extends IActivityManager.Stub mBindServiceEventListeners.forEach(l -> l.onBindingService((String) msg.obj, msg.arg1)); } break; + case SERVICE_SHORT_FGS_TIMEOUT_MSG: { + mServices.onShortFgsTimeout((ServiceRecord) msg.obj); + } break; + case SERVICE_SHORT_FGS_ANR_TIMEOUT_MSG: { + mServices.onShortFgsAnrTimeout((ServiceRecord) msg.obj); + } break; } } } diff --git a/services/core/java/com/android/server/am/OomAdjuster.java b/services/core/java/com/android/server/am/OomAdjuster.java index 8082e45b3238..034c1b3d55ad 100644 --- a/services/core/java/com/android/server/am/OomAdjuster.java +++ b/services/core/java/com/android/server/am/OomAdjuster.java @@ -1818,7 +1818,14 @@ public class OomAdjuster { newAdj = PERCEPTIBLE_APP_ADJ; newProcState = PROCESS_STATE_IMPORTANT_FOREGROUND; - } else if (psr.hasForegroundServices() && !psr.hasNonShortForegroundServices()) { + } 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.) + // 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) diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 0468152acdd8..4c0bd073a4de 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -315,6 +315,87 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN final ArrayList<StartItem> pendingStarts = new ArrayList<StartItem>(); // start() arguments that haven't yet been delivered. + /** + * Information specific to "SHORT_SERVICE" FGS. + */ + class ShortFgsInfo { + /** Time FGS started */ + private final long mStartTime; + + /** + * Copied from {@link #mStartForegroundCount}. If this is different from the parent's, + * that means this instance is stale. + */ + private int mStartForegroundCount; + + /** Service's "start ID" when this short-service started. */ + private int mStartId; + + ShortFgsInfo(long startTime) { + mStartTime = startTime; + update(); + } + + /** + * Update {@link #mStartForegroundCount} and {@link #mStartId}. + * (but not {@link #mStartTime}) + */ + public void update() { + this.mStartForegroundCount = ServiceRecord.this.mStartForegroundCount; + this.mStartId = getLastStartId(); + } + + long getStartTime() { + return mStartTime; + } + + int getStartForegroundCount() { + return mStartForegroundCount; + } + + int getStartId() { + return mStartId; + } + + /** + * @return whether this {@link ShortFgsInfo} is still "current" or not -- i.e. + * it's "start foreground count" is the same as that of the ServiceRecord's. + * + * Note, we do _not_ check the "start id" here, because the start id increments if the + * app calls startService() or startForegroundService() on the same service, + * but that will _not_ update the ShortFgsInfo, and will not extend the timeout. + * + * TODO(short-service): Make sure, calling startService will not extend or remove the + * timeout, in CTS. + */ + boolean isCurrent() { + return this.mStartForegroundCount == ServiceRecord.this.mStartForegroundCount; + } + + /** Time when Service.onTimeout() should be called */ + long getTimeoutTime() { + return mStartTime + ams.mConstants.mShortFgsTimeoutDuration; + } + + /** Time when the procstate should be lowered. */ + long getProcStateDemoteTime() { + return mStartTime + ams.mConstants.mShortFgsTimeoutDuration + + ams.mConstants.mShortFgsProcStateExtraWaitDuration; + } + + /** Time when the app should be declared ANR. */ + long getAnrTime() { + return mStartTime + ams.mConstants.mShortFgsTimeoutDuration + + ams.mConstants.mShortFgsAnrExtraWaitDuration; + } + } + + /** + * Keep track of short-fgs specific information. This field gets cleared when the timeout + * stops. + */ + private ShortFgsInfo mShortFgsInfo; + void dumpStartList(PrintWriter pw, String prefix, List<StartItem> list, long now) { final int N = list.size(); for (int i=0; i<N; i++) { @@ -456,6 +537,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN } } proto.end(token); + + // TODO(short-service) Add FGS info } void dump(PrintWriter pw, String prefix) { @@ -508,8 +591,25 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN } if (isForeground || foregroundId != 0) { pw.print(prefix); pw.print("isForeground="); pw.print(isForeground); - pw.print(" foregroundId="); pw.print(foregroundId); - pw.print(" foregroundNoti="); pw.println(foregroundNoti); + pw.print(" foregroundId="); pw.print(foregroundId); + pw.printf(" types=%08X", foregroundServiceType); + pw.print(" foregroundNoti="); pw.println(foregroundNoti); + + if (isShortFgs() && mShortFgsInfo != null) { + pw.print(prefix); pw.print("isShortFgs=true"); + pw.print(" startId="); pw.print(mShortFgsInfo.getStartId()); + pw.print(" startForegroundCount="); + pw.print(mShortFgsInfo.getStartForegroundCount()); + pw.print(" startTime="); + TimeUtils.formatDuration(mShortFgsInfo.getStartTime(), now, pw); + pw.print(" timeout="); + TimeUtils.formatDuration(mShortFgsInfo.getTimeoutTime(), now, pw); + pw.print(" demoteTime="); + TimeUtils.formatDuration(mShortFgsInfo.getProcStateDemoteTime(), now, pw); + pw.print(" anrTime="); + TimeUtils.formatDuration(mShortFgsInfo.getAnrTime(), now, pw); + pw.println(); + } } if (mIsFgsDelegate) { pw.print(prefix); pw.print("isFgsDelegate="); pw.println(mIsFgsDelegate); @@ -1238,4 +1338,66 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN return isForeground && (foregroundServiceType == ServiceInfo.FOREGROUND_SERVICE_TYPE_SHORT_SERVICE); } + + public ShortFgsInfo getShortFgsInfo() { + return isShortFgs() ? mShortFgsInfo : null; + } + + /** + * Call it when a short FGS starts. + */ + public void setShortFgsInfo(long uptimeNow) { + this.mShortFgsInfo = new ShortFgsInfo(uptimeNow); + } + + /** @return whether {@link #mShortFgsInfo} is set or not. */ + public boolean hasShortFgsInfo() { + return mShortFgsInfo != null; + } + + /** + * Call it when a short FGS stops. + */ + public void clearShortFgsInfo() { + this.mShortFgsInfo = null; + } + + /** + * @return true if it's a short FGS that's still up and running, and should be timed out. + */ + public boolean shouldTriggerShortFgsTimeout() { + if (!isAppAlive()) { + return false; + } + if (!this.startRequested || !isShortFgs() || mShortFgsInfo == null + || !mShortFgsInfo.isCurrent()) { + return false; + } + return mShortFgsInfo.getTimeoutTime() < SystemClock.uptimeMillis(); + } + + /** + * @return true if it's a short FGS that's still up and running, and should be declared + * an ANR. + */ + public boolean shouldTriggerShortFgsAnr() { + if (!isAppAlive()) { + return false; + } + if (!this.startRequested || !isShortFgs() || mShortFgsInfo == null + || !mShortFgsInfo.isCurrent()) { + return false; + } + return mShortFgsInfo.getAnrTime() < SystemClock.uptimeMillis(); + } + + private boolean isAppAlive() { + if (app == null) { + return false; + } + if (app.getThread() == null || app.isKilled() || app.isKilledByAm()) { + return false; + } + return true; + } } |