Resolve Thread Visibility Issue in ProcessErrorStateRecord.appNotResponding

Use processCpuTracker as an intrinsic lock to correct a thread visibility
issue within the ProcessErrorStateRecord.appNotResponding method. This ensures
proper synchronization and prevents potential ANR dumping errors due to
concurrent access.

Test: Tested manually
Bug: 278852424
Change-Id: I93e394e7da90ed1ad23b11b36c775f648ccfdb63
This commit is contained in:
Mohamad Mahmoud
2023-06-07 09:34:00 +00:00
parent d6ac360382
commit e495b37efd
2 changed files with 30 additions and 18 deletions

View File

@@ -51,11 +51,11 @@ import android.util.SparseBooleanArray;
import com.android.internal.annotations.CompositeRWLock;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.modules.expresslog.Counter;
import com.android.internal.os.ProcessCpuTracker;
import com.android.internal.os.TimeoutRecord;
import com.android.internal.os.anr.AnrLatencyTracker;
import com.android.internal.util.FrameworkStatsLog;
import com.android.modules.expresslog.Counter;
import com.android.server.ResourcePressureUtil;
import com.android.server.criticalevents.CriticalEventLog;
import com.android.server.stats.pull.ProcfsMemoryUtil.MemorySnapshot;
@@ -456,6 +456,11 @@ class ProcessErrorStateRecord {
String currentPsiState = ResourcePressureUtil.currentPsiState();
latencyTracker.currentPsiStateReturned();
report.append(currentPsiState);
// The 'processCpuTracker' variable is a shared resource that might be initialized and
// updated in a different thread. In order to prevent thread visibility issues, which
// can occur when one thread does not immediately see the changes made to
// 'processCpuTracker' by another thread, it is necessary to use synchronization whenever
// 'processCpuTracker' is accessed or modified.
ProcessCpuTracker processCpuTracker = new ProcessCpuTracker(true);
// We push the native pids collection task to the helper thread through
@@ -517,12 +522,16 @@ class ProcessErrorStateRecord {
}
mService.updateCpuStatsNow();
mService.mAppProfiler.printCurrentCpuState(report, anrTime);
info.append(processCpuTracker.printCurrentLoad());
synchronized (processCpuTracker) {
info.append(processCpuTracker.printCurrentLoad());
}
info.append(report);
}
report.append(tracesFileException.getBuffer());
info.append(processCpuTracker.printCurrentState(anrTime));
synchronized (processCpuTracker) {
info.append(processCpuTracker.printCurrentState(anrTime));
}
Slog.e(TAG, info.toString());
if (tracesFile == null) {

View File

@@ -464,28 +464,31 @@ public class StackTracesDumpHelper {
latencyTracker.processCpuTrackerMethodsCalled();
}
ArrayList<Integer> extraPids = new ArrayList<>();
processCpuTracker.init();
synchronized (processCpuTracker) {
processCpuTracker.init();
}
try {
Thread.sleep(200);
} catch (InterruptedException ignored) {
}
processCpuTracker.update();
synchronized (processCpuTracker) {
processCpuTracker.update();
// We'll take the stack crawls of just the top apps using CPU.
final int workingStatsNumber = processCpuTracker.countWorkingStats();
for (int i = 0; i < workingStatsNumber && extraPids.size() < 2; i++) {
ProcessCpuTracker.Stats stats = processCpuTracker.getWorkingStats(i);
if (lastPids.indexOfKey(stats.pid) >= 0) {
if (DEBUG_ANR) {
Slog.d(TAG, "Collecting stacks for extra pid " + stats.pid);
}
// We'll take the stack crawls of just the top apps using CPU.
final int workingStatsNumber = processCpuTracker.countWorkingStats();
for (int i = 0; i < workingStatsNumber && extraPids.size() < 2; i++) {
ProcessCpuTracker.Stats stats = processCpuTracker.getWorkingStats(i);
if (lastPids.indexOfKey(stats.pid) >= 0) {
if (DEBUG_ANR) {
Slog.d(TAG, "Collecting stacks for extra pid " + stats.pid);
extraPids.add(stats.pid);
} else {
Slog.i(TAG,
"Skipping next CPU consuming process, not a java proc: "
+ stats.pid);
}
extraPids.add(stats.pid);
} else {
Slog.i(TAG,
"Skipping next CPU consuming process, not a java proc: "
+ stats.pid);
}
}
if (latencyTracker != null) {