From 7926fe4632484ff73a136d83635c67b74eb5923f Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Fri, 11 Nov 2022 16:06:48 -0800 Subject: [PATCH 1/3] BroadcastProcessQueue: ensure accuracy of persistent and instrumented flags Persistent apps are guaranteed to be running, are never frozen, and could get out of sync if broadcasts are deferred. Accordingly, never defer those broadcasts. Additionally, there was a case where both the persistent and instrumented flags could be incorrect: 1. App starts and hits attachApplicationLocked without ever being a broadcast target. 2. BroadcastQueueModernImpl.onApplicationAttachedLocked runs, but there is no queue for the app because nothing has enqueued a broadcast (which calls getOrCreateProcessQueue). Importantly, queue.setProcess(app) is not called. 3. Eventually, something enqueues a broadcast for the app. This calls getOrCreateProcessQueue as part of enqueueBroadcastLocked. 4. getOrCreateProcessQueue creates the queue and manually assigns app without calling setProcess. 5. mProcessInstrumented and mProcessPersistent are never assigned. The fix is to call setProcess(ProcessRecord) from getOrCreateProcessQueue when a ProcessRecord is available when a queue is created for an app. Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Test: atest CtsWifiTestCases:android.net.wifi.cts.ConcurrencyTest#testRequestNetworkInfo Bug: 258718309 Change-Id: I3059cc602b8bc75d72fe3c15dd6674f91a37a0cd --- .../server/am/BroadcastProcessQueue.java | 20 +++++++++++++++++++ .../server/am/BroadcastQueueModernImpl.java | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java index 47ca427be9ff4..739d2777fe173 100644 --- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java +++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java @@ -164,6 +164,7 @@ class BroadcastProcessQueue { private boolean mProcessCached; private boolean mProcessInstrumented; + private boolean mProcessPersistent; private String mCachedToString; private String mCachedToShortString; @@ -323,8 +324,10 @@ class BroadcastProcessQueue { this.app = app; if (app != null) { setProcessInstrumented(app.getActiveInstrumentation() != null); + setProcessPersistent(app.isPersistent()); } else { setProcessInstrumented(false); + setProcessPersistent(false); } } @@ -351,6 +354,17 @@ class BroadcastProcessQueue { } } + /** + * Update if this process is in the "persistent" state, which signals broadcast dispatch should + * bypass all pauses or delays to prevent the system from becoming out of sync with itself. + */ + public void setProcessPersistent(boolean persistent) { + if (mProcessPersistent != persistent) { + mProcessPersistent = persistent; + invalidateRunnableAt(); + } + } + /** * Return if we know of an actively running "warm" process for this queue. */ @@ -636,6 +650,7 @@ class BroadcastProcessQueue { static final int REASON_MAX_PENDING = 3; static final int REASON_BLOCKED = 4; static final int REASON_INSTRUMENTED = 5; + static final int REASON_PERSISTENT = 6; static final int REASON_CONTAINS_FOREGROUND = 10; static final int REASON_CONTAINS_ORDERED = 11; static final int REASON_CONTAINS_ALARM = 12; @@ -652,6 +667,7 @@ class BroadcastProcessQueue { REASON_MAX_PENDING, REASON_BLOCKED, REASON_INSTRUMENTED, + REASON_PERSISTENT, REASON_CONTAINS_FOREGROUND, REASON_CONTAINS_ORDERED, REASON_CONTAINS_ALARM, @@ -672,6 +688,7 @@ class BroadcastProcessQueue { case REASON_MAX_PENDING: return "MAX_PENDING"; case REASON_BLOCKED: return "BLOCKED"; case REASON_INSTRUMENTED: return "INSTRUMENTED"; + case REASON_PERSISTENT: return "PERSISTENT"; case REASON_CONTAINS_FOREGROUND: return "CONTAINS_FOREGROUND"; case REASON_CONTAINS_ORDERED: return "CONTAINS_ORDERED"; case REASON_CONTAINS_ALARM: return "CONTAINS_ALARM"; @@ -731,6 +748,9 @@ class BroadcastProcessQueue { } else if (mCountManifest > 0) { mRunnableAt = runnableAt; mRunnableAtReason = REASON_CONTAINS_MANIFEST; + } else if (mProcessPersistent) { + mRunnableAt = runnableAt; + mRunnableAtReason = REASON_PERSISTENT; } else if (mProcessCached) { mRunnableAt = runnableAt + constants.DELAY_CACHED_MILLIS; mRunnableAtReason = REASON_CACHED; diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 3dee2627e45d5..b6f05d038b45a 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -1462,7 +1462,7 @@ class BroadcastQueueModernImpl extends BroadcastQueue { } BroadcastProcessQueue created = new BroadcastProcessQueue(mConstants, processName, uid); - created.app = mService.getProcessRecordLocked(processName, uid); + created.setProcess(mService.getProcessRecordLocked(processName, uid)); if (leaf == null) { mProcessQueues.put(uid, created); From 379975d9e7d9538f2078973daa5a319d774265f1 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Fri, 11 Nov 2022 12:29:40 -0800 Subject: [PATCH 2/3] Revert "Revert "BroadcastQueue: more increasing delay knobs."" Delay works (as far as we can tell) once persistent apps are exempted. Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Test: atest CtsWifiTestCases:android.net.wifi.cts.ConcurrencyTest#testRequestNetworkInfo Bug: 258718309 Change-Id: I21e240edaca601de327e15db726af73b210607eb --- .../core/java/com/android/server/am/BroadcastConstants.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/am/BroadcastConstants.java b/services/core/java/com/android/server/am/BroadcastConstants.java index 56909e35bbe30..dfac82cbe91b8 100644 --- a/services/core/java/com/android/server/am/BroadcastConstants.java +++ b/services/core/java/com/android/server/am/BroadcastConstants.java @@ -170,7 +170,7 @@ public class BroadcastConstants { */ public long DELAY_NORMAL_MILLIS = DEFAULT_DELAY_NORMAL_MILLIS; private static final String KEY_DELAY_NORMAL_MILLIS = "bcast_delay_normal_millis"; - private static final long DEFAULT_DELAY_NORMAL_MILLIS = 0; + private static final long DEFAULT_DELAY_NORMAL_MILLIS = +500; /** * For {@link BroadcastQueueModernImpl}: Delay to apply to broadcasts @@ -178,7 +178,7 @@ public class BroadcastConstants { */ public long DELAY_CACHED_MILLIS = DEFAULT_DELAY_CACHED_MILLIS; private static final String KEY_DELAY_CACHED_MILLIS = "bcast_delay_cached_millis"; - private static final long DEFAULT_DELAY_CACHED_MILLIS = +30_000; + private static final long DEFAULT_DELAY_CACHED_MILLIS = +120_000; /** * For {@link BroadcastQueueModernImpl}: Delay to apply to urgent From f4bc936d1cc6dc8da63a5f5561fb1df9cf285d37 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Fri, 11 Nov 2022 12:23:50 -0800 Subject: [PATCH 3/3] ProcessStateRecord: add oom_adj type tracing Enable the debug flag for trace output in system_server of all OomAdjuster state transitions. Test: local trace Bug: 247573320 Change-Id: Id4519038198012ed3eb43f21a5a99f9323a592a7 --- .../com/android/server/am/ProcessStateRecord.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/core/java/com/android/server/am/ProcessStateRecord.java b/services/core/java/com/android/server/am/ProcessStateRecord.java index d2ef479ed5241..2ad2077d01bc9 100644 --- a/services/core/java/com/android/server/am/ProcessStateRecord.java +++ b/services/core/java/com/android/server/am/ProcessStateRecord.java @@ -30,6 +30,7 @@ import android.annotation.ElapsedRealtimeLong; import android.app.ActivityManager; import android.content.ComponentName; import android.os.SystemClock; +import android.os.Trace; import android.util.Slog; import android.util.TimeUtils; @@ -43,6 +44,9 @@ import java.io.PrintWriter; * The state info of the process, including proc state, oom adj score, et al. */ final class ProcessStateRecord { + // Enable this to trace all OomAdjuster state transitions + private static final boolean TRACE_OOM_ADJ = false; + private final ProcessRecord mApp; private final ActivityManagerService mService; private final ActivityManagerGlobalLock mProcLock; @@ -916,6 +920,12 @@ final class ProcessStateRecord { @GuardedBy("mService") void setAdjType(String adjType) { + if (TRACE_OOM_ADJ) { + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, + "oom:" + mApp.processName + "/u" + mApp.uid, 0); + Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, + "oom:" + mApp.processName + "/u" + mApp.uid, adjType, 0); + } mAdjType = adjType; } @@ -1153,6 +1163,10 @@ final class ProcessStateRecord { @GuardedBy({"mService", "mProcLock"}) void onCleanupApplicationRecordLSP() { + if (TRACE_OOM_ADJ) { + Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER, + "oom:" + mApp.processName + "/u" + mApp.uid, 0); + } setHasForegroundActivities(false); mHasShownUi = false; mForcingToImportant = null;