Fix potential overflow with casting long to int in AppExitInfoTracker

It'll result in unexpected behavior during sorting.

Also updated the test case to capture this case.

Bug: 186132521
Bug: 189975360
Test: atest ApplicationExitInfoTest
Test: atest CtsAppExitTestCases:ActivityManagerAppExitInfoTest
Change-Id: I7542f73b1acf4ddee16f712d0d97223e151935f7

2 files changed, 40 insertions, 34 deletions

diff --git a/services/core/java/com/android/server/am/AppExitInfoTracker.java b/services/core/java/com/android/server/am/AppExitInfoTracker.java
index 1b5483af46ed..abb824395578 100644
--- a/services/core/java/com/android/server/am/AppExitInfoTracker.java
+++ b/services/core/java/com/android/server/am/AppExitInfoTracker.java
@@ -24,6 +24,7 @@ import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_PROCESSES;
 import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM;
 import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME;
 
+import android.annotation.CurrentTimeMillisLong;
 import android.annotation.Nullable;
 import android.app.ApplicationExitInfo;
 import android.app.ApplicationExitInfo.Reason;
@@ -287,8 +288,8 @@ public final class AppExitInfoTracker {
         if (!mAppExitInfoLoaded.get()) {
             return;
         }
-        mKillHandler.obtainMessage(KillHandler.MSG_PROC_DIED, obtainRawRecord(app))
-                .sendToTarget();
+        mKillHandler.obtainMessage(KillHandler.MSG_PROC_DIED,
+                obtainRawRecord(app, System.currentTimeMillis())).sendToTarget();
     }
 
     void scheduleNoteAppKill(final ProcessRecord app, final @Reason int reason,
@@ -300,7 +301,7 @@ public final class AppExitInfoTracker {
             return;
         }
 
-        ApplicationExitInfo raw = obtainRawRecord(app);
+        ApplicationExitInfo raw = obtainRawRecord(app, System.currentTimeMillis());
         raw.setReason(reason);
         raw.setSubReason(subReason);
         raw.setDescription(msg);
@@ -542,7 +543,8 @@ public final class AppExitInfoTracker {
                         return AppExitInfoTracker.FOREACH_ACTION_NONE;
                     });
 
-                    Collections.sort(list, (a, b) -> (int) (b.getTimestamp() - a.getTimestamp()));
+                    Collections.sort(list,
+                            (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp()));
                     int size = list.size();
                     if (maxNum > 0) {
                         size = Math.min(size, maxNum);
@@ -976,7 +978,7 @@ public final class AppExitInfoTracker {
     }
 
     @VisibleForTesting
-    ApplicationExitInfo obtainRawRecord(ProcessRecord app) {
+    ApplicationExitInfo obtainRawRecord(ProcessRecord app, @CurrentTimeMillisLong long timestamp) {
         ApplicationExitInfo info = mRawRecordsPool.acquire();
         if (info == null) {
             info = new ApplicationExitInfo();
@@ -998,7 +1000,7 @@ public final class AppExitInfoTracker {
             info.setImportance(procStateToImportance(app.mState.getSetProcState()));
             info.setPss(app.mProfile.getLastPss());
             info.setRss(app.mProfile.getLastRss());
-            info.setTimestamp(System.currentTimeMillis());
+            info.setTimestamp(timestamp);
         }
 
         return info;
@@ -1298,7 +1300,7 @@ public final class AppExitInfoTracker {
                         results.add(mInfos.valueAt(i));
                     }
                     Collections.sort(results,
-                            (a, b) -> (int) (b.getTimestamp() - a.getTimestamp()));
+                            (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
@@ -1318,7 +1320,7 @@ public final class AppExitInfoTracker {
                             list.add(mInfos.valueAt(i));
                         }
                         Collections.sort(list,
-                                (a, b) -> (int) (b.getTimestamp() - a.getTimestamp()));
+                                (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp()));
                         for (int i = 0; i < maxNum; i++) {
                             results.add(list.get(i));
                         }
@@ -1412,7 +1414,7 @@ public final class AppExitInfoTracker {
             for (int i = mInfos.size() - 1; i >= 0; i--) {
                 list.add(mInfos.valueAt(i));
             }
-            Collections.sort(list, (a, b) -> (int) (b.getTimestamp() - a.getTimestamp()));
+            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);
@@ -1629,7 +1631,8 @@ public final class AppExitInfoTracker {
         }
     }
 
-    private static boolean isFresh(long timestamp) {
+    @VisibleForTesting
+    boolean isFresh(long timestamp) {
         // A process could be dying but being stuck in some state, i.e.,
         // being TRACED by tombstoned, thus the zygote receives SIGCHILD
         // way after we already knew the kill (maybe because we did the kill :P),
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 d220444f47de..803a0c10fe1c 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/ApplicationExitInfoTest.java
@@ -36,10 +36,12 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyLong;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
 
+import android.annotation.CurrentTimeMillisLong;
 import android.app.ApplicationExitInfo;
 import android.content.ComponentName;
 import android.content.Context;
@@ -163,14 +165,15 @@ public class ApplicationExitInfoTest {
         }
     }
 
-    private void updateExitInfo(ProcessRecord app) {
-        ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app);
+    private void updateExitInfo(ProcessRecord app, @CurrentTimeMillisLong long timestamp) {
+        ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app, timestamp);
         mAppExitInfoTracker.handleNoteProcessDiedLocked(raw);
         mAppExitInfoTracker.recycleRawRecord(raw);
     }
 
-    private void noteAppKill(ProcessRecord app, int reason, int subReason, String msg) {
-        ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app);
+    private void noteAppKill(ProcessRecord app, int reason, int subReason, String msg,
+            @CurrentTimeMillisLong long timestamp) {
+        ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app, timestamp);
         raw.setReason(reason);
         raw.setSubReason(subReason);
         raw.setDescription(msg);
@@ -190,6 +193,7 @@ public class ApplicationExitInfoTest {
 
         // Test application calls System.exit()
         doNothing().when(mAppExitInfoTracker).schedulePersistProcessExitInfo(anyBoolean());
+        doReturn(true).when(mAppExitInfoTracker).isFresh(anyLong());
 
         final int app1Uid = 10123;
         final int app1Pid1 = 12345;
@@ -216,7 +220,7 @@ public class ApplicationExitInfoTest {
         final byte[] app1Cookie2 = {(byte) 0x08, (byte) 0x07, (byte) 0x06, (byte) 0x05,
                 (byte) 0x04, (byte) 0x03, (byte) 0x02, (byte) 0x01};
 
-        final long now1 = System.currentTimeMillis();
+        final long now1 = 1;
         ProcessRecord app = makeProcessRecord(
                 app1Pid1,                    // pid
                 app1Uid,                     // uid
@@ -240,7 +244,7 @@ public class ApplicationExitInfoTest {
         doReturn(null)
                 .when(mAppExitInfoTracker.mAppExitInfoSourceLmkd)
                 .remove(anyInt(), anyInt());
-        updateExitInfo(app);
+        updateExitInfo(app, now1);
 
         ArrayList<ApplicationExitInfo> list = new ArrayList<ApplicationExitInfo>();
         mAppExitInfoTracker.getExitInfo(app1PackageName, app1Uid, app1Pid1, 0, list);
@@ -290,11 +294,11 @@ public class ApplicationExitInfoTest {
                 .when(mAppExitInfoTracker.mAppExitInfoSourceLmkd)
                 .remove(anyInt(), anyInt());
         noteAppKill(app, ApplicationExitInfo.REASON_USER_REQUESTED,
-                ApplicationExitInfo.SUBREASON_UNKNOWN, null);
+                ApplicationExitInfo.SUBREASON_UNKNOWN, null, now1s);
 
         // Case 2: create another app1 process record with a different pid
         sleep(1);
-        final long now2 = System.currentTimeMillis();
+        final long now2 = 2;
         app = makeProcessRecord(
                 app1Pid2,               // pid
                 app1Uid,                // uid
@@ -316,16 +320,15 @@ public class ApplicationExitInfoTest {
         doReturn(new Pair<Long, Object>(now2, Integer.valueOf(makeExitStatus(exitCode))))
                 .when(mAppExitInfoTracker.mAppExitInfoSourceZygote)
                 .remove(anyInt(), anyInt());
-        updateExitInfo(app);
+        updateExitInfo(app, now2);
         list.clear();
 
         // Get all the records for app1Uid
         mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list);
         assertEquals(3, list.size());
 
-        info = list.get(0);
+        info = list.get(1);
 
-        // Verify the most recent one
         verifyApplicationExitInfo(
                 info,                                 // info
                 now2,                                 // timestamp
@@ -346,7 +349,7 @@ public class ApplicationExitInfoTest {
         assertTrue(ArrayUtils.equals(info.getProcessStateSummary(), app1Cookie2,
                 app1Cookie2.length));
 
-        info = list.get(1);
+        info = list.get(0);
         verifyApplicationExitInfo(
                 info,                                      // info
                 now1s,                                     // timestamp
@@ -386,7 +389,7 @@ public class ApplicationExitInfoTest {
         doReturn(new Pair<Long, Object>(now3, Integer.valueOf(makeSignalStatus(sigNum))))
                 .when(mAppExitInfoTracker.mAppExitInfoSourceZygote)
                 .remove(anyInt(), anyInt());
-        updateExitInfo(app);
+        updateExitInfo(app, now3);
         list.clear();
         mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 0, list);
 
@@ -463,7 +466,7 @@ public class ApplicationExitInfoTest {
                 app2Rss1,                    // rss
                 app2ProcessName,             // processName
                 app2PackageName);            // packageName
-        updateExitInfo(app);
+        updateExitInfo(app, now4);
         list.clear();
         mAppExitInfoTracker.getExitInfo(app2PackageName, app2UidUser2, app2PidUser2, 0, list);
         assertEquals(1, list.size());
@@ -523,9 +526,9 @@ public class ApplicationExitInfoTest {
                 app3ProcessName,         // processName
                 app3PackageName);        // packageName
         noteAppKill(app, ApplicationExitInfo.REASON_CRASH_NATIVE,
-                ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description);
+                ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description, now5);
 
-        updateExitInfo(app);
+        updateExitInfo(app, now5);
         list.clear();
         mAppExitInfoTracker.getExitInfo(app3PackageName, app3UidUser2, app3PidUser2, 0, list);
         assertEquals(1, list.size());
@@ -648,11 +651,11 @@ public class ApplicationExitInfoTest {
                 app3PackageName);            // packageName
         mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app3IsolatedUid, app3Uid);
         noteAppKill(app, ApplicationExitInfo.REASON_CRASH,
-                ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description2);
+                ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description2, now6);
 
         assertEquals(app3Uid, mAppExitInfoTracker.mIsolatedUidRecords
                 .getUidByIsolatedUid(app3IsolatedUid).longValue());
-        updateExitInfo(app);
+        updateExitInfo(app, now6);
         assertNull(mAppExitInfoTracker.mIsolatedUidRecords.getUidByIsolatedUid(app3IsolatedUid));
 
         list.clear();
@@ -736,9 +739,9 @@ public class ApplicationExitInfoTest {
 
         mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app1IsolatedUidUser2, app1UidUser2);
         noteAppKill(app, ApplicationExitInfo.REASON_OTHER,
-                ApplicationExitInfo.SUBREASON_UNKNOWN, app1Description);
+                ApplicationExitInfo.SUBREASON_UNKNOWN, app1Description, now8);
 
-        updateExitInfo(app);
+        updateExitInfo(app, now8);
         list.clear();
         mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 1, list);
         assertEquals(1, list.size());
@@ -802,8 +805,8 @@ public class ApplicationExitInfoTest {
                 traceFile, traceStart, traceEnd);
 
         noteAppKill(app, ApplicationExitInfo.REASON_OTHER,
-                ApplicationExitInfo.SUBREASON_TOO_MANY_EMPTY, app1Description2);
-        updateExitInfo(app);
+                ApplicationExitInfo.SUBREASON_TOO_MANY_EMPTY, app1Description2, now9);
+        updateExitInfo(app, now9);
         list.clear();
         mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1Pid2User2, 1, list);
         assertEquals(1, list.size());
@@ -859,7 +862,7 @@ public class ApplicationExitInfoTest {
         mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list);
         assertEquals(3, list.size());
 
-        info = list.get(0);
+        info = list.get(1);
 
         exitCode = 6;
         verifyApplicationExitInfo(
@@ -879,7 +882,7 @@ public class ApplicationExitInfoTest {
                 IMPORTANCE_SERVICE,                   // importance
                 null);                                // description
 
-        info = list.get(1);
+        info = list.get(0);
         verifyApplicationExitInfo(
                 info,                                      // info
                 now1s,                                     // timestamp