diff --git a/CHANGELOG.md b/CHANGELOG.md index 107b916808a..47b6ffae764 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ ### Fixes +- Fix main thread identification parsing for ApplicationExitInfo ANRs ([#5733](https://github.com/getsentry/sentry-java/pull/5733)) +- Do not send threads without stacktraces for ApplicationExitInfo ANRs ([#5733](https://github.com/getsentry/sentry-java/pull/5733)) - Record byte-level client reports when event processors discard logs or trace metrics ([#5718](https://github.com/getsentry/sentry-java/pull/5718)) - Name the device-info caching thread `SentryDeviceInfoCache` so all threads spawned by the SDK are identifiable ([#5684](https://github.com/getsentry/sentry-java/pull/5684)) - Apply byte-category rate limits to log and trace metric envelope items ([#5716](https://github.com/getsentry/sentry-java/pull/5716)) 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 index f5ce8a745ce..08432d2f41d 100644 --- 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 @@ -45,6 +45,12 @@ public class ThreadDumpParser { private static final Pattern BEGIN_UNMANAGED_NATIVE_THREAD_RE = Pattern.compile("\"(.*)\" (.*) ?sysTid=(\\d+)"); + // e.g. "----- pid 12345 at 2024-01-01 10:00:00.000000000+0000 -----" + private static final Pattern PID_RE = Pattern.compile("----- pid (\\d+) at .*"); + + // e.g. " | sysTid=12345 nice=-10 cgrp=top-app sched=0/0 handle=0x7deceb74f8" + private static final Pattern SYS_TID_RE = Pattern.compile("\\s*\\|\\s*sysTid=(\\d+).*"); + // For reference, see native_stack_dump.cc and tombstone_proto_to_text.cpp in Android sources // Groups // 0:entire regex @@ -104,6 +110,11 @@ public class ThreadDumpParser { private final boolean isBackground; + // the process id parsed from the thread dump header; on Linux/Android the main thread's kernel + // thread id (sysTid) always equals the process id, so we use it to reliably detect the main + // thread + private @Nullable Long processId; + private final @NotNull SentryStackTraceFactory stackTraceFactory; private final @NotNull Map debugImages; @@ -139,6 +150,7 @@ public void parse(final @NotNull Lines lines) { final Matcher beginManagedThreadRe = BEGIN_MANAGED_THREAD_RE.matcher(""); final Matcher beginUnmanagedNativeThreadRe = BEGIN_UNMANAGED_NATIVE_THREAD_RE.matcher(""); + final Matcher pidRe = PID_RE.matcher(""); while (lines.hasNext()) { final Line line = lines.next(); @@ -156,10 +168,14 @@ public void parse(final @NotNull Lines lines) { if (thread != null) { threads.add(thread); } + } else if (matches(pidRe, text)) { + processId = getLong(pidRe, 1, null); } else { artContextParser.parseLine(text); } } + + markThreads(); } private SentryThread parseThread(final @NotNull Lines lines) { @@ -185,7 +201,11 @@ private SentryThread parseThread(final @NotNull Lines lines) { return null; } sentryThread.setId(tid); - sentryThread.setName(beginManagedThreadRe.group(1)); + final String name = beginManagedThreadRe.group(1); + sentryThread.setName(name); + if ("main".equals(name)) { + sentryThread.setMain(true); + } final String state = beginManagedThreadRe.group(5); // sanitizing thread that have more details after their actual state, e.g. // "Native (still starting up)" <- we just need "Native" here @@ -205,19 +225,18 @@ private SentryThread parseThread(final @NotNull Lines lines) { } sentryThread.setId(sysTid); sentryThread.setName(beginUnmanagedNativeThreadRe.group(1)); - } - - final String threadName = sentryThread.getName(); - if (threadName != null) { - final boolean isMain = threadName.equals("main"); - sentryThread.setMain(isMain); - // since it's an ANR, the crashed thread will always be main - sentryThread.setCrashed(isMain); - sentryThread.setCurrent(isMain && !isBackground); + if (sysTid.equals(processId)) { + sentryThread.setMain(true); + } } // thread stacktrace final SentryStackTrace stackTrace = parseStacktrace(lines, sentryThread); + final List frames = stackTrace.getFrames(); + if (frames == null || frames.isEmpty()) { + // skip threads without a stacktrace, they are not actionable + return null; + } sentryThread.setStacktrace(stackTrace); return sentryThread; } @@ -238,6 +257,7 @@ private SentryStackTrace parseStacktrace( final Matcher waitingToLockRe = WAITING_TO_LOCK_RE.matcher(""); final Matcher waitingToLockUnknownRe = WAITING_TO_LOCK_UNKNOWN_RE.matcher(""); final Matcher blankRe = BLANK_RE.matcher(""); + final Matcher sysTidRe = SYS_TID_RE.matcher(""); while (lines.hasNext()) { final Line line = lines.next(); @@ -246,7 +266,12 @@ private SentryStackTrace parseStacktrace( break; } final String text = line.text; - if (matches(javaRe, text)) { + if (matches(sysTidRe, text)) { + final Long sysTid = getLong(sysTidRe, 1, null); + if (sysTid != null && sysTid.equals(processId)) { + thread.setMain(true); + } + } else if (matches(javaRe, text)) { final SentryStackFrame frame = new SentryStackFrame(); final String packageName = javaRe.group(1); final String className = javaRe.group(2); @@ -365,6 +390,24 @@ private SentryStackTrace parseStacktrace( return stackTrace; } + private void markThreads() { + for (final @NotNull SentryThread thread : threads) { + if (Boolean.TRUE.equals(thread.isMain())) { + // the OS may have renamed the main thread to the (truncated) process name; normalize it + // back to "main" so downstream consumers see a consistent name + thread.setName("main"); + + // since it's an ANR, the crashed thread will always be main + thread.setCrashed(true); + thread.setCurrent(!isBackground); + } else { + thread.setCrashed(false); + thread.setCurrent(false); + thread.setMain(false); + } + } + } + private boolean matches(final @NotNull Matcher matcher, final @NotNull String text) { matcher.reset(text); return matcher.matches(); 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 index b7db35b63ce..ec5dbd58902 100644 --- 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 @@ -100,12 +100,15 @@ class ThreadDumpParserTest { 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" } + // the OS named the main thread after the process; it is detected via sysTid==processId (9955) + // and its name is normalized back to "main" + val thread = threads.find { it.isMain == true } assertEquals(9955, thread!!.id) + assertEquals("main", thread.name) assertNull(thread.state) - assertEquals(false, thread.isCrashed) - assertEquals(false, thread.isMain) - assertEquals(false, thread.isCurrent) + assertEquals(true, thread.isCrashed) + assertEquals(true, thread.isMain) + assertEquals(true, thread.isCurrent) // Reverse frames so we can index them with the active frame at index 0 val frames = thread.stacktrace!!.frames!!.reversed() @@ -182,6 +185,38 @@ class ThreadDumpParserTest { assertEquals(8.054, artContext.gcWaitingTime) } + @Test + fun `detects main thread via sysTid matching the process id when OS renames it`() { + val lines = Lines.readLines(File("src/test/resources/thread_dump_process_name_main.txt")) + val parser = + ThreadDumpParser(SentryOptions().apply { addInAppInclude("io.sentry.samples") }, false) + parser.parse(lines) + val threads = parser.threads + // the main thread has been renamed to the (truncated) process name, but its sysTid equals the + // process id, which is how we detect it - its name is then normalized back to "main" + val main = threads.find { it.isMain == true } + assertNotNull(main) + assertEquals("main", main!!.name) + assertEquals(true, main.isCrashed) + assertEquals(true, main.isCurrent) + val background = threads.find { it.name == "Thread-2" } + assertNotNull(background) + assertEquals(false, background!!.isMain) + assertEquals(false, background.isCrashed) + } + + @Test + fun `skips threads without a stacktrace`() { + val lines = Lines.readLines(File("src/test/resources/thread_dump_no_stacktrace.txt")) + val parser = + ThreadDumpParser(SentryOptions().apply { addInAppInclude("io.sentry.samples") }, false) + parser.parse(lines) + val threads = parser.threads + // the thread without any frames is skipped, only the one with a stacktrace remains + assertEquals(1, threads.size) + assertEquals("main", threads.first().name) + } + @Test fun `thread dump garbage`() { val lines = Lines.readLines(File("src/test/resources/thread_dump_bad_data.txt")) diff --git a/sentry-android-core/src/test/resources/thread_dump_no_stacktrace.txt b/sentry-android-core/src/test/resources/thread_dump_no_stacktrace.txt new file mode 100644 index 00000000000..e0411327546 --- /dev/null +++ b/sentry-android-core/src/test/resources/thread_dump_no_stacktrace.txt @@ -0,0 +1,21 @@ + +----- pid 12345 at 2024-01-01 10:00:00.000000000+0000 ----- +Cmd line: io.sentry.samples.android +Build fingerprint: 'google/sdk_gphone64_arm64/emu64a:13/TE1A.220922.012/9302419:userdebug/dev-keys' +ABI: 'arm64' + +DALVIK THREADS (2): +"main" prio=5 tid=1 Runnable + | group="main" sCount=0 ucsCount=0 flags=0 obj=0x72a985e0 self=0xb400007cabc57380 + | sysTid=12345 nice=-10 cgrp=top-app sched=0/0 handle=0x7deceb74f8 + | state=R schedstat=( 324804784 183300334 997 ) utm=23 stm=8 core=3 HZ=100 + | stack=0x7ff93a9000-0x7ff93ab000 stackSize=8188KB + | held mutexes= + at io.sentry.samples.android.MainActivity.onCreate(MainActivity.java:42) + +"Thread-2" prio=5 tid=2 Sleeping + | group="main" sCount=1 ucsCount=0 flags=1 obj=0x136c0518 self=0xb400007cabc82ad0 + | sysTid=12346 nice=0 cgrp=top-app sched=0/0 handle=0x7ace0a9cb0 + | state=S schedstat=( 574039 4838087 11 ) utm=0 stm=0 core=1 HZ=100 + | stack=0x7acdfb2000-0x7acdfb4000 stackSize=991KB + | held mutexes= diff --git a/sentry-android-core/src/test/resources/thread_dump_process_name_main.txt b/sentry-android-core/src/test/resources/thread_dump_process_name_main.txt new file mode 100644 index 00000000000..80b864aae73 --- /dev/null +++ b/sentry-android-core/src/test/resources/thread_dump_process_name_main.txt @@ -0,0 +1,24 @@ + +----- pid 12345 at 2024-01-01 10:00:00.000000000+0000 ----- +Cmd line: io.sentry.samples.android +Build fingerprint: 'google/sdk_gphone64_arm64/emu64a:13/TE1A.220922.012/9302419:userdebug/dev-keys' +ABI: 'arm64' + +DALVIK THREADS (2): +"io.sentry.samples.android" prio=5 tid=1 Runnable + | group="main" sCount=0 ucsCount=0 flags=0 obj=0x72a985e0 self=0xb400007cabc57380 + | sysTid=12345 nice=-10 cgrp=top-app sched=0/0 handle=0x7deceb74f8 + | state=R schedstat=( 324804784 183300334 997 ) utm=23 stm=8 core=3 HZ=100 + | stack=0x7ff93a9000-0x7ff93ab000 stackSize=8188KB + | held mutexes= + at io.sentry.samples.android.MainActivity.onCreate(MainActivity.java:42) + +"Thread-2" prio=5 tid=2 Sleeping + | group="main" sCount=1 ucsCount=0 flags=1 obj=0x136c0518 self=0xb400007cabc82ad0 + | sysTid=12346 nice=0 cgrp=top-app sched=0/0 handle=0x7ace0a9cb0 + | state=S schedstat=( 574039 4838087 11 ) utm=0 stm=0 core=1 HZ=100 + | stack=0x7acdfb2000-0x7acdfb4000 stackSize=991KB + | held mutexes= + at java.lang.Thread.sleep(Native method) + at io.sentry.samples.android.BackgroundWorker.run(BackgroundWorker.java:20) +