diff options
| -rw-r--r-- | core/java/android/content/Context.java | 6 | ||||
| -rw-r--r-- | core/java/android/content/ContextWrapper.java | 3 | ||||
| -rw-r--r-- | core/java/android/content/pm/dex/OWNERS | 5 | ||||
| -rw-r--r-- | core/java/android/hardware/OWNERS | 2 | ||||
| -rw-r--r-- | core/java/android/net/OWNERS | 2 | ||||
| -rw-r--r-- | core/jni/OWNERS | 4 | ||||
| -rw-r--r-- | core/proto/android/server/activitymanagerservice.proto | 2 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/net/OWNERS | 2 | ||||
| -rw-r--r-- | nfc/lint-baseline.xml | 55 | ||||
| -rw-r--r-- | packages/SettingsLib/OWNERS | 3 | ||||
| -rw-r--r-- | services/OWNERS | 2 | ||||
| -rw-r--r-- | services/core/java/com/android/server/am/AppExitInfoTracker.java | 417 | ||||
| -rw-r--r-- | services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java | 226 | ||||
| -rw-r--r-- | tests/BootImageProfileTest/OWNERS | 5 |
14 files changed, 485 insertions, 249 deletions
diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index c7e5d88c299d..a6eed50a594a 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -4419,7 +4419,8 @@ public abstract class Context { * @see #DISPLAY_HASH_SERVICE * @see android.view.displayhash.DisplayHashManager */ - public abstract @Nullable Object getSystemService(@ServiceName @NonNull String name); + // TODO(b/347269120): Re-add @Nullable + public abstract Object getSystemService(@ServiceName @NonNull String name); /** * Return the handle to a system-level service by class. @@ -4463,7 +4464,8 @@ public abstract class Context { * <b>never</b> throw a {@link RuntimeException} if the name is not supported. */ @SuppressWarnings("unchecked") - public final @Nullable <T> T getSystemService(@NonNull Class<T> serviceClass) { + // TODO(b/347269120): Re-add @Nullable + public final <T> T getSystemService(@NonNull Class<T> serviceClass) { // Because subclasses may override getSystemService(String) we cannot // perform a lookup by class alone. We must first map the class to its // service name then invoke the string-based method. diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index e0cf0a5f8178..a475c2925881 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -932,7 +932,8 @@ public class ContextWrapper extends Context { } @Override - public @Nullable Object getSystemService(String name) { + // TODO(b/347269120): Re-add @Nullable + public Object getSystemService(String name) { return mBase.getSystemService(name); } diff --git a/core/java/android/content/pm/dex/OWNERS b/core/java/android/content/pm/dex/OWNERS index 267e5d58f9a6..558b5f795f77 100644 --- a/core/java/android/content/pm/dex/OWNERS +++ b/core/java/android/content/pm/dex/OWNERS @@ -1,7 +1,6 @@ # Bug component: 86431 -toddke@android.com -toddke@google.com patb@google.com -calin@google.com ngeoffray@google.com +jiakaiz@google.com +mast@google.com diff --git a/core/java/android/hardware/OWNERS b/core/java/android/hardware/OWNERS index d2a2f12ec59f..51ad1519941b 100644 --- a/core/java/android/hardware/OWNERS +++ b/core/java/android/hardware/OWNERS @@ -5,7 +5,7 @@ michaelwr@google.com sumir@google.com # Camera -per-file *Camera*=cychen@google.com,epeev@google.com,etalvala@google.com,shuzhenwang@google.com,yinchiayeh@google.com,zhijunhe@google.com,jchowdhary@google.com +per-file *Camera*=cychen@google.com,epeev@google.com,etalvala@google.com,shuzhenwang@google.com,zhijunhe@google.com,jchowdhary@google.com # Sensor Privacy per-file *SensorPrivacy* = file:platform/frameworks/native:/libs/sensorprivacy/OWNERS diff --git a/core/java/android/net/OWNERS b/core/java/android/net/OWNERS index feeef55a957b..92ea0cf601fc 100644 --- a/core/java/android/net/OWNERS +++ b/core/java/android/net/OWNERS @@ -3,6 +3,6 @@ set noparent include platform/frameworks/base:/services/core/java/com/android/server/net/OWNERS per-file **IpSec* = file:/services/core/java/com/android/server/vcn/OWNERS -per-file SSL*,Uri*,Url* = prb@google.com,oth@google.com,narayan@google.com,ngeoffray@google.com +per-file SSL*,Uri*,Url* = prb@google.com,sorinbasca@google.com,narayan@google.com,ngeoffray@google.com per-file SntpClient* = file:/services/core/java/com/android/server/timedetector/OWNERS per-file Uri.java,Uri.aidl = varunshah@google.com diff --git a/core/jni/OWNERS b/core/jni/OWNERS index 1a27367a2b06..30ce63cfc744 100644 --- a/core/jni/OWNERS +++ b/core/jni/OWNERS @@ -2,7 +2,7 @@ per-file *Camera*,*camera* = file:platform/frameworks/av:/camera/OWNERS # Connectivity -per-file android_net_* = codewiz@google.com, jchalard@google.com, lorenzo@google.com, reminv@google.com, satk@google.com +per-file android_net_* = jchalard@google.com, lorenzo@google.com, reminv@google.com, satk@google.com # Choreographer per-file android_view_DisplayEventReceiver* = file:platform/frameworks/native:/services/surfaceflinger/OWNERS @@ -83,7 +83,7 @@ per-file android_text_* = file:/core/java/android/text/OWNERS # These are highly common-use files per-file Android.bp = file:/graphics/java/android/graphics/OWNERS per-file AndroidRuntime.cpp = file:/graphics/java/android/graphics/OWNERS -per-file AndroidRuntime.cpp = calin@google.com, ngeoffray@google.com, oth@google.com +per-file AndroidRuntime.cpp = file:platform/art:main:/OWNERS # Although marked "view" this is mostly graphics stuff per-file android_view_* = file:/graphics/java/android/graphics/OWNERS # File used for Android Studio layoutlib diff --git a/core/proto/android/server/activitymanagerservice.proto b/core/proto/android/server/activitymanagerservice.proto index 75cfba02120e..121f348bccd4 100644 --- a/core/proto/android/server/activitymanagerservice.proto +++ b/core/proto/android/server/activitymanagerservice.proto @@ -1037,7 +1037,7 @@ message AppsExitInfoProto { optional int32 uid = 1; repeated .android.app.ApplicationExitInfoProto app_exit_info = 2; - repeated .android.app.ApplicationExitInfoProto app_recoverable_crash = 3; + repeated .android.app.ApplicationExitInfoProto app_recoverable_crash = 3 [deprecated=true]; } repeated User users = 2; } diff --git a/core/tests/coretests/src/android/net/OWNERS b/core/tests/coretests/src/android/net/OWNERS index beb77dc8f4fd..831f444c6ed4 100644 --- a/core/tests/coretests/src/android/net/OWNERS +++ b/core/tests/coretests/src/android/net/OWNERS @@ -1,5 +1,5 @@ include /services/core/java/com/android/server/net/OWNERS -per-file SSL*,Url* = prb@google.com,oth@google.com,narayan@google.com,ngeoffray@google.com +per-file SSL*,Url* = file:platform/libcore:main:/OWNERS_net_tests per-file SntpClient* = file:/services/core/java/com/android/server/timedetector/OWNERS per-file Uri* = varunshah@google.com diff --git a/nfc/lint-baseline.xml b/nfc/lint-baseline.xml index 1dfdd29e480a..d0f797e5c6b8 100644 --- a/nfc/lint-baseline.xml +++ b/nfc/lint-baseline.xml @@ -210,4 +210,59 @@ column="23"/> </issue> + <issue + id="FlaggedApi" + message="Method `PollingFrame()` is a flagged API and should be inside an `if (Flags.nfcReadPollingLoop())` check (or annotate the surrounding method `handleMessage` with `@FlaggedApi(Flags.FLAG_NFC_READ_POLLING_LOOP) to transfer requirement to caller`)" + errorLine1=" pollingFrames.add(new PollingFrame(frame));" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~"> + <location + file="frameworks/base/nfc/java/android/nfc/cardemulation/HostApduService.java" + line="335" + column="43"/> + </issue> + + <issue + id="FlaggedApi" + message="Method `processPollingFrames()` is a flagged API and should be inside an `if (Flags.nfcReadPollingLoop())` check (or annotate the surrounding method `handleMessage` with `@FlaggedApi(Flags.FLAG_NFC_READ_POLLING_LOOP) to transfer requirement to caller`)" + errorLine1=" processPollingFrames(pollingFrames);" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> + <location + file="frameworks/base/nfc/java/android/nfc/cardemulation/HostApduService.java" + line="337" + column="21"/> + </issue> + + <issue + id="FlaggedApi" + message="Method `NfcOemExtension()` is a flagged API and should be inside an `if (Flags.nfcOemExtension())` check (or annotate the surrounding method `NfcAdapter` with `@FlaggedApi(Flags.FLAG_NFC_OEM_EXTENSION) to transfer requirement to caller`)" + errorLine1=" mNfcOemExtension = new NfcOemExtension(mContext, this);" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> + <location + file="frameworks/base/nfc/java/android/nfc/NfcAdapter.java" + line="895" + column="28"/> + </issue> + + <issue + id="FlaggedApi" + message="Method `onVendorNciResponse()` is a flagged API and should be inside an `if (Flags.nfcVendorCmd())` check (or annotate the surrounding method `onVendorResponseReceived` with `@FlaggedApi(Flags.FLAG_NFC_VENDOR_CMD) to transfer requirement to caller`)" + errorLine1=" executor.execute(() -> callback.onVendorNciResponse(gid, oid, payload));" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> + <location + file="frameworks/base/nfc/java/android/nfc/NfcVendorNciCallbackListener.java" + line="88" + column="44"/> + </issue> + + <issue + id="FlaggedApi" + message="Method `onVendorNciNotification()` is a flagged API and should be inside an `if (Flags.nfcVendorCmd())` check (or annotate the surrounding method `onVendorNotificationReceived` with `@FlaggedApi(Flags.FLAG_NFC_VENDOR_CMD) to transfer requirement to caller`)" + errorLine1=" executor.execute(() -> callback.onVendorNciNotification(gid, oid, payload));" + errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> + <location + file="frameworks/base/nfc/java/android/nfc/NfcVendorNciCallbackListener.java" + line="106" + column="44"/> + </issue> + </issues>
\ No newline at end of file diff --git a/packages/SettingsLib/OWNERS b/packages/SettingsLib/OWNERS index 5966c9f759fb..62ed66cdce67 100644 --- a/packages/SettingsLib/OWNERS +++ b/packages/SettingsLib/OWNERS @@ -11,3 +11,6 @@ ykhung@google.com # Exempt resource files (because they are in a flat directory and too hard to manage via OWNERS) per-file *.xml=* + +# Notification-related utilities +per-file */notification/* = file:/packages/SystemUI/src/com/android/systemui/statusbar/notification/OWNERS diff --git a/services/OWNERS b/services/OWNERS index 3ce5ae170329..69e4e24f8fa3 100644 --- a/services/OWNERS +++ b/services/OWNERS @@ -1,7 +1,7 @@ per-file Android.bp = file:platform/build/soong:/OWNERS #{LAST_RESORT_SUGGESTION} # art-team@ manages the system server profile -per-file art-profile* = calin@google.com, ngeoffray@google.com, vmarko@google.com +per-file art-profile* = file:platform/art:main:/OWNERS_boot_profile per-file java/com/android/server/HsumBootUserInitializer.java = file:/MULTIUSER_OWNERS diff --git a/services/core/java/com/android/server/am/AppExitInfoTracker.java b/services/core/java/com/android/server/am/AppExitInfoTracker.java index 666e5600a8b6..47b65eb885ab 100644 --- a/services/core/java/com/android/server/am/AppExitInfoTracker.java +++ b/services/core/java/com/android/server/am/AppExitInfoTracker.java @@ -88,6 +88,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.zip.GZIPOutputStream; @@ -104,9 +105,9 @@ public final class AppExitInfoTracker { private static final long APP_EXIT_INFO_PERSIST_INTERVAL = TimeUnit.MINUTES.toMillis(30); /** These are actions that the forEach* should take after each iteration */ - private static final int FOREACH_ACTION_NONE = 0; - private static final int FOREACH_ACTION_REMOVE_ITEM = 1; - private static final int FOREACH_ACTION_STOP_ITERATION = 2; + @VisibleForTesting static final int FOREACH_ACTION_NONE = 0; + @VisibleForTesting static final int FOREACH_ACTION_REMOVE_ITEM = 1; + @VisibleForTesting static final int FOREACH_ACTION_STOP_ITERATION = 2; private static final int APP_EXIT_RAW_INFO_POOL_SIZE = 8; @@ -125,7 +126,7 @@ public final class AppExitInfoTracker { private static final String APP_TRACE_FILE_SUFFIX = ".gz"; - private final Object mLock = new Object(); + @VisibleForTesting final Object mLock = new Object(); /** * Initialized in {@link #init} and read-only after that. @@ -410,6 +411,23 @@ public final class AppExitInfoTracker { } /** + * Certain types of crashes should not be updated. This could end up deleting valuable + * information, for example, if a test application crashes and then the `am instrument` + * finishes, then the crash whould be replaced with a `reason == USER_REQUESTED` + * ApplicationExitInfo from ActivityManager, and the original crash would be lost. + */ + private boolean preventExitInfoUpdate(final ApplicationExitInfo exitInfo) { + switch (exitInfo.getReason()) { + case ApplicationExitInfo.REASON_ANR: + case ApplicationExitInfo.REASON_CRASH: + case ApplicationExitInfo.REASON_CRASH_NATIVE: + return true; + default: + return false; + } + } + + /** * Make note when ActivityManagerService decides to kill an application process. */ @VisibleForTesting @@ -418,10 +436,10 @@ public final class AppExitInfoTracker { ApplicationExitInfo info = getExitInfoLocked( raw.getPackageName(), raw.getPackageUid(), raw.getPid()); - if (info == null) { + if (info == null || preventExitInfoUpdate(info)) { info = addExitInfoLocked(raw); } else { - // always override the existing info since we are now more informational. + // Override the existing info since we have more information. info.setReason(raw.getReason()); info.setSubReason(raw.getSubReason()); info.setStatus(0); @@ -431,24 +449,8 @@ public final class AppExitInfoTracker { scheduleLogToStatsdLocked(info, true); } - /** - * Make note when ActivityManagerService gets a recoverable native crash, as the process isn't - * being killed but the crash should still be added to AppExitInfo. Also, because we're not - * crashing, don't log out to statsd. - */ - @VisibleForTesting - @GuardedBy("mLock") - void handleNoteAppRecoverableCrashLocked(final ApplicationExitInfo raw) { - addExitInfoLocked(raw, /* recoverable */ true); - } - @GuardedBy("mLock") private ApplicationExitInfo addExitInfoLocked(ApplicationExitInfo raw) { - return addExitInfoLocked(raw, /* recoverable */ false); - } - - @GuardedBy("mLock") - private ApplicationExitInfo addExitInfoLocked(ApplicationExitInfo raw, boolean recoverable) { if (!mAppExitInfoLoaded.get()) { Slog.w(TAG, "Skipping saving the exit info due to ongoing loading from storage"); return null; @@ -464,13 +466,13 @@ public final class AppExitInfoTracker { } } for (int i = 0; i < packages.length; i++) { - addExitInfoInnerLocked(packages[i], uid, info, recoverable); + addExitInfoInnerLocked(packages[i], uid, info); } // SDK sandbox exits are stored under both real and package UID if (Process.isSdkSandboxUid(uid)) { for (int i = 0; i < packages.length; i++) { - addExitInfoInnerLocked(packages[i], raw.getPackageUid(), info, recoverable); + addExitInfoInnerLocked(packages[i], raw.getPackageUid(), info); } } @@ -526,72 +528,77 @@ public final class AppExitInfoTracker { if (k != null) { uid = k; } - ArrayList<ApplicationExitInfo> tlist = mTmpInfoList; - tlist.clear(); final int targetUid = uid; + // Launder the modification bit through a `final` array, as Java doesn't allow you to mutate + // a captured boolean inside of a lambda. + final boolean[] isModified = {false}; forEachPackageLocked((packageName, records) -> { AppExitInfoContainer container = records.get(targetUid); if (container == null) { return FOREACH_ACTION_NONE; } - tlist.clear(); - container.getExitInfoLocked(pid, 1, tlist); - if (tlist.size() == 0) { - return FOREACH_ACTION_NONE; - } - ApplicationExitInfo info = tlist.get(0); - if (info.getRealUid() != targetUid) { - tlist.clear(); + mTmpInfoList.clear(); + container.getExitInfosLocked(pid, /* maxNum */ 0, mTmpInfoList); + if (mTmpInfoList.size() == 0) { return FOREACH_ACTION_NONE; } - // Okay found it, update its reason. - updateExistingExitInfoRecordLocked(info, status, reason); - return FOREACH_ACTION_STOP_ITERATION; + for (int i = 0, size = mTmpInfoList.size(); i < size; i++) { + ApplicationExitInfo info = mTmpInfoList.get(i); + if (info.getRealUid() != targetUid) { + continue; + } + // We only update the most recent `ApplicationExitInfo` for this pid, which will + // always be the first one we se as `getExitInfosLocked()` returns them sorted + // by most-recent-first. + isModified[0] = true; + updateExistingExitInfoRecordLocked(info, status, reason); + return FOREACH_ACTION_STOP_ITERATION; + } + return FOREACH_ACTION_NONE; }); - return tlist.size() > 0; + mTmpInfoList.clear(); + return isModified[0]; } /** * Get the exit info with matching package name, filterUid and filterPid (if > 0) */ @VisibleForTesting - void getExitInfo(final String packageName, final int filterUid, - final int filterPid, final int maxNum, final ArrayList<ApplicationExitInfo> results) { + void getExitInfo(final String packageName, final int filterUid, final int filterPid, + final int maxNum, final List<ApplicationExitInfo> results) { final long identity = Binder.clearCallingIdentity(); try { synchronized (mLock) { - boolean emptyPackageName = TextUtils.isEmpty(packageName); - if (!emptyPackageName) { - // fast path + if (!TextUtils.isEmpty(packageName)) { + // Fast path - just a single package. AppExitInfoContainer container = mData.get(packageName, filterUid); if (container != null) { - container.getExitInfoLocked(filterPid, maxNum, results); + container.getExitInfosLocked(filterPid, maxNum, results); } - } else { - // slow path - final ArrayList<ApplicationExitInfo> list = mTmpInfoList2; - list.clear(); - // get all packages - forEachPackageLocked((name, records) -> { - AppExitInfoContainer container = records.get(filterUid); - if (container != null) { - mTmpInfoList.clear(); - list.addAll(container.toListLocked(mTmpInfoList, filterPid)); - } - return AppExitInfoTracker.FOREACH_ACTION_NONE; - }); - - Collections.sort(list, - (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); - int size = list.size(); - if (maxNum > 0) { - size = Math.min(size, maxNum); - } - for (int i = 0; i < size; i++) { - results.add(list.get(i)); + return; + } + + // Slow path - get all the packages. + forEachPackageLocked((name, records) -> { + AppExitInfoContainer container = records.get(filterUid); + if (container != null) { + container.getExitInfosLocked(filterPid, /* maxNum */ 0, results); } - list.clear(); + return AppExitInfoTracker.FOREACH_ACTION_NONE; + }); + + // And while the results for each package are sorted, we should + // sort over and trim the quantity of global results as well. + Collections.sort( + results, (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); + if (maxNum <= 0) { + return; + } + + int elementsToRemove = results.size() - maxNum; + for (int i = 0; i < elementsToRemove; i++) { + results.removeLast(); } } } finally { @@ -606,12 +613,10 @@ public final class AppExitInfoTracker { @GuardedBy("mLock") private ApplicationExitInfo getExitInfoLocked(final String packageName, final int filterUid, final int filterPid) { - ArrayList<ApplicationExitInfo> list = mTmpInfoList; - list.clear(); - getExitInfo(packageName, filterUid, filterPid, 1, list); - - ApplicationExitInfo info = list.size() > 0 ? list.get(0) : null; - list.clear(); + mTmpInfoList.clear(); + getExitInfo(packageName, filterUid, filterPid, 1, mTmpInfoList); + ApplicationExitInfo info = mTmpInfoList.size() > 0 ? mTmpInfoList.getFirst() : null; + mTmpInfoList.clear(); return info; } @@ -878,8 +883,7 @@ public final class AppExitInfoTracker { } @GuardedBy("mLock") - private void addExitInfoInnerLocked(String packageName, int uid, ApplicationExitInfo info, - boolean recoverable) { + private void addExitInfoInnerLocked(String packageName, int uid, ApplicationExitInfo info) { AppExitInfoContainer container = mData.get(packageName, uid); if (container == null) { container = new AppExitInfoContainer(mAppExitInfoHistoryListSize); @@ -893,11 +897,7 @@ public final class AppExitInfoTracker { } mData.put(packageName, uid, container); } - if (recoverable) { - container.addRecoverableCrashLocked(info); - } else { - container.addExitInfoLocked(info); - } + container.addExitInfoLocked(info); } @GuardedBy("mLock") @@ -1205,7 +1205,7 @@ public final class AppExitInfoTracker { forEachPackageLocked((name, records) -> { for (int i = records.size() - 1; i >= 0; i--) { final AppExitInfoContainer container = records.valueAt(i); - container.forEachRecordLocked((pid, info) -> { + container.forEachRecordLocked((info) -> { final File traceFile = info.getTraceFile(); if (traceFile != null) { allFiles.remove(traceFile.getName()); @@ -1322,90 +1322,72 @@ public final class AppExitInfoTracker { * A container class of {@link android.app.ApplicationExitInfo} */ final class AppExitInfoContainer { - private SparseArray<ApplicationExitInfo> mInfos; // index is a pid - private SparseArray<ApplicationExitInfo> mRecoverableCrashes; // index is a pid + private ArrayList<ApplicationExitInfo> mExitInfos; private int mMaxCapacity; private int mUid; // Application uid, not isolated uid. AppExitInfoContainer(final int maxCapacity) { - mInfos = new SparseArray<ApplicationExitInfo>(); - mRecoverableCrashes = new SparseArray<ApplicationExitInfo>(); + mExitInfos = new ArrayList<ApplicationExitInfo>(); mMaxCapacity = maxCapacity; } + @VisibleForTesting @GuardedBy("mLock") - void getInfosLocked(SparseArray<ApplicationExitInfo> map, final int filterPid, - final int maxNum, ArrayList<ApplicationExitInfo> results) { - if (filterPid > 0) { - ApplicationExitInfo r = map.get(filterPid); - if (r != null) { - results.add(r); + void getExitInfosLocked( + final int filterPid, final int maxNum, List<ApplicationExitInfo> results) { + if (mExitInfos.size() == 0) { + return; + } + + // Most of the callers might only be interested with the most recent + // ApplicationExitInfo, and so we can special case an O(n) walk. + if (maxNum == 1) { + ApplicationExitInfo result = null; + for (int i = 0, size = mExitInfos.size(); i < size; i++) { + ApplicationExitInfo info = mExitInfos.get(i); + if (filterPid > 0 && info.getPid() != filterPid) { + continue; + } + + if (result == null || result.getTimestamp() < info.getTimestamp()) { + result = info; + } } + if (result != null) { + results.add(result); + } + return; + } + + mTmpInfoList2.clear(); + if (filterPid <= 0) { + mTmpInfoList2.addAll(mExitInfos); } else { - final int numRep = map.size(); - if (maxNum <= 0 || numRep <= maxNum) { - // Return all records. - for (int i = 0; i < numRep; i++) { - results.add(map.valueAt(i)); - } - Collections.sort(results, - (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); - } else { - if (maxNum == 1) { - // Most of the caller might be only interested with the most recent one - ApplicationExitInfo r = map.valueAt(0); - for (int i = 1; i < numRep; i++) { - ApplicationExitInfo t = map.valueAt(i); - if (r.getTimestamp() < t.getTimestamp()) { - r = t; - } - } - results.add(r); - } else { - // Huh, need to sort it out then. - ArrayList<ApplicationExitInfo> list = mTmpInfoList2; - list.clear(); - for (int i = 0; i < numRep; i++) { - list.add(map.valueAt(i)); - } - Collections.sort(list, - (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); - for (int i = 0; i < maxNum; i++) { - results.add(list.get(i)); - } - list.clear(); + for (int i = 0, size = mExitInfos.size(); i < size; i++) { + ApplicationExitInfo info = mExitInfos.get(i); + if (info.getPid() == filterPid) { + mTmpInfoList2.add(info); } } } - } - @GuardedBy("mLock") - void getExitInfoLocked(final int filterPid, final int maxNum, - ArrayList<ApplicationExitInfo> results) { - getInfosLocked(mInfos, filterPid, maxNum, results); + Collections.sort( + mTmpInfoList2, (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); + if (maxNum <= 0) { + results.addAll(mTmpInfoList2); + return; + } + + int elementsToRemove = mTmpInfoList2.size() - maxNum; + for (int i = 0; i < elementsToRemove; i++) { + mTmpInfoList2.removeLast(); + } + results.addAll(mTmpInfoList2); } + @VisibleForTesting @GuardedBy("mLock") - void addInfoLocked(SparseArray<ApplicationExitInfo> map, ApplicationExitInfo info) { - int size; - if ((size = map.size()) >= mMaxCapacity) { - int oldestIndex = -1; - long oldestTimeStamp = Long.MAX_VALUE; - for (int i = 0; i < size; i++) { - ApplicationExitInfo r = map.valueAt(i); - if (r.getTimestamp() < oldestTimeStamp) { - oldestTimeStamp = r.getTimestamp(); - oldestIndex = i; - } - } - if (oldestIndex >= 0) { - final File traceFile = map.valueAt(oldestIndex).getTraceFile(); - if (traceFile != null) { - traceFile.delete(); - } - map.removeAt(oldestIndex); - } - } + void addExitInfoLocked(ApplicationExitInfo info) { // Claim the state information if there is any int uid = info.getPackageUid(); // SDK sandbox app states and app traces are stored under real UID @@ -1420,24 +1402,39 @@ public final class AppExitInfoTracker { if (info.getTraceFile() == null) { info.setTraceFile(findAndRemoveFromSparse2dArray(mActiveAppTraces, uid, pid)); } - info.setAppTraceRetriever(mAppTraceRetriever); - map.append(pid, info); - } - @GuardedBy("mLock") - void addExitInfoLocked(ApplicationExitInfo info) { - addInfoLocked(mInfos, info); + mExitInfos.add(info); + if (mExitInfos.size() <= mMaxCapacity) { + return; + } + + ApplicationExitInfo oldest = null; + for (int i = 0, size = mExitInfos.size(); i < size; i++) { + ApplicationExitInfo info2 = mExitInfos.get(i); + if (oldest == null || info2.getTimestamp() < oldest.getTimestamp()) { + oldest = info2; + } + } + File traceFile = oldest.getTraceFile(); + if (traceFile != null) { + traceFile.delete(); + } + mExitInfos.remove(oldest); } @GuardedBy("mLock") - void addRecoverableCrashLocked(ApplicationExitInfo info) { - addInfoLocked(mRecoverableCrashes, info); + ApplicationExitInfo getLastExitInfoForPid(final int pid) { + mTmpInfoList.clear(); + getExitInfosLocked(pid, /* maxNum */ 1, mTmpInfoList); + ApplicationExitInfo info = mTmpInfoList.size() == 0 ? null : mTmpInfoList.getFirst(); + mTmpInfoList.clear(); + return info; } @GuardedBy("mLock") boolean appendTraceIfNecessaryLocked(final int pid, final File traceFile) { - final ApplicationExitInfo r = mInfos.get(pid); + final ApplicationExitInfo r = getLastExitInfoForPid(pid); if (r != null) { r.setTraceFile(traceFile); r.setAppTraceRetriever(mAppTraceRetriever); @@ -1447,49 +1444,36 @@ public final class AppExitInfoTracker { } @GuardedBy("mLock") - void destroyLocked(SparseArray<ApplicationExitInfo> map) { - for (int i = map.size() - 1; i >= 0; i--) { - ApplicationExitInfo ai = map.valueAt(i); - final File traceFile = ai.getTraceFile(); + void destroyLocked() { + for (int i = 0, size = mExitInfos.size(); i < size; i++) { + ApplicationExitInfo info = mExitInfos.get(i); + final File traceFile = info.getTraceFile(); if (traceFile != null) { traceFile.delete(); } - ai.setTraceFile(null); - ai.setAppTraceRetriever(null); + info.setTraceFile(null); + info.setAppTraceRetriever(null); } } + /** + * Go through each record in an *unspecified* order, execute `callback()` on each element, + * and potentially do some action (stopping iteration, removing the element, etc.) based on + * the return value of the callback. + */ @GuardedBy("mLock") - void destroyLocked() { - destroyLocked(mInfos); - destroyLocked(mRecoverableCrashes); - } - - @GuardedBy("mLock") - void forEachRecordLocked(final BiFunction<Integer, ApplicationExitInfo, Integer> callback) { + void forEachRecordLocked(final Function<ApplicationExitInfo, Integer> callback) { if (callback == null) return; - for (int i = mInfos.size() - 1; i >= 0; i--) { - switch (callback.apply(mInfos.keyAt(i), mInfos.valueAt(i))) { + for (int i = mExitInfos.size() - 1; i >= 0; i--) { + ApplicationExitInfo info = mExitInfos.get(i); + switch (callback.apply(info)) { case FOREACH_ACTION_STOP_ITERATION: return; case FOREACH_ACTION_REMOVE_ITEM: - final File traceFile = mInfos.valueAt(i).getTraceFile(); - if (traceFile != null) { + File traceFile; + if ((traceFile = info.getTraceFile()) != null) { traceFile.delete(); } - mInfos.removeAt(i); - break; - } - } - for (int i = mRecoverableCrashes.size() - 1; i >= 0; i--) { - switch (callback.apply( - mRecoverableCrashes.keyAt(i), mRecoverableCrashes.valueAt(i))) { - case FOREACH_ACTION_STOP_ITERATION: return; - case FOREACH_ACTION_REMOVE_ITEM: - final File traceFile = mRecoverableCrashes.valueAt(i).getTraceFile(); - if (traceFile != null) { - traceFile.delete(); - } - mRecoverableCrashes.removeAt(i); + mExitInfos.remove(info); break; } } @@ -1497,30 +1481,20 @@ public final class AppExitInfoTracker { @GuardedBy("mLock") void dumpLocked(PrintWriter pw, String prefix, SimpleDateFormat sdf) { - ArrayList<ApplicationExitInfo> list = new ArrayList<ApplicationExitInfo>(); - for (int i = mInfos.size() - 1; i >= 0; i--) { - list.add(mInfos.valueAt(i)); - } - for (int i = mRecoverableCrashes.size() - 1; i >= 0; i--) { - list.add(mRecoverableCrashes.valueAt(i)); - } - Collections.sort(list, (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp())); - int size = list.size(); - for (int i = 0; i < size; i++) { - list.get(i).dump(pw, prefix + " ", "#" + i, sdf); + mTmpInfoList.clear(); + getExitInfosLocked(/* filterPid */ 0, /* maxNum */ 0, mTmpInfoList); + for (int i = 0, size = mTmpInfoList.size(); i < size; i++) { + mTmpInfoList.get(i).dump(pw, prefix + " ", "#" + i, sdf); } + mTmpInfoList.clear(); } @GuardedBy("mLock") void writeToProto(ProtoOutputStream proto, long fieldId) { long token = proto.start(fieldId); proto.write(AppsExitInfoProto.Package.User.UID, mUid); - for (int i = 0; i < mInfos.size(); i++) { - mInfos.valueAt(i).writeToProto(proto, AppsExitInfoProto.Package.User.APP_EXIT_INFO); - } - for (int i = 0; i < mRecoverableCrashes.size(); i++) { - mRecoverableCrashes.valueAt(i).writeToProto( - proto, AppsExitInfoProto.Package.User.APP_RECOVERABLE_CRASH); + for (int i = 0, size = mExitInfos.size(); i < size; i++) { + mExitInfos.get(i).writeToProto(proto, AppsExitInfoProto.Package.User.APP_EXIT_INFO); } proto.end(token); } @@ -1539,14 +1513,7 @@ public final class AppExitInfoTracker { case (int) AppsExitInfoProto.Package.User.APP_EXIT_INFO: { ApplicationExitInfo info = new ApplicationExitInfo(); info.readFromProto(proto, AppsExitInfoProto.Package.User.APP_EXIT_INFO); - mInfos.put(info.getPid(), info); - break; - } - case (int) AppsExitInfoProto.Package.User.APP_RECOVERABLE_CRASH: { - ApplicationExitInfo info = new ApplicationExitInfo(); - info.readFromProto( - proto, AppsExitInfoProto.Package.User.APP_RECOVERABLE_CRASH); - mRecoverableCrashes.put(info.getPid(), info); + mExitInfos.add(info); break; } } @@ -1554,24 +1521,6 @@ public final class AppExitInfoTracker { proto.end(token); return mUid; } - - @GuardedBy("mLock") - List<ApplicationExitInfo> toListLocked(List<ApplicationExitInfo> list, int filterPid) { - if (list == null) { - list = new ArrayList<ApplicationExitInfo>(); - } - for (int i = mInfos.size() - 1; i >= 0; i--) { - if (filterPid == 0 || filterPid == mInfos.keyAt(i)) { - list.add(mInfos.valueAt(i)); - } - } - for (int i = mRecoverableCrashes.size() - 1; i >= 0; i--) { - if (filterPid == 0 || filterPid == mRecoverableCrashes.keyAt(i)) { - list.add(mRecoverableCrashes.valueAt(i)); - } - } - return list; - } } /** @@ -1750,7 +1699,11 @@ public final class AppExitInfoTracker { case MSG_APP_RECOVERABLE_CRASH: { ApplicationExitInfo raw = (ApplicationExitInfo) msg.obj; synchronized (mLock) { - handleNoteAppRecoverableCrashLocked(raw); + // Unlike MSG_APP_KILL, this is a recoverable crash, and + // so we want to bypass the statsd app-kill logging. + // Hence, call `addExitInfoLocked()` directly instead of + // `handleNoteAppKillLocked()`. + addExitInfoLocked(raw); } recycleRawRecord(raw); } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java b/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java index e15942bb8f9a..adcbf5c9d059 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java @@ -84,7 +84,11 @@ import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; import java.util.Random; +import java.util.function.Function; import java.util.zip.GZIPInputStream; /** @@ -940,6 +944,228 @@ public class ApplicationExitInfoTest { } } + private ApplicationExitInfo createExitInfo(int i) { + ApplicationExitInfo info = new ApplicationExitInfo(); + info.setPid(i); + info.setTimestamp(1000 + i); + info.setPackageUid(2000); + return info; + } + + @SuppressWarnings("GuardedBy") + private ArrayList<ApplicationExitInfo> getExitInfosHelper( + AppExitInfoTracker.AppExitInfoContainer container, int filterPid, int maxNum) { + ArrayList<ApplicationExitInfo> infos = new ArrayList<ApplicationExitInfo>(); + container.getExitInfosLocked(filterPid, maxNum, infos); + return infos; + } + + @SuppressWarnings("GuardedBy") + private void checkAreHelper(AppExitInfoTracker.AppExitInfoContainer container, int filterPid, + int maxNum, List<Integer> expected, Function<ApplicationExitInfo, Integer> func) { + ArrayList<Integer> values = new ArrayList<Integer>(); + getExitInfosHelper(container, filterPid, maxNum) + .forEach((exitInfo) -> values.add(func.apply(exitInfo))); + assertEquals(values, expected); + + HashMap<Integer, Integer> expectedMultiset = new HashMap<Integer, Integer>(); + expected.forEach( + (elem) -> expectedMultiset.put(elem, expectedMultiset.getOrDefault(elem, 0) + 1)); + // `maxNum` isn't a parameter supported by `forEachRecordLocked()s`, but we can emulate it + // by stopping iteration when we've seen enough elements. + int[] numElementsToObserveWrapped = {maxNum}; + container.forEachRecordLocked((exitInfo) -> { + // Same thing as above, `filterPid` isn't a parameter supported out of the box for + // `forEachRecordLocked()`, but we emulate it here. + if (filterPid > 0 && filterPid != exitInfo.getPid()) { + return AppExitInfoTracker.FOREACH_ACTION_NONE; + } + + Integer key = func.apply(exitInfo); + assertTrue(expectedMultiset.toString(), expectedMultiset.containsKey(key)); + Integer references = expectedMultiset.get(key); + if (references == 1) { + expectedMultiset.remove(key); + } else { + expectedMultiset.put(key, references - 1); + } + if (--numElementsToObserveWrapped[0] == 0) { + return AppExitInfoTracker.FOREACH_ACTION_STOP_ITERATION; + } + return AppExitInfoTracker.FOREACH_ACTION_NONE; + }); + assertEquals(expectedMultiset.size(), 0); + } + + private void checkPidsAre(AppExitInfoTracker.AppExitInfoContainer container, int filterPid, + int maxNum, List<Integer> expectedPids) { + checkAreHelper(container, filterPid, maxNum, expectedPids, (exitInfo) -> exitInfo.getPid()); + } + + private void checkPidsAre( + AppExitInfoTracker.AppExitInfoContainer container, List<Integer> expectedPids) { + checkPidsAre(container, 0, 0, expectedPids); + } + + private void checkTimestampsAre(AppExitInfoTracker.AppExitInfoContainer container, + int filterPid, int maxNum, List<Integer> expectedTimestamps) { + checkAreHelper(container, filterPid, maxNum, expectedTimestamps, + (exitInfo) -> (int) exitInfo.getTimestamp()); + } + + private void checkTimestampsAre( + AppExitInfoTracker.AppExitInfoContainer container, List<Integer> expectedTimestamps) { + checkTimestampsAre(container, 0, 0, expectedTimestamps); + } + + @SuppressWarnings("GuardedBy") + private AppExitInfoTracker.AppExitInfoContainer createBasicContainer() { + AppExitInfoTracker.AppExitInfoContainer container = + mAppExitInfoTracker.new AppExitInfoContainer(3); + container.addExitInfoLocked(createExitInfo(10)); + container.addExitInfoLocked(createExitInfo(30)); + container.addExitInfoLocked(createExitInfo(20)); + return container; + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerGetExitInfosIsSortedNewestFirst() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + checkPidsAre(container, Arrays.asList(30, 20, 10)); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerRemovesOldestReports() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + container.addExitInfoLocked(createExitInfo(40)); + checkPidsAre(container, Arrays.asList(40, 30, 20)); + + container.addExitInfoLocked(createExitInfo(50)); + checkPidsAre(container, Arrays.asList(50, 40, 30)); + + container.addExitInfoLocked(createExitInfo(45)); + checkPidsAre(container, Arrays.asList(50, 45, 40)); + + // Adding an older report shouldn't remove the newer ones. + container.addExitInfoLocked(createExitInfo(15)); + checkPidsAre(container, Arrays.asList(50, 45, 40)); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerFilterByPid() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + assertEquals(1, getExitInfosHelper(container, 30, 0).size()); + assertEquals(30, getExitInfosHelper(container, 0, 0).get(0).getPid()); + + assertEquals(1, getExitInfosHelper(container, 30, 0).size()); + assertEquals(20, getExitInfosHelper(container, 20, 0).get(0).getPid()); + + assertEquals(1, getExitInfosHelper(container, 10, 0).size()); + assertEquals(10, getExitInfosHelper(container, 10, 0).get(0).getPid()); + + assertEquals(0, getExitInfosHelper(container, 1337, 0).size()); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerLimitQuantityOfResults() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + checkPidsAre(container, /* filterPid */ 30, /* maxNum */ 1, Arrays.asList(30)); + checkPidsAre(container, /* filterPid */ 30, /* maxNum */ 1000, Arrays.asList(30)); + + checkPidsAre(container, /* filterPid */ 20, /* maxNum */ 1, Arrays.asList(20)); + checkPidsAre(container, /* filterPid */ 20, /* maxNum */ 1000, Arrays.asList(20)); + + checkPidsAre(container, /* filterPid */ 10, /* maxNum */ 1, Arrays.asList(10)); + checkPidsAre(container, /* filterPid */ 10, /* maxNum */ 1000, Arrays.asList(10)); + + checkPidsAre(container, /* filterPid */ 1337, /* maxNum */ 1, Arrays.asList()); + checkPidsAre(container, /* filterPid */ 1337, /* maxNum */ 1000, Arrays.asList()); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerLastExitInfoForPid() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + assertEquals(30, container.getLastExitInfoForPid(30).getPid()); + assertEquals(20, container.getLastExitInfoForPid(20).getPid()); + assertEquals(10, container.getLastExitInfoForPid(10).getPid()); + assertEquals(null, container.getLastExitInfoForPid(1337)); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerCanHoldMultipleFromSamePid() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + ApplicationExitInfo info = createExitInfo(100); + ApplicationExitInfo info2 = createExitInfo(100); + ApplicationExitInfo info3 = createExitInfo(100); + info2.setTimestamp(1337); + info3.setTimestamp(31337); + + container.addExitInfoLocked(info); + assertEquals(1100, container.getLastExitInfoForPid(100).getTimestamp()); + container.addExitInfoLocked(info2); + assertEquals(1337, container.getLastExitInfoForPid(100).getTimestamp()); + container.addExitInfoLocked(info3); + assertEquals(31337, container.getLastExitInfoForPid(100).getTimestamp()); + + checkPidsAre(container, Arrays.asList(100, 100, 100)); + checkTimestampsAre(container, Arrays.asList(31337, 1337, 1100)); + + checkPidsAre(container, /* filterPid */ 100, /* maxNum */ 0, Arrays.asList(100, 100, 100)); + checkTimestampsAre( + container, /* filterPid */ 100, /* maxNum */ 0, Arrays.asList(31337, 1337, 1100)); + + checkPidsAre(container, /* filterPid */ 100, /* maxNum */ 2, Arrays.asList(100, 100)); + checkTimestampsAre( + container, /* filterPid */ 100, /* maxNum */ 2, Arrays.asList(31337, 1337)); + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerIteration() throws Exception { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + checkPidsAre(container, Arrays.asList(30, 20, 10)); + + // Unfortunately relying on order for this test, which is implemented as "last inserted" -> + // "first inserted". Note that this is insertion order, not timestamp. Thus, it's 20 -> 30 + // -> 10, as defined by `createBasicContainer()`. + List<Integer> elements = Arrays.asList(20, 30, 10); + for (int i = 0, size = elements.size(); i < size; i++) { + ArrayList<Integer> processedEntries = new ArrayList<Integer>(); + final int finalIndex = i; + container.forEachRecordLocked((exitInfo) -> { + processedEntries.add(new Integer(exitInfo.getPid())); + if (exitInfo.getPid() == elements.get(finalIndex)) { + return AppExitInfoTracker.FOREACH_ACTION_STOP_ITERATION; + } + return AppExitInfoTracker.FOREACH_ACTION_NONE; + }); + assertEquals(processedEntries, elements.subList(0, i + 1)); + } + } + + @Test + @SuppressWarnings("GuardedBy") + public void testContainerIterationRemove() throws Exception { + for (int pidToRemove : Arrays.asList(30, 20, 10)) { + AppExitInfoTracker.AppExitInfoContainer container = createBasicContainer(); + container.forEachRecordLocked((exitInfo) -> { + if (exitInfo.getPid() == pidToRemove) { + return AppExitInfoTracker.FOREACH_ACTION_REMOVE_ITEM; + } + return AppExitInfoTracker.FOREACH_ACTION_NONE; + }); + ArrayList<Integer> pidsRemaining = new ArrayList<Integer>(Arrays.asList(30, 20, 10)); + pidsRemaining.remove(new Integer(pidToRemove)); + checkPidsAre(container, pidsRemaining); + } + } + private static int makeExitStatus(int exitCode) { return (exitCode << 8) & 0xff00; } diff --git a/tests/BootImageProfileTest/OWNERS b/tests/BootImageProfileTest/OWNERS index 57303e748738..64775f824fa4 100644 --- a/tests/BootImageProfileTest/OWNERS +++ b/tests/BootImageProfileTest/OWNERS @@ -1,4 +1 @@ -calin@google.com -ngeoffray@google.com -vmarko@google.com -yawanng@google.com +include platform/art:main:/OWNERS_boot_profile |