diff --git a/CHANGELOG.md b/CHANGELOG.md index 107b916808..9d0da03b0e 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 0388b7de48..f4813df42a 100644 --- a/sentry-android-core/build.gradle.kts +++ b/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) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java index 1e649c1783..3de8b941ef 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryLogcatAdapter.java +++ b/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 isCapturing = + new ThreadLocal() { + @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); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt index 1a84a1282d..ebe8022d15 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryLogcatAdapterTest.kt +++ b/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,