fix(logcat): Add re-entrancy guard to SentryLogcatAdapter

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Fixes
 
+- Fix `StackOverflowError` from infinite recursion in `SentryLogcatAdapter` when a `beforeBreadcrumb` or `beforeSendLog` callback logs while logcat instrumentation is enabled ([#5734](https://github.com/getsentry/sentry-java/pull/5734))
 - 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))

sentry-android-core/build.gradle.kts

@@ -104,6 +104,7 @@ dependencies {
   // tests
   testImplementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION))
   testImplementation(libs.roboelectric)
+  testImplementation(libs.google.truth)
   testImplementation(libs.kotlin.test.junit)
   testImplementation(libs.androidx.core.ktx)
   testImplementation(libs.androidx.test.core)

sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java

@@ -19,6 +19,24 @@
 @ApiStatus.Internal
 public final class SentryLogcatAdapter {
 
+  /**
+   * Re-entrancy guard to prevent infinite recursion. The Sentry Android Gradle Plugin rewrites
+   * {@code android.util.Log.*} calls in the app's own code to {@code SentryLogcatAdapter.*} (SDK
+   * classes under {@code io.sentry.*} are excluded). Recursion happens when code that runs while a
+   * Logcat breadcrumb or log is being captured itself calls a rewritten {@code Log.*} method. The
+   * common paths are an app {@code beforeBreadcrumb} or {@code beforeSendLog} callback that logs:
+   * SentryLogcatAdapter.w() -> addAsBreadcrumb()/addAsLog() -> app callback -> Log.w() (rewritten)
+   * -> SentryLogcatAdapter.w() -> ... The guard makes any re-entrant capture on the same thread a
+   * no-op, while the real {@code Log.*} call is left untouched so logcat output is preserved.
+   */
+  private static final ThreadLocal<Boolean> isCapturing =
+      new ThreadLocal<Boolean>() {
+        @Override
+        protected Boolean initialValue() {
+          return Boolean.FALSE;
+        }
+      };
+
   private static void addAsBreadcrumb(
       @Nullable String tag, @NotNull SentryLevel level, @Nullable String msg) {
     addAsBreadcrumb(tag, level, msg, null);
@@ -34,36 +52,52 @@ private static void addAsBreadcrumb(
       @NotNull final SentryLevel level,
       @Nullable final String msg,
       @Nullable final Throwable tr) {
-    Breadcrumb breadcrumb = new Breadcrumb();
-    breadcrumb.setCategory("Logcat");
-    breadcrumb.setMessage(msg);
-    breadcrumb.setLevel(level);
-    if (tag != null) {
-      breadcrumb.setData("tag", tag);
+    if (Boolean.TRUE.equals(isCapturing.get())) {
+      return;
     }
-    if (tr != null && tr.getMessage() != null) {
-      breadcrumb.setData("throwable", tr.getMessage());
+    isCapturing.set(Boolean.TRUE);
+    try {
+      Breadcrumb breadcrumb = new Breadcrumb();
+      breadcrumb.setCategory("Logcat");
+      breadcrumb.setMessage(msg);
+      breadcrumb.setLevel(level);
+      if (tag != null) {
+        breadcrumb.setData("tag", tag);
+      }
+      if (tr != null && tr.getMessage() != null) {
+        breadcrumb.setData("throwable", tr.getMessage());
+      }
+      Sentry.addBreadcrumb(breadcrumb);
+    } finally {
+      isCapturing.set(Boolean.FALSE);
     }
-    Sentry.addBreadcrumb(breadcrumb);
   }
 
   private static void addAsLog(
       @NotNull final SentryLogLevel level,
       @Nullable final String msg,
       @Nullable final Throwable tr) {
+    if (Boolean.TRUE.equals(isCapturing.get())) {
+      return;
+    }
     final @NotNull ScopesAdapter scopes = ScopesAdapter.getInstance();
     // Check if logs are enabled before doing expensive operations
     if (!scopes.getOptions().getLogs().isEnabled()) {
       return;
     }
-    final @Nullable String trMessage = tr != null ? tr.getMessage() : null;
-    final @NotNull SentryLogParameters params = new SentryLogParameters();
-    params.setOrigin("auto.log.logcat");
-
-    if (tr == null || trMessage == null) {
-      scopes.logger().log(level, params, msg);
-    } else {
-      scopes.logger().log(level, params, msg != null ? (msg + "\n" + trMessage) : trMessage);
+    isCapturing.set(Boolean.TRUE);
+    try {
+      final @Nullable String trMessage = tr != null ? tr.getMessage() : null;
+      final @NotNull SentryLogParameters params = new SentryLogParameters();
+      params.setOrigin("auto.log.logcat");
+
+      if (tr == null || trMessage == null) {
+        scopes.logger().log(level, params, msg);
+      } else {
+        scopes.logger().log(level, params, msg != null ? (msg + "\n" + trMessage) : trMessage);
+      }
+    } finally {
+      isCapturing.set(Boolean.FALSE);
     }
   }
 

sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt

@@ -2,6 +2,7 @@ package io.sentry.android.core
 
 import android.os.Bundle
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.google.common.truth.Truth.assertThat
 import io.sentry.Breadcrumb
 import io.sentry.Sentry
 import io.sentry.SentryLevel
@@ -202,6 +203,48 @@ class SentryLogcatAdapterTest {
     )
   }
 
+  @Test
+  fun `re-entrant logcat call while adding a breadcrumb does not recurse`() {
+    // Reproduces SDK-CRASHES-JAVA-3T3H: when the Sentry Android Gradle Plugin rewrites the app's
+    // Log.* calls to SentryLogcatAdapter.*, a beforeBreadcrumb callback that logs re-enters the
+    // adapter while a Logcat breadcrumb is being added, which without a guard recurses until a
+    // StackOverflowError.
+    fixture.initSut {
+      it.beforeBreadcrumb =
+        SentryOptions.BeforeBreadcrumbCallback { breadcrumb, _ ->
+          fixture.breadcrumbs.add(breadcrumb)
+          SentryLogcatAdapter.w(tag, "$commonMsg re-entrant")
+          breadcrumb
+        }
+    }
+
+    SentryLogcatAdapter.w(tag, "$commonMsg warning")
+
+    // The re-entrant call is skipped by the guard, so only the original breadcrumb is recorded.
+    assertThat(fixture.breadcrumbs).hasSize(1)
+    assertThat(fixture.breadcrumbs.first().message).isEqualTo("$commonMsg warning")
+  }
+
+  @Test
+  fun `re-entrant logcat call while sending a log does not recurse`() {
+    // Same recursion as the breadcrumb path, but via a beforeSendLog callback that logs while a
+    // Logcat log is being sent.
+    fixture.initSut {
+      it.logs.beforeSend =
+        SentryOptions.Logs.BeforeSendLogCallback { logEvent ->
+          fixture.logs.add(logEvent)
+          SentryLogcatAdapter.w(tag, "$commonMsg re-entrant")
+          logEvent
+        }
+    }
+
+    SentryLogcatAdapter.w(tag, "$commonMsg warning")
+
+    // The re-entrant call is skipped by the guard, so only the original log is recorded.
+    assertThat(fixture.logs).hasSize(1)
+    assertThat(fixture.logs.first().body).isEqualTo("$commonMsg warning")
+  }
+
   private fun Breadcrumb.assert(
     expectedTag: String,
     expectedMessage: String,