feat(anr): Profile main thread when ANR and report ANR profiles to Sentry #4899
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package io.sentry.android.core.anr; | ||
|
|
||
| import java.util.Arrays; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
|
|
||
| @ApiStatus.Internal | ||
| public class AggregatedStackTrace { | ||
| // the number of frames of the stacktrace | ||
| final int depth; | ||
|
|
||
| // the quality of the stack trace, higher means better | ||
| final int quality; | ||
|
|
||
| private final StackTraceElement[] stack; | ||
|
|
||
| // 0 is the most detailed frame in the stacktrace | ||
| private final int stackStartIdx; | ||
| private final int stackEndIdx; | ||
|
|
||
| // the total number of times this exact stacktrace was captured | ||
| int count; | ||
|
|
||
| // first time the stacktrace occured | ||
| private long startTimeMs; | ||
|
|
||
| // last time the stacktrace occured | ||
| private long endTimeMs; | ||
|
|
||
| public AggregatedStackTrace( | ||
| final StackTraceElement[] stack, | ||
| final int stackStartIdx, | ||
| final int stackEndIdx, | ||
| final long timestampMs, | ||
| final int quality) { | ||
| this.stack = stack; | ||
| this.stackStartIdx = stackStartIdx; | ||
| this.stackEndIdx = stackEndIdx; | ||
| this.depth = stackEndIdx - stackStartIdx; | ||
cursor[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| this.startTimeMs = timestampMs; | ||
| this.endTimeMs = timestampMs; | ||
| this.count = 1; | ||
| this.quality = quality; | ||
| } | ||
|
|
||
| public void add(long timestampMs) { | ||
| this.startTimeMs = Math.min(startTimeMs, timestampMs); | ||
| this.endTimeMs = Math.max(endTimeMs, timestampMs); | ||
| this.count++; | ||
| } | ||
|
|
||
| public StackTraceElement[] getStack() { | ||
| return Arrays.copyOfRange(stack, stackStartIdx, stackEndIdx + 1); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package io.sentry.android.core.anr; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| @ApiStatus.Internal | ||
| public class AnrCulpritIdentifier { | ||
|
|
||
| // common Java and Android packages who are less relevant for being the actual culprit | ||
| private static final List<String> lowQualityPackages = new ArrayList<>(9); | ||
|
|
||
| { | ||
| lowQualityPackages.add("java.lang"); | ||
| lowQualityPackages.add("java.util"); | ||
| lowQualityPackages.add("android.app"); | ||
| lowQualityPackages.add("android.os.Handler"); | ||
| lowQualityPackages.add("android.os.Looper"); | ||
| lowQualityPackages.add("android.view"); | ||
| lowQualityPackages.add("android.widget"); | ||
| lowQualityPackages.add("com.android.internal"); | ||
| lowQualityPackages.add("com.google.android"); | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @param dumps | ||
| * @return | ||
| */ | ||
| @Nullable | ||
| public static AggregatedStackTrace identify(final @NotNull List<AnrStackTrace> dumps) { | ||
| if (dumps.isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| // fold all stacktraces and count their occurrences | ||
| final Map<Integer, AggregatedStackTrace> stackTraceMap = new HashMap<>(); | ||
| for (final AnrStackTrace dump : dumps) { | ||
|
|
||
| // entry 0 is the most detailed element in the stacktrace | ||
| // so create sub-stacks (1..n, 2..n, ...) to capture the most common root cause of an ANR | ||
| for (int i = 0; i < dump.stack.length - 1; i++) { | ||
| final int key = subArrayHashCode(dump.stack, i, dump.stack.length - 1); | ||
| int quality = 10; | ||
| final String clazz = dump.stack[i].getClassName(); | ||
| for (String ignoredPackage : lowQualityPackages) { | ||
| if (clazz.startsWith(ignoredPackage)) { | ||
| quality = 1; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| @Nullable AggregatedStackTrace aggregatedStackTrace = stackTraceMap.get(key); | ||
| if (aggregatedStackTrace == null) { | ||
| aggregatedStackTrace = | ||
| new AggregatedStackTrace( | ||
| dump.stack, i, dump.stack.length - 1, dump.timestampMs, quality); | ||
| stackTraceMap.put(key, aggregatedStackTrace); | ||
| } else { | ||
| aggregatedStackTrace.add(dump.timestampMs); | ||
| } | ||
| } | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // the deepest stacktrace with most count wins | ||
| return Collections.max( | ||
| stackTraceMap.values(), | ||
| (c1, c2) -> { | ||
| final int countComparison = Integer.compare(c1.count * c1.quality, c2.count * c2.quality); | ||
| if (countComparison == 0) { | ||
| return Integer.compare(c1.depth, c2.depth); | ||
| } | ||
| return countComparison; | ||
| }); | ||
| } | ||
|
|
||
| private static int subArrayHashCode( | ||
| final @NotNull Object[] arr, final int stackStartIdx, final int stackEndIdx) { | ||
| int result = 1; | ||
| for (int i = stackStartIdx; i <= stackEndIdx; i++) { | ||
| final Object item = arr[i]; | ||
| result = 31 * result + item.hashCode(); | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug:
AnrProfilingIntegrationandAnrV2Integrationconcurrently access the sameQueueFilewithout synchronization.Severity: CRITICAL | Confidence: 1.00
🔍 Detailed Analysis
The
AnrProfilingIntegration(writer) andAnrV2Integration(reader) concurrently access the same underlyingQueueFileatoptions.getCacheDirPath() + "anr_profile"without any synchronization. This violates the explicit contract ofQueueFilewhich states, "Only one instance should access a given file at a time." Concurrent unsynchronized access can lead to data corruption, lost stack traces, or file format inconsistencies due to conflicting writes and reads to the shared file header and data structures.💡 Suggested Fix
Implement a synchronization mechanism (e.g., file-level locking) to ensure exclusive access to the
QueueFile, or refactor to use a singleQueueFileinstance managed by a central component.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2623394