From 09ef27279990f07786000a7a09ce98b1657728d2 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Wed, 12 Aug 2020 14:58:10 -0700 Subject: [PATCH] Lock ProcessRecord.baseProcessTracker with proper locks Bug: 163812139 Test: atest ProcStatsValidationTests Test: atest ProcessStatsDumpsysTest Test: atest CtsIncidentHostTestCases:ProcStatsProtoTest Change-Id: I11ea2b3137b06376639b94998a0235e7f3545911 --- .../server/am/ActivityManagerService.java | 51 ++++++++++++------- .../com/android/server/am/ProcessRecord.java | 11 ++-- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 91a1487c52450..4dc381496ee32 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4601,10 +4601,12 @@ public class ActivityManagerService extends IActivityManager.Stub proc.lastMemInfoTime = SystemClock.uptimeMillis(); if (proc.thread != null && proc.setAdj == oomAdj) { // Record this for posterity if the process has been stable. - proc.baseProcessTracker.addPss(infos[i].getTotalPss(), - infos[i].getTotalUss(), infos[i].getTotalRss(), false, - ProcessStats.ADD_PSS_EXTERNAL_SLOW, endTime - startTime, - proc.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + proc.baseProcessTracker.addPss(infos[i].getTotalPss(), + infos[i].getTotalUss(), infos[i].getTotalRss(), false, + ProcessStats.ADD_PSS_EXTERNAL_SLOW, endTime - startTime, + proc.pkgList.mPkgList); + } for (int ipkg = proc.pkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = proc.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.PROCESS_MEMORY_STAT_REPORTED, @@ -4661,8 +4663,11 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized (this) { if (proc.thread != null && proc.setAdj == oomAdj) { // Record this for posterity if the process has been stable. - proc.baseProcessTracker.addPss(pss[i], tmpUss[0], tmpUss[2], false, - ProcessStats.ADD_PSS_EXTERNAL, endTime-startTime, proc.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + proc.baseProcessTracker.addPss(pss[i], tmpUss[0], tmpUss[2], false, + ProcessStats.ADD_PSS_EXTERNAL, endTime - startTime, + proc.pkgList.mPkgList); + } for (int ipkg = proc.pkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = proc.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.PROCESS_MEMORY_STAT_REPORTED, @@ -12099,8 +12104,10 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized (this) { if (r.thread != null && oomAdj == r.getSetAdjWithServices()) { // Record this for posterity if the process has been stable. - r.baseProcessTracker.addPss(myTotalPss, myTotalUss, myTotalRss, true, - reportType, endTime-startTime, r.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + r.baseProcessTracker.addPss(myTotalPss, myTotalUss, myTotalRss, true, + reportType, endTime - startTime, r.pkgList.mPkgList); + } for (int ipkg = r.pkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = r.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.PROCESS_MEMORY_STAT_REPORTED, @@ -12705,8 +12712,10 @@ public class ActivityManagerService extends IActivityManager.Stub synchronized (this) { if (r.thread != null && oomAdj == r.getSetAdjWithServices()) { // Record this for posterity if the process has been stable. - r.baseProcessTracker.addPss(myTotalPss, myTotalUss, myTotalRss, true, - reportType, endTime-startTime, r.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + r.baseProcessTracker.addPss(myTotalPss, myTotalUss, myTotalRss, true, + reportType, endTime - startTime, r.pkgList.mPkgList); + } for (int ipkg = r.pkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = r.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.PROCESS_MEMORY_STAT_REPORTED, @@ -15917,8 +15926,10 @@ public class ActivityManagerService extends IActivityManager.Stub EventLogTags.writeAmPss(proc.pid, proc.uid, proc.processName, pss * 1024, uss * 1024, swapPss * 1024, rss * 1024, statType, procState, pssDuration); proc.lastPssTime = now; - proc.baseProcessTracker.addPss( - pss, uss, rss, true, statType, pssDuration, proc.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + proc.baseProcessTracker.addPss( + pss, uss, rss, true, statType, pssDuration, proc.pkgList.mPkgList); + } for (int ipkg = proc.pkgList.mPkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = proc.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.PROCESS_MEMORY_STAT_REPORTED, @@ -16279,7 +16290,9 @@ public class ActivityManagerService extends IActivityManager.Stub ApplicationExitInfo.REASON_EXCESSIVE_RESOURCE_USAGE, ApplicationExitInfo.SUBREASON_EXCESSIVE_CPU, true); - app.baseProcessTracker.reportExcessiveCpu(app.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + app.baseProcessTracker.reportExcessiveCpu(app.pkgList.mPkgList); + } for (int ipkg = app.pkgList.size() - 1; ipkg >= 0; ipkg--) { ProcessStats.ProcessStateHolder holder = app.pkgList.valueAt(ipkg); FrameworkStatsLog.write(FrameworkStatsLog.EXCESSIVE_CPU_USAGE_REPORTED, @@ -16391,11 +16404,13 @@ public class ActivityManagerService extends IActivityManager.Stub } final void setProcessTrackerStateLocked(ProcessRecord proc, int memFactor, long now) { - if (proc.thread != null && proc.baseProcessTracker != null) { - final int procState = proc.getReportedProcState(); - if (procState != PROCESS_STATE_NONEXISTENT) { - proc.baseProcessTracker.setState( - procState, memFactor, now, proc.pkgList.mPkgList); + synchronized (mProcessStats.mLock) { + if (proc.thread != null && proc.baseProcessTracker != null) { + final int procState = proc.getReportedProcState(); + if (procState != PROCESS_STATE_NONEXISTENT) { + proc.baseProcessTracker.setState( + procState, memFactor, now, proc.pkgList.mPkgList); + } } } } diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java index e3e13391a8b0d..14bc6cbed7d78 100644 --- a/services/core/java/com/android/server/am/ProcessRecord.java +++ b/services/core/java/com/android/server/am/ProcessRecord.java @@ -1079,8 +1079,8 @@ class ProcessRecord implements WindowProcessListener { */ public void resetPackageList(ProcessStatsService tracker) { final int N = pkgList.size(); - if (baseProcessTracker != null) { - synchronized (tracker.mLock) { + synchronized (tracker.mLock) { + if (baseProcessTracker != null) { long now = SystemClock.uptimeMillis(); baseProcessTracker.setState(ProcessStats.STATE_NOTHING, tracker.getMemFactorLocked(), now, pkgList.mPkgList); @@ -1108,10 +1108,11 @@ class ProcessRecord implements WindowProcessListener { holder.state.makeActive(); } } + } else if (N != 1) { + pkgList.clear(); + pkgList.put(info.packageName, + new ProcessStats.ProcessStateHolder(info.longVersionCode)); } - } else if (N != 1) { - pkgList.clear(); - pkgList.put(info.packageName, new ProcessStats.ProcessStateHolder(info.longVersionCode)); } }