Merge "Fix potential overflow with casting long to int in AppExitInfoTracker" into sc-dev

This commit is contained in:
Jing Ji
2021-06-04 22:49:41 +00:00
committed by Android (Google) Code Review
2 changed files with 40 additions and 34 deletions

View File

@@ -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_AM;
import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME; import static com.android.server.am.ActivityManagerDebugConfig.TAG_WITH_CLASS_NAME;
import android.annotation.CurrentTimeMillisLong;
import android.annotation.Nullable; import android.annotation.Nullable;
import android.app.ApplicationExitInfo; import android.app.ApplicationExitInfo;
import android.app.ApplicationExitInfo.Reason; import android.app.ApplicationExitInfo.Reason;
@@ -287,8 +288,8 @@ public final class AppExitInfoTracker {
if (!mAppExitInfoLoaded.get()) { if (!mAppExitInfoLoaded.get()) {
return; return;
} }
mKillHandler.obtainMessage(KillHandler.MSG_PROC_DIED, obtainRawRecord(app)) mKillHandler.obtainMessage(KillHandler.MSG_PROC_DIED,
.sendToTarget(); obtainRawRecord(app, System.currentTimeMillis())).sendToTarget();
} }
void scheduleNoteAppKill(final ProcessRecord app, final @Reason int reason, void scheduleNoteAppKill(final ProcessRecord app, final @Reason int reason,
@@ -300,7 +301,7 @@ public final class AppExitInfoTracker {
return; return;
} }
ApplicationExitInfo raw = obtainRawRecord(app); ApplicationExitInfo raw = obtainRawRecord(app, System.currentTimeMillis());
raw.setReason(reason); raw.setReason(reason);
raw.setSubReason(subReason); raw.setSubReason(subReason);
raw.setDescription(msg); raw.setDescription(msg);
@@ -542,7 +543,8 @@ public final class AppExitInfoTracker {
return AppExitInfoTracker.FOREACH_ACTION_NONE; 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(); int size = list.size();
if (maxNum > 0) { if (maxNum > 0) {
size = Math.min(size, maxNum); size = Math.min(size, maxNum);
@@ -976,7 +978,7 @@ public final class AppExitInfoTracker {
} }
@VisibleForTesting @VisibleForTesting
ApplicationExitInfo obtainRawRecord(ProcessRecord app) { ApplicationExitInfo obtainRawRecord(ProcessRecord app, @CurrentTimeMillisLong long timestamp) {
ApplicationExitInfo info = mRawRecordsPool.acquire(); ApplicationExitInfo info = mRawRecordsPool.acquire();
if (info == null) { if (info == null) {
info = new ApplicationExitInfo(); info = new ApplicationExitInfo();
@@ -998,7 +1000,7 @@ public final class AppExitInfoTracker {
info.setImportance(procStateToImportance(app.mState.getSetProcState())); info.setImportance(procStateToImportance(app.mState.getSetProcState()));
info.setPss(app.mProfile.getLastPss()); info.setPss(app.mProfile.getLastPss());
info.setRss(app.mProfile.getLastRss()); info.setRss(app.mProfile.getLastRss());
info.setTimestamp(System.currentTimeMillis()); info.setTimestamp(timestamp);
} }
return info; return info;
@@ -1298,7 +1300,7 @@ public final class AppExitInfoTracker {
results.add(mInfos.valueAt(i)); results.add(mInfos.valueAt(i));
} }
Collections.sort(results, Collections.sort(results,
(a, b) -> (int) (b.getTimestamp() - a.getTimestamp())); (a, b) -> Long.compare(b.getTimestamp(), a.getTimestamp()));
} else { } else {
if (maxNum == 1) { if (maxNum == 1) {
// Most of the caller might be only interested with the most recent one // 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)); list.add(mInfos.valueAt(i));
} }
Collections.sort(list, 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++) { for (int i = 0; i < maxNum; i++) {
results.add(list.get(i)); results.add(list.get(i));
} }
@@ -1412,7 +1414,7 @@ public final class AppExitInfoTracker {
for (int i = mInfos.size() - 1; i >= 0; i--) { for (int i = mInfos.size() - 1; i >= 0; i--) {
list.add(mInfos.valueAt(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(); int size = list.size();
for (int i = 0; i < size; i++) { for (int i = 0; i < size; i++) {
list.get(i).dump(pw, prefix + " ", "#" + i, sdf); 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., // A process could be dying but being stuck in some state, i.e.,
// being TRACED by tombstoned, thus the zygote receives SIGCHILD // being TRACED by tombstoned, thus the zygote receives SIGCHILD
// way after we already knew the kill (maybe because we did the kill :P), // way after we already knew the kill (maybe because we did the kill :P),

View File

@@ -36,10 +36,12 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
import android.annotation.CurrentTimeMillisLong;
import android.app.ApplicationExitInfo; import android.app.ApplicationExitInfo;
import android.content.ComponentName; import android.content.ComponentName;
import android.content.Context; import android.content.Context;
@@ -163,14 +165,15 @@ public class ApplicationExitInfoTest {
} }
} }
private void updateExitInfo(ProcessRecord app) { private void updateExitInfo(ProcessRecord app, @CurrentTimeMillisLong long timestamp) {
ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app); ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app, timestamp);
mAppExitInfoTracker.handleNoteProcessDiedLocked(raw); mAppExitInfoTracker.handleNoteProcessDiedLocked(raw);
mAppExitInfoTracker.recycleRawRecord(raw); mAppExitInfoTracker.recycleRawRecord(raw);
} }
private void noteAppKill(ProcessRecord app, int reason, int subReason, String msg) { private void noteAppKill(ProcessRecord app, int reason, int subReason, String msg,
ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app); @CurrentTimeMillisLong long timestamp) {
ApplicationExitInfo raw = mAppExitInfoTracker.obtainRawRecord(app, timestamp);
raw.setReason(reason); raw.setReason(reason);
raw.setSubReason(subReason); raw.setSubReason(subReason);
raw.setDescription(msg); raw.setDescription(msg);
@@ -190,6 +193,7 @@ public class ApplicationExitInfoTest {
// Test application calls System.exit() // Test application calls System.exit()
doNothing().when(mAppExitInfoTracker).schedulePersistProcessExitInfo(anyBoolean()); doNothing().when(mAppExitInfoTracker).schedulePersistProcessExitInfo(anyBoolean());
doReturn(true).when(mAppExitInfoTracker).isFresh(anyLong());
final int app1Uid = 10123; final int app1Uid = 10123;
final int app1Pid1 = 12345; final int app1Pid1 = 12345;
@@ -216,7 +220,7 @@ public class ApplicationExitInfoTest {
final byte[] app1Cookie2 = {(byte) 0x08, (byte) 0x07, (byte) 0x06, (byte) 0x05, final byte[] app1Cookie2 = {(byte) 0x08, (byte) 0x07, (byte) 0x06, (byte) 0x05,
(byte) 0x04, (byte) 0x03, (byte) 0x02, (byte) 0x01}; (byte) 0x04, (byte) 0x03, (byte) 0x02, (byte) 0x01};
final long now1 = System.currentTimeMillis(); final long now1 = 1;
ProcessRecord app = makeProcessRecord( ProcessRecord app = makeProcessRecord(
app1Pid1, // pid app1Pid1, // pid
app1Uid, // uid app1Uid, // uid
@@ -240,7 +244,7 @@ public class ApplicationExitInfoTest {
doReturn(null) doReturn(null)
.when(mAppExitInfoTracker.mAppExitInfoSourceLmkd) .when(mAppExitInfoTracker.mAppExitInfoSourceLmkd)
.remove(anyInt(), anyInt()); .remove(anyInt(), anyInt());
updateExitInfo(app); updateExitInfo(app, now1);
ArrayList<ApplicationExitInfo> list = new ArrayList<ApplicationExitInfo>(); ArrayList<ApplicationExitInfo> list = new ArrayList<ApplicationExitInfo>();
mAppExitInfoTracker.getExitInfo(app1PackageName, app1Uid, app1Pid1, 0, list); mAppExitInfoTracker.getExitInfo(app1PackageName, app1Uid, app1Pid1, 0, list);
@@ -290,11 +294,11 @@ public class ApplicationExitInfoTest {
.when(mAppExitInfoTracker.mAppExitInfoSourceLmkd) .when(mAppExitInfoTracker.mAppExitInfoSourceLmkd)
.remove(anyInt(), anyInt()); .remove(anyInt(), anyInt());
noteAppKill(app, ApplicationExitInfo.REASON_USER_REQUESTED, 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 // Case 2: create another app1 process record with a different pid
sleep(1); sleep(1);
final long now2 = System.currentTimeMillis(); final long now2 = 2;
app = makeProcessRecord( app = makeProcessRecord(
app1Pid2, // pid app1Pid2, // pid
app1Uid, // uid app1Uid, // uid
@@ -316,16 +320,15 @@ public class ApplicationExitInfoTest {
doReturn(new Pair<Long, Object>(now2, Integer.valueOf(makeExitStatus(exitCode)))) doReturn(new Pair<Long, Object>(now2, Integer.valueOf(makeExitStatus(exitCode))))
.when(mAppExitInfoTracker.mAppExitInfoSourceZygote) .when(mAppExitInfoTracker.mAppExitInfoSourceZygote)
.remove(anyInt(), anyInt()); .remove(anyInt(), anyInt());
updateExitInfo(app); updateExitInfo(app, now2);
list.clear(); list.clear();
// Get all the records for app1Uid // Get all the records for app1Uid
mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list); mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list);
assertEquals(3, list.size()); assertEquals(3, list.size());
info = list.get(0); info = list.get(1);
// Verify the most recent one
verifyApplicationExitInfo( verifyApplicationExitInfo(
info, // info info, // info
now2, // timestamp now2, // timestamp
@@ -346,7 +349,7 @@ public class ApplicationExitInfoTest {
assertTrue(ArrayUtils.equals(info.getProcessStateSummary(), app1Cookie2, assertTrue(ArrayUtils.equals(info.getProcessStateSummary(), app1Cookie2,
app1Cookie2.length)); app1Cookie2.length));
info = list.get(1); info = list.get(0);
verifyApplicationExitInfo( verifyApplicationExitInfo(
info, // info info, // info
now1s, // timestamp now1s, // timestamp
@@ -386,7 +389,7 @@ public class ApplicationExitInfoTest {
doReturn(new Pair<Long, Object>(now3, Integer.valueOf(makeSignalStatus(sigNum)))) doReturn(new Pair<Long, Object>(now3, Integer.valueOf(makeSignalStatus(sigNum))))
.when(mAppExitInfoTracker.mAppExitInfoSourceZygote) .when(mAppExitInfoTracker.mAppExitInfoSourceZygote)
.remove(anyInt(), anyInt()); .remove(anyInt(), anyInt());
updateExitInfo(app); updateExitInfo(app, now3);
list.clear(); list.clear();
mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 0, list); mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 0, list);
@@ -463,7 +466,7 @@ public class ApplicationExitInfoTest {
app2Rss1, // rss app2Rss1, // rss
app2ProcessName, // processName app2ProcessName, // processName
app2PackageName); // packageName app2PackageName); // packageName
updateExitInfo(app); updateExitInfo(app, now4);
list.clear(); list.clear();
mAppExitInfoTracker.getExitInfo(app2PackageName, app2UidUser2, app2PidUser2, 0, list); mAppExitInfoTracker.getExitInfo(app2PackageName, app2UidUser2, app2PidUser2, 0, list);
assertEquals(1, list.size()); assertEquals(1, list.size());
@@ -523,9 +526,9 @@ public class ApplicationExitInfoTest {
app3ProcessName, // processName app3ProcessName, // processName
app3PackageName); // packageName app3PackageName); // packageName
noteAppKill(app, ApplicationExitInfo.REASON_CRASH_NATIVE, noteAppKill(app, ApplicationExitInfo.REASON_CRASH_NATIVE,
ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description); ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description, now5);
updateExitInfo(app); updateExitInfo(app, now5);
list.clear(); list.clear();
mAppExitInfoTracker.getExitInfo(app3PackageName, app3UidUser2, app3PidUser2, 0, list); mAppExitInfoTracker.getExitInfo(app3PackageName, app3UidUser2, app3PidUser2, 0, list);
assertEquals(1, list.size()); assertEquals(1, list.size());
@@ -648,11 +651,11 @@ public class ApplicationExitInfoTest {
app3PackageName); // packageName app3PackageName); // packageName
mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app3IsolatedUid, app3Uid); mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app3IsolatedUid, app3Uid);
noteAppKill(app, ApplicationExitInfo.REASON_CRASH, noteAppKill(app, ApplicationExitInfo.REASON_CRASH,
ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description2); ApplicationExitInfo.SUBREASON_UNKNOWN, app3Description2, now6);
assertEquals(app3Uid, mAppExitInfoTracker.mIsolatedUidRecords assertEquals(app3Uid, mAppExitInfoTracker.mIsolatedUidRecords
.getUidByIsolatedUid(app3IsolatedUid).longValue()); .getUidByIsolatedUid(app3IsolatedUid).longValue());
updateExitInfo(app); updateExitInfo(app, now6);
assertNull(mAppExitInfoTracker.mIsolatedUidRecords.getUidByIsolatedUid(app3IsolatedUid)); assertNull(mAppExitInfoTracker.mIsolatedUidRecords.getUidByIsolatedUid(app3IsolatedUid));
list.clear(); list.clear();
@@ -736,9 +739,9 @@ public class ApplicationExitInfoTest {
mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app1IsolatedUidUser2, app1UidUser2); mAppExitInfoTracker.mIsolatedUidRecords.addIsolatedUid(app1IsolatedUidUser2, app1UidUser2);
noteAppKill(app, ApplicationExitInfo.REASON_OTHER, noteAppKill(app, ApplicationExitInfo.REASON_OTHER,
ApplicationExitInfo.SUBREASON_UNKNOWN, app1Description); ApplicationExitInfo.SUBREASON_UNKNOWN, app1Description, now8);
updateExitInfo(app); updateExitInfo(app, now8);
list.clear(); list.clear();
mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 1, list); mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1PidUser2, 1, list);
assertEquals(1, list.size()); assertEquals(1, list.size());
@@ -802,8 +805,8 @@ public class ApplicationExitInfoTest {
traceFile, traceStart, traceEnd); traceFile, traceStart, traceEnd);
noteAppKill(app, ApplicationExitInfo.REASON_OTHER, noteAppKill(app, ApplicationExitInfo.REASON_OTHER,
ApplicationExitInfo.SUBREASON_TOO_MANY_EMPTY, app1Description2); ApplicationExitInfo.SUBREASON_TOO_MANY_EMPTY, app1Description2, now9);
updateExitInfo(app); updateExitInfo(app, now9);
list.clear(); list.clear();
mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1Pid2User2, 1, list); mAppExitInfoTracker.getExitInfo(app1PackageName, app1UidUser2, app1Pid2User2, 1, list);
assertEquals(1, list.size()); assertEquals(1, list.size());
@@ -859,7 +862,7 @@ public class ApplicationExitInfoTest {
mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list); mAppExitInfoTracker.getExitInfo(null, app1Uid, 0, 0, list);
assertEquals(3, list.size()); assertEquals(3, list.size());
info = list.get(0); info = list.get(1);
exitCode = 6; exitCode = 6;
verifyApplicationExitInfo( verifyApplicationExitInfo(
@@ -879,7 +882,7 @@ public class ApplicationExitInfoTest {
IMPORTANCE_SERVICE, // importance IMPORTANCE_SERVICE, // importance
null); // description null); // description
info = list.get(1); info = list.get(0);
verifyApplicationExitInfo( verifyApplicationExitInfo(
info, // info info, // info
now1s, // timestamp now1s, // timestamp