Fix code formatting, make API 21 level compatible, minor improvements

markushi · markushi · commit 5d22e0ac4b15 · 2025-01-22T10:12:52.000+01:00
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2Integration.java
@@ -42,7 +42,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -270,9 +269,9 @@ private void reportAsSentryEvent(
         event.setMessage(sentryMessage);
       } else if (result.type == ParseResult.Type.DUMP) {
         event.setThreads(result.threads);
-        if (!result.debugImages.isEmpty()) {
+        if (result.debugImages != null) {
           final DebugMeta debugMeta = new DebugMeta();
-          debugMeta.setImages(new ArrayList<>(result.debugImages.values()));
+          debugMeta.setImages(result.debugImages);
           event.setDebugMeta(debugMeta);
         }
       }
@@ -319,15 +318,19 @@ private void reportAsSentryEvent(
         final Lines lines = Lines.readLines(reader);
 
         final ThreadDumpParser threadDumpParser = new ThreadDumpParser(options, isBackground);
-        final List<SentryThread> threads = threadDumpParser.parse(lines);
+        threadDumpParser.parse(lines);
+
+        final @NotNull List<SentryThread> threads = threadDumpParser.getThreads();
+        final @NotNull List<DebugImage> debugImages = threadDumpParser.getDebugImages();
+
         if (threads.isEmpty()) {
           // if the list is empty this means the system failed to capture a proper thread dump of
           // the android threads, and only contains kernel-level threads and statuses, those ANRs
           // are not actionable and neither they are reported by Google Play Console, so we just
           // fall back to not reporting them
           return new ParseResult(ParseResult.Type.NO_DUMP);
         }
-        return new ParseResult(ParseResult.Type.DUMP, dump, threads, threadDumpParser.getDebugImages());
+        return new ParseResult(ParseResult.Type.DUMP, dump, threads, debugImages);
       } catch (Throwable e) {
         options.getLogger().log(SentryLevel.WARNING, "Failed to parse ANR thread dump", e);
         return new ParseResult(ParseResult.Type.ERROR, dump);
@@ -411,7 +414,7 @@ enum Type {
     final Type type;
     final byte[] dump;
     final @Nullable List<SentryThread> threads;
-    final @Nullable Map<String, DebugImage> debugImages;
+    final @Nullable List<DebugImage> debugImages;
 
     ParseResult(final @NotNull Type type) {
       this.type = type;
@@ -428,7 +431,10 @@ enum Type {
     }
 
     ParseResult(
-        final @NotNull Type type, final byte[] dump, final @Nullable List<SentryThread> threads, final @Nullable Map<String, DebugImage> debugImages) {
+        final @NotNull Type type,
+        final byte[] dump,
+        final @Nullable List<SentryThread> threads,
+        final @Nullable List<DebugImage> debugImages) {
       this.type = type;
       this.dump = dump;
       this.threads = threads;
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/threaddump/ThreadDumpParser.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/threaddump/ThreadDumpParser.java
@@ -27,14 +27,12 @@
 import io.sentry.protocol.SentryStackTrace;
 import io.sentry.protocol.SentryThread;
 import java.math.BigInteger;
+import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
-import java.nio.BufferUnderflowException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -50,26 +48,38 @@ public class ThreadDumpParser {
       Pattern.compile("\"(.*)\" (.*) ?sysTid=(\\d+)");
 
   // For reference, see native_stack_dump.cc and tombstone_proto_to_text.cpp in Android sources
+  // Groups
+  // 0:entire regex
+  // 1:index
+  // 2:pc
+  // 3:mapinfo
+  // 4:filename
+  // 5:mapoffset
+  // 6:function
+  // 7:fnoffset
+  // 7:buildid
   private static final Pattern NATIVE_RE =
       Pattern.compile(
           // "  native: #12 pc 0xabcd1234"
-          " *(?:native: )?#(?<index>\\d+) \\S+ (?<pc>[0-9a-fA-F]+)"
-          // The map info includes a filename and an optional offset into the file
-          + ("\\s+(?<mapinfo>"
-             // "/path/to/file.ext",
-             + "(?<filename>.*?)"
-             // optional " (deleted)" suffix (deleted files) needed here to bias regex correctly
-             + "(?:\\s+\\(deleted\\))?"
-             // " (offset 0xabcd1234)", if the mapping is not into the beginning of the file
-             + "(?:\\s+\\(offset (?<mapoffset>.*?)\\))?"
-             + ")")
-          // Optional function
-          + ("(?:\\s+\\((?:"
-             + "\\?\\?\\?" // " (???) marks a missing function, so don't capture it in a group
-             + "|(?<function>.*?)(?:\\+(?<fnoffset>\\d+))?" // " (func+1234)", offset is optional
-             + ")\\))?")
-          // Optional " (BuildId: abcd1234abcd1234abcd1234abcd1234abcd1234)"
-          + "(?:\\s+\\(BuildId: (?<buildid>.*?)\\))?");
+          " *(?:native: )?#(\\d+) \\S+ ([0-9a-fA-F]+)"
+              // The map info includes a filename and an optional offset into the file
+              + ("\\s+("
+                  // "/path/to/file.ext",
+                  + "(.*?)"
+                  // optional " (deleted)" suffix (deleted files) needed here to bias regex
+                  // correctly
+                  + "(?:\\s+\\(deleted\\))?"
+                  // " (offset 0xabcd1234)", if the mapping is not into the beginning of the file
+                  + "(?:\\s+\\(offset (.*?)\\))?"
+                  + ")")
+              // Optional function
+              + ("(?:\\s+\\((?:"
+                  + "\\?\\?\\?" // " (???) marks a missing function, so don't capture it in a group
+                  + "|(.*?)(?:\\+(\\d+))?" // " (func+1234)", offset is
+                  // optional
+                  + ")\\))?")
+              // Optional " (BuildId: abcd1234abcd1234abcd1234abcd1234abcd1234)"
+              + "(?:\\s+\\(BuildId: (.*?)\\))?");
 
   private static final Pattern JAVA_RE =
       Pattern.compile(" *at (?:(.+)\\.)?([^.]+)\\.([^.]+)\\((.*):([\\d-]+)\\)");
@@ -100,16 +110,24 @@ public class ThreadDumpParser {
 
   private final @NotNull Map<String, DebugImage> debugImages;
 
+  private final @NotNull List<SentryThread> threads;
+
   public ThreadDumpParser(final @NotNull SentryOptions options, final boolean isBackground) {
     this.options = options;
     this.isBackground = isBackground;
     this.stackTraceFactory = new SentryStackTraceFactory(options);
-    this.debugImages = new HashMap<String, DebugImage>();
+    this.debugImages = new HashMap<>();
+    this.threads = new ArrayList<>();
+  }
+
+  @NotNull
+  public List<DebugImage> getDebugImages() {
+    return new ArrayList<>(debugImages.values());
   }
 
   @NotNull
-  public Map<String, DebugImage> getDebugImages() {
-    return debugImages;
+  public List<SentryThread> getThreads() {
+    return threads;
   }
 
   @Nullable
@@ -118,21 +136,20 @@ private static String buildIdToDebugId(String buildId) {
       // Abuse BigInteger as a hex string parser. Extra byte needed to handle leading zeros.
       final ByteBuffer buf = ByteBuffer.wrap(new BigInteger("10" + buildId, 16).toByteArray());
       buf.get();
-      return String.format("%08x-%04x-%04x-%04x-%04x%08x",
-                           buf.order(ByteOrder.LITTLE_ENDIAN).getInt(),
-                           buf.getShort(),
-                           buf.getShort(),
-                           buf.order(ByteOrder.BIG_ENDIAN).getShort(),
-                           buf.getShort(),
-                           buf.getInt());
+      return String.format(
+          "%08x-%04x-%04x-%04x-%04x%08x",
+          buf.order(ByteOrder.LITTLE_ENDIAN).getInt(),
+          buf.getShort(),
+          buf.getShort(),
+          buf.order(ByteOrder.BIG_ENDIAN).getShort(),
+          buf.getShort(),
+          buf.getInt());
     } catch (NumberFormatException | BufferUnderflowException e) {
       return null;
     }
   }
 
-  @NotNull
-  public List<SentryThread> parse(final @NotNull Lines lines) {
-    final List<SentryThread> sentryThreads = new ArrayList<>();
+  public void parse(final @NotNull Lines lines) {
 
     final Matcher beginManagedThreadRe = BEGIN_MANAGED_THREAD_RE.matcher("");
     final Matcher beginUnmanagedNativeThreadRe = BEGIN_UNMANAGED_NATIVE_THREAD_RE.matcher("");
@@ -141,7 +158,7 @@ public List<SentryThread> parse(final @NotNull Lines lines) {
       final Line line = lines.next();
       if (line == null) {
         options.getLogger().log(SentryLevel.WARNING, "Internal error while parsing thread dump.");
-        return sentryThreads;
+        return;
       }
       final String text = line.text;
       // we only handle managed threads, as unmanaged/not attached do not have the thread id and
@@ -151,11 +168,10 @@ public List<SentryThread> parse(final @NotNull Lines lines) {
 
         final SentryThread thread = parseThread(lines);
         if (thread != null) {
-          sentryThreads.add(thread);
+          threads.add(thread);
         }
       }
     }
-    return sentryThreads;
   }
 
   private SentryThread parseThread(final @NotNull Lines lines) {
@@ -242,22 +258,34 @@ private SentryStackTrace parseStacktrace(
         break;
       }
       final String text = line.text;
-      if (matches(nativeRe, text)) {
+      if (matches(javaRe, text)) {
+        final SentryStackFrame frame = new SentryStackFrame();
+        final String packageName = javaRe.group(1);
+        final String className = javaRe.group(2);
+        final String module = String.format("%s.%s", packageName, className);
+        frame.setModule(module);
+        frame.setFunction(javaRe.group(3));
+        frame.setFilename(javaRe.group(4));
+        frame.setLineno(getUInteger(javaRe, 5, null));
+        frame.setInApp(stackTraceFactory.isInApp(module));
+        frames.add(frame);
+        lastJavaFrame = frame;
+      } else if (matches(nativeRe, text)) {
         final SentryStackFrame frame = new SentryStackFrame();
-        frame.setPackage(nativeRe.group("mapinfo"));
-        frame.setFunction(nativeRe.group("function"));
-        frame.setLineno(getInteger(nativeRe, "fnoffset", null));
-        frame.setInstructionAddr("0x" + nativeRe.group("pc"));
+        frame.setPackage(nativeRe.group(3));
+        frame.setFunction(nativeRe.group(6));
+        frame.setLineno(getInteger(nativeRe, 7, null));
+        frame.setInstructionAddr("0x" + nativeRe.group(2));
         frame.setPlatform("native");
 
-        final String buildId = nativeRe.group("buildid");
+        final String buildId = nativeRe.group(8);
         final String debugId = buildId == null ? null : buildIdToDebugId(buildId);
         if (debugId != null) {
           if (!debugImages.containsKey(debugId)) {
             final DebugImage debugImage = new DebugImage();
             debugImage.setDebugId(debugId);
             debugImage.setType("elf");
-            debugImage.setCodeFile(nativeRe.group("filename"));
+            debugImage.setCodeFile(nativeRe.group(4));
             debugImage.setCodeId(buildId);
             debugImages.put(debugId, debugImage);
           }
@@ -267,18 +295,6 @@ private SentryStackTrace parseStacktrace(
 
         frames.add(frame);
         lastJavaFrame = null;
-      } else if (matches(javaRe, text)) {
-        final SentryStackFrame frame = new SentryStackFrame();
-        final String packageName = javaRe.group(1);
-        final String className = javaRe.group(2);
-        final String module = String.format("%s.%s", packageName, className);
-        frame.setModule(module);
-        frame.setFunction(javaRe.group(3));
-        frame.setFilename(javaRe.group(4));
-        frame.setLineno(getUInteger(javaRe, 5, null));
-        frame.setInApp(stackTraceFactory.isInApp(module));
-        frames.add(frame);
-        lastJavaFrame = frame;
       } else if (matches(jniRe, text)) {
         final SentryStackFrame frame = new SentryStackFrame();
         final String packageName = jniRe.group(1);
@@ -395,8 +411,8 @@ private Long getLong(
 
   @Nullable
   private Integer getInteger(
-      final @NotNull Matcher matcher, final String group, final @Nullable Integer defaultValue) {
-    final String str = matcher.group(group);
+      final @NotNull Matcher matcher, final int groupIndex, final @Nullable Integer defaultValue) {
+    final String str = matcher.group(groupIndex);
     if (str == null || str.length() == 0) {
       return defaultValue;
     } else {
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/threaddump/ThreadDumpParserTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/threaddump/ThreadDumpParserTest.kt
@@ -18,7 +18,8 @@ class ThreadDumpParserTest {
             SentryOptions().apply { addInAppInclude("io.sentry.samples") },
             false
         )
-        val threads = parser.parse(lines)
+        parser.parse(lines)
+        val threads = parser.threads
         // just verifying a few important threads, as there are many
         val main = threads.find { it.name == "main" }
         assertEquals(1, main!!.id)
@@ -85,8 +86,10 @@ class ThreadDumpParserTest {
 
         val nativeFrame = randomThread.stacktrace!!.frames!!.get(5)
         assertEquals("/system/lib64/libandroid_runtime.so", nativeFrame.`package`)
-        assertEquals("android::android_os_MessageQueue_nativePollOnce(_JNIEnv*, _jobject*, long, int)",
-                     nativeFrame.function)
+        assertEquals(
+            "android::android_os_MessageQueue_nativePollOnce(_JNIEnv*, _jobject*, long, int)",
+            nativeFrame.function
+        )
         assertEquals(44, nativeFrame.lineno)
         assertNull(nativeFrame.isNative) // Confusing, but "isNative" means JVM frame for a JNI method
         assertEquals("native", nativeFrame.platform)
@@ -99,7 +102,8 @@ class ThreadDumpParserTest {
             SentryOptions().apply { addInAppInclude("io.sentry.samples") },
             false
         )
-        val threads = parser.parse(lines)
+        parser.parse(lines)
+        val threads = parser.threads
         // just verifying a few important threads, as there are many
         val thread = threads.find { it.name == "samples.android" }
         assertEquals(9955, thread!!.id)
@@ -129,9 +133,10 @@ class ThreadDumpParserTest {
 
         val spaceFrame = frames.get(14)
         assertEquals(
-            "[anon:dalvik-classes16.dex extracted in memory from /data/app/~~izn1xSZpFlzfVmWi_I0xlQ=="
-            + "/io.sentry.samples.android-tQSGMNiGA-qdjZm6lPOcNw==/base.apk!classes16.dex]",
-            spaceFrame.`package`)
+            "[anon:dalvik-classes16.dex extracted in memory from /data/app/~~izn1xSZpFlzfVmWi_I0xlQ==" +
+                "/io.sentry.samples.android-tQSGMNiGA-qdjZm6lPOcNw==/base.apk!classes16.dex]",
+            spaceFrame.`package`
+        )
         assertNull(spaceFrame.function)
         assertNull(spaceFrame.lineno)
         assertEquals("0x00000000000306f0", spaceFrame.instructionAddr)
@@ -152,7 +157,7 @@ class ThreadDumpParserTest {
         assertNull(deletedFrame.addrMode)
 
         val debugImages = parser.debugImages
-        val image = debugImages["499d48ba-c085-17cf-3209-da67405662f9"]
+        val image = debugImages.first { image -> image.debugId == "499d48ba-c085-17cf-3209-da67405662f9" }
         assertNotNull(image)
         assertEquals("499d48ba-c085-17cf-3209-da67405662f9", image.debugId)
         assertEquals("/apex/com.android.runtime/lib64/bionic/libc.so", image.codeFile)
@@ -166,7 +171,7 @@ class ThreadDumpParserTest {
             SentryOptions().apply { addInAppInclude("io.sentry.samples") },
             false
         )
-        val threads = parser.parse(lines)
-        assertTrue(threads.isEmpty())
+        parser.parse(lines)
+        assertTrue(parser.threads.isEmpty())
     }
 }
diff --git a/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java b/sentry/src/main/java/io/sentry/protocol/SentryStackFrame.java
@@ -100,10 +100,10 @@ public final class SentryStackFrame implements JsonUnknown, JsonSerializable {
    *
    * <ul>
    *   <li><code>"abs"</code> (the default): <code>instruction_addr</code> is absolute.
-   *   <li><code>"rel:$idx"</code>: <code>instruction_addr</code> is relative to the
-   *       <code>debug_meta.image</code> identified by its index in the list.
-   *   <li><code>"rel:$uuid"</code>: <code>instruction_addr</code> is relative to the
-   *       <code>debug_meta.image</code> identified by its <code>debug_id</code>.
+   *   <li><code>"rel:$idx"</code>: <code>instruction_addr</code> is relative to the <code>
+   *       debug_meta.image</code> identified by its index in the list.
+   *   <li><code>"rel:$uuid"</code>: <code>instruction_addr</code> is relative to the <code>
+   *       debug_meta.image</code> identified by its <code>debug_id</code>.
    * </ul>
    */
   private @Nullable String addrMode;
@@ -282,7 +282,7 @@ public void setInstructionAddr(final @Nullable String instructionAddr) {
   }
 
   public void setAddrMode(final @Nullable String addrMode) {
-      this.addrMode = addrMode;
+    this.addrMode = addrMode;
   }
 
   public @Nullable Boolean isNative() {
