From 2b15346c7ec6ed511e54733aed50373867abb834 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 7 Jul 2026 15:43:10 +0200 Subject: [PATCH 1/4] fix(core): Prevent recursion when a callback triggers another capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user beforeSend/beforeBreadcrumb/beforeSendLog callback that itself captures — directly, or transitively through a logging integration that routes back into Sentry (e.g. Timber, or the Gradle plugin's logcat instrumentation) — recursed until a StackOverflowError, because the callbacks run synchronously on the caller thread with no re-entrancy protection. Add a shared, thread-local SentryCallbackReentrancyGuard that is set only while a callback's execute() runs. Capture entry points (captureEvent, captureTransaction, captureLog, Scope.addBreadcrumb) drop nested captures while the guard is active, breaking the loop for every capture type at once. Dropping (rather than sending) the nested capture is intentional: bypassing beforeSend would send unscrubbed data. The nested capture is dropped, not sent. This also makes the per-integration guard in SentryLogcatAdapter redundant. Co-Authored-By: Claude Opus 4.8 --- .../timber/SentryTimberIntegrationTest.kt | 45 +++++++++++++ sentry/api/sentry.api | 6 ++ sentry/src/main/java/io/sentry/Scope.java | 8 +++ .../src/main/java/io/sentry/SentryClient.java | 45 +++++++++++++ .../util/SentryCallbackReentrancyGuard.java | 41 ++++++++++++ sentry/src/test/java/io/sentry/ScopeTest.kt | 22 +++++++ .../test/java/io/sentry/SentryClientTest.kt | 64 +++++++++++++++++++ 7 files changed, 231 insertions(+) create mode 100644 sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java diff --git a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt index 43a45da7bb3..3c2cabfd5cc 100644 --- a/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt +++ b/sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt @@ -1,10 +1,14 @@ package io.sentry.android.timber import io.sentry.IScopes +import io.sentry.ITransportFactory +import io.sentry.ScopesAdapter +import io.sentry.Sentry import io.sentry.SentryLevel import io.sentry.SentryLogLevel import io.sentry.SentryOptions import io.sentry.protocol.SdkVersion +import io.sentry.transport.ITransport import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -12,6 +16,7 @@ import kotlin.test.assertTrue import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import timber.log.Timber class SentryTimberIntegrationTest { @@ -112,4 +117,44 @@ class SentryTimberIntegrationTest { assertTrue(fixture.options.sdkVersion!!.integrationSet.contains("Timber")) } + + @Test + fun `a beforeSend callback that logs via Timber does not recurse`() { + // End-to-end guard against SDK-CRASHES-JAVA-3T3H style recursion: with a real Sentry instance, + // a beforeSend callback that logs through the planted SentryTimberTree must not loop back into + // capture forever. + val transport = mock() + val transportFactory = mock() + whenever(transportFactory.create(any(), any())).thenReturn(transport) + + var beforeSendInvocations = 0 + Sentry.init { options -> + options.dsn = "https://key@sentry.io/123" + options.setTransportFactory(transportFactory) + options.beforeSend = + SentryOptions.BeforeSendCallback { event, _ -> + beforeSendInvocations++ + Timber.e("logging from beforeSend") + event + } + } + Timber.plant( + SentryTimberTree( + ScopesAdapter.getInstance(), + SentryLevel.ERROR, + SentryLevel.INFO, + SentryLogLevel.INFO, + ) + ) + + try { + Timber.e("outer error") + + // Without the core re-entrancy guard this recurses until a StackOverflowError. The nested + // Timber.e is dropped before its own beforeSend, so the callback runs exactly once. + assertEquals(1, beforeSendInvocations) + } finally { + Sentry.close() + } + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 09ede4804a3..a80618412db 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7893,6 +7893,12 @@ public final class io/sentry/util/ScopesUtil { public static fun printScopesChain (Lio/sentry/IScopes;)V } +public final class io/sentry/util/SentryCallbackReentrancyGuard { + public static fun enter ()V + public static fun exit ()V + public static fun isActive ()Z +} + public final class io/sentry/util/SentryRandom { public fun ()V public static fun current ()Lio/sentry/util/Random; diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index f5c57f5ac5d..ad5acd17ee8 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -16,6 +16,7 @@ import io.sentry.util.ExceptionUtils; import io.sentry.util.Objects; import io.sentry.util.Pair; +import io.sentry.util.SentryCallbackReentrancyGuard; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; @@ -465,6 +466,7 @@ public Queue getBreadcrumbs() { @NotNull Breadcrumb breadcrumb, final @NotNull Hint hint) { try { + SentryCallbackReentrancyGuard.enter(); breadcrumb = callback.execute(breadcrumb, hint); } catch (Throwable e) { options @@ -477,6 +479,8 @@ public Queue getBreadcrumbs() { if (e.getMessage() != null) { breadcrumb.setData("sentry:message", e.getMessage()); } + } finally { + SentryCallbackReentrancyGuard.exit(); } return breadcrumb; } @@ -493,6 +497,10 @@ public void addBreadcrumb(@NotNull Breadcrumb breadcrumb, @Nullable Hint hint) { if (breadcrumb == null || breadcrumbs instanceof DisabledQueue) { return; } + // Drop breadcrumbs added from within a user callback to prevent recursion. + if (SentryCallbackReentrancyGuard.isActive()) { + return; + } SentryOptions.BeforeBreadcrumbCallback callback = options.getBeforeBreadcrumb(); if (callback != null) { if (hint == null) { diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 02934b1ed7d..8af7839c661 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -107,6 +107,15 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul @NotNull SentryEvent event, final @Nullable IScope scope, @Nullable Hint hint) { Objects.requireNonNull(event, "SentryEvent is required."); + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Event captured from within a callback (beforeSend/beforeBreadcrumb/beforeSendLog) was dropped to prevent recursion."); + return SentryId.EMPTY_ID; + } + if (hint == null) { hint = new Hint(); } @@ -957,6 +966,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint final @Nullable ProfilingTraceData profilingTraceData) { Objects.requireNonNull(transaction, "Transaction is required."); + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Transaction captured from within a callback was dropped to prevent recursion."); + return SentryId.EMPTY_ID; + } + if (hint == null) { hint = new Hint(); } @@ -1281,6 +1299,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint @ApiStatus.Experimental @Override public void captureLog(@Nullable SentryLogEvent logEvent, @Nullable IScope scope) { + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Log captured from within a callback was dropped to prevent recursion."); + return; + } + if (logEvent != null && scope != null) { logEvent = processLogEvent(logEvent, scope.getEventProcessors()); if (logEvent == null) { @@ -1595,6 +1622,7 @@ private void sortBreadcrumbsByDate( final SentryOptions.BeforeSendCallback beforeSend = options.getBeforeSend(); if (beforeSend != null) { try { + SentryCallbackReentrancyGuard.enter(); event = beforeSend.execute(event, hint); } catch (Throwable e) { options @@ -1606,6 +1634,8 @@ private void sortBreadcrumbsByDate( // drop event in case of an error in beforeSend due to PII concerns event = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return event; @@ -1617,6 +1647,7 @@ private void sortBreadcrumbsByDate( options.getBeforeSendTransaction(); if (beforeSendTransaction != null) { try { + SentryCallbackReentrancyGuard.enter(); transaction = beforeSendTransaction.execute(transaction, hint); } catch (Throwable e) { options @@ -1628,6 +1659,8 @@ private void sortBreadcrumbsByDate( // drop transaction in case of an error in beforeSend due to PII concerns transaction = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return transaction; @@ -1638,6 +1671,7 @@ private void sortBreadcrumbsByDate( final SentryOptions.BeforeSendCallback beforeSendFeedback = options.getBeforeSendFeedback(); if (beforeSendFeedback != null) { try { + SentryCallbackReentrancyGuard.enter(); event = beforeSendFeedback.execute(event, hint); } catch (Throwable e) { options @@ -1646,6 +1680,8 @@ private void sortBreadcrumbsByDate( // drop feedback in case of an error in beforeSend due to PII concerns event = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return event; @@ -1656,6 +1692,7 @@ private void sortBreadcrumbsByDate( final SentryOptions.BeforeSendReplayCallback beforeSendReplay = options.getBeforeSendReplay(); if (beforeSendReplay != null) { try { + SentryCallbackReentrancyGuard.enter(); event = beforeSendReplay.execute(event, hint); } catch (Throwable e) { options @@ -1667,6 +1704,8 @@ private void sortBreadcrumbsByDate( // drop event in case of an error in beforeSend due to PII concerns event = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return event; @@ -1677,6 +1716,7 @@ private void sortBreadcrumbsByDate( options.getLogs().getBeforeSend(); if (beforeSendLog != null) { try { + SentryCallbackReentrancyGuard.enter(); event = beforeSendLog.execute(event); } catch (Throwable e) { options @@ -1688,6 +1728,8 @@ private void sortBreadcrumbsByDate( // drop event in case of an error in beforeSendLog due to PII concerns event = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return event; @@ -1699,6 +1741,7 @@ private void sortBreadcrumbsByDate( options.getMetrics().getBeforeSend(); if (beforeSendMetric != null) { try { + SentryCallbackReentrancyGuard.enter(); event = beforeSendMetric.execute(event, hint); } catch (Throwable e) { options @@ -1710,6 +1753,8 @@ private void sortBreadcrumbsByDate( // drop event in case of an error in beforeSendMetric due to PII concerns event = null; + } finally { + SentryCallbackReentrancyGuard.exit(); } } return event; diff --git a/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java b/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java new file mode 100644 index 00000000000..afd303731fa --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java @@ -0,0 +1,41 @@ +package io.sentry.util; + +import org.jetbrains.annotations.ApiStatus; + +/** + * Thread-local re-entrancy guard that marks whether a user-supplied {@code before*} callback + * ({@code beforeSend}, {@code beforeBreadcrumb}, {@code beforeSendLog}, ...) is currently executing + * on the current thread. + * + *

A callback that itself triggers another SDK capture on the same thread — directly, or + * transitively through a logging integration that routes back into Sentry (e.g. Timber or the + * Gradle plugin's logcat instrumentation) — would otherwise recurse indefinitely and throw {@link + * StackOverflowError}. Capture entry points consult {@link #isActive()} and drop the nested capture + * while a callback is running. + * + *

The flag is set ONLY around each callback's {@code execute(...)} invocation, never around the + * whole capture pipeline, so captures made by event processors (which run outside the callback) are + * not affected. + */ +@ApiStatus.Internal +public final class SentryCallbackReentrancyGuard { + + private static final ThreadLocal isRunning = new ThreadLocal<>(); + + private SentryCallbackReentrancyGuard() {} + + /** Whether a user callback is currently executing on this thread. */ + public static boolean isActive() { + return Boolean.TRUE.equals(isRunning.get()); + } + + /** Marks that a user callback is starting to execute on this thread. */ + public static void enter() { + isRunning.set(Boolean.TRUE); + } + + /** Marks that the user callback finished executing on this thread. */ + public static void exit() { + isRunning.set(Boolean.FALSE); + } +} diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index 4b0047fdc18..bf1eda4c747 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/sentry/src/test/java/io/sentry/ScopeTest.kt @@ -372,6 +372,28 @@ class ScopeTest { assertFalse(called) } + @Test + fun `when beforeBreadcrumb adds another breadcrumb, the nested breadcrumb is dropped and does not recurse`() { + var invocations = 0 + lateinit var scope: Scope + val options = + SentryOptions().apply { + beforeBreadcrumb = + SentryOptions.BeforeBreadcrumbCallback { breadcrumb, _ -> + invocations++ + scope.addBreadcrumb(Breadcrumb()) + breadcrumb + } + } + + scope = Scope(options) + scope.addBreadcrumb(Breadcrumb()) + + // Callback runs only for the outer breadcrumb; the nested one is dropped before its callback. + assertEquals(1, invocations) + assertEquals(1, scope.breadcrumbs.count()) + } + @Test fun `when adding breadcrumb and maxBreadcrumb is not 0, beforeBreadcrumb is executed`() { var called = false diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 5b407bfef3d..8442e6143f6 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -284,6 +284,70 @@ class SentryClientTest { ) } + @Test + fun `when beforeSend captures another event, the nested capture is dropped and does not recurse`() { + var invocations = 0 + lateinit var sut: SentryClient + fixture.sentryOptions.setBeforeSend { e, _ -> + invocations++ + sut.captureEvent(SentryEvent()) + e + } + sut = fixture.getSut() + + sut.captureEvent(SentryEvent()) + + // Callback runs only for the outer event; the nested capture is dropped before its callback. + assertEquals(1, invocations) + verify(fixture.transport, times(1)).send(any(), anyOrNull()) + } + + @Test + fun `when beforeSend captures a log, the nested log is dropped`() { + val scope = createScope() + fixture.sentryOptions.logs.isEnabled = true + lateinit var sut: SentryClient + fixture.sentryOptions.setBeforeSend { e, _ -> + sut.captureLog( + SentryLogEvent(SentryId(), SentryNanotimeDate(), "nested", SentryLogLevel.WARN), + scope, + ) + e + } + sut = fixture.getSut() + + sut.captureEvent(SentryEvent()) + + // The shared guard spans capture types: a log emitted from beforeSend is dropped too. + verify(fixture.loggerBatchProcessor, never()).add(any()) + verify(fixture.transport, times(1)).send(any(), anyOrNull()) + } + + @Test + fun `when beforeSendLog logs again, the nested log is dropped and does not recurse`() { + val scope = createScope() + fixture.sentryOptions.logs.isEnabled = true + var invocations = 0 + lateinit var sut: SentryClient + fixture.sentryOptions.logs.setBeforeSend { l -> + invocations++ + sut.captureLog( + SentryLogEvent(SentryId(), SentryNanotimeDate(), "nested", SentryLogLevel.WARN), + scope, + ) + l + } + sut = fixture.getSut() + + sut.captureLog( + SentryLogEvent(SentryId(), SentryNanotimeDate(), "outer", SentryLogLevel.WARN), + scope, + ) + + assertEquals(1, invocations) + verify(fixture.loggerBatchProcessor, times(1)).add(any()) + } + @Test fun `when beforeSendLog is set, callback is invoked`() { val scope = createScope() From 1c810b5a9a3291ad8e1c241ad6401b9121e6e66f Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 7 Jul 2026 15:44:03 +0200 Subject: [PATCH 2/4] docs(changelog): Add entry for callback re-entrancy guard Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 107b916808a..5a97ba246a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ ### Fixes +- Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, or `beforeSendLog` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737)) + - Captures made from within a user callback (event, transaction, breadcrumb, or log) are now dropped while that callback runs, instead of recursing. Captures made by event processors are unaffected. - 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)) From 67371e2562b5bb534f1f46bf97da3f4bcda2f63b Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 7 Jul 2026 15:59:02 +0200 Subject: [PATCH 3/4] fix(core): Guard captureFeedback, captureReplayEvent, captureMetric too These three capture methods had their beforeSend* executors wrapped by the re-entrancy guard but no entry check, so they were never dropped when a callback was active. That broke the "callbacks never nest" invariant: a callback that captured feedback/replay/metric ran that executor, whose exit() cleared the shared flag mid-callback, re-enabling recursion for any captureEvent/captureLog that followed in the same callback. They also recursed directly (e.g. beforeSendFeedback -> captureFeedback). Add the same isActive() entry guard to all three so every executor-wrapped capture path also drops while a callback runs. Co-Authored-By: Claude Opus 4.8 --- .../src/main/java/io/sentry/SentryClient.java | 27 +++++++++++++ .../test/java/io/sentry/SentryClientTest.kt | 38 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 8af7839c661..fb0233d93b6 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -309,6 +309,15 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin @NotNull SentryReplayEvent event, final @Nullable IScope scope, @Nullable Hint hint) { Objects.requireNonNull(event, "SessionReplay is required."); + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Replay event captured from within a callback was dropped to prevent recursion."); + return SentryId.EMPTY_ID; + } + if (hint == null) { hint = new Hint(); } @@ -1189,6 +1198,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint @Override public @NotNull SentryId captureFeedback( final @NotNull Feedback feedback, @Nullable Hint hint, final @NotNull IScope scope) { + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Feedback captured from within a callback was dropped to prevent recursion."); + return SentryId.EMPTY_ID; + } + SentryEvent event = new SentryEvent(); event.getContexts().setFeedback(feedback); @@ -1362,6 +1380,15 @@ public void captureMetric( @Nullable SentryMetricsEvent metricsEvent, final @Nullable IScope scope, @Nullable Hint hint) { + if (SentryCallbackReentrancyGuard.isActive()) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Metric captured from within a callback was dropped to prevent recursion."); + return; + } + if (hint == null) { hint = new Hint(); } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 8442e6143f6..cfe81ca8487 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -348,6 +348,44 @@ class SentryClientTest { verify(fixture.loggerBatchProcessor, times(1)).add(any()) } + @Test + fun `when beforeSend captures feedback before an event, the guard is not cleared prematurely`() { + val scope = createScope() + var invocations = 0 + lateinit var sut: SentryClient + fixture.sentryOptions.setBeforeSend { e, _ -> + invocations++ + // Capturing feedback must not clear the re-entrancy guard for captures that follow it in the + // same callback, otherwise the captureEvent below would recurse. + sut.captureFeedback(Feedback("feedback"), null, scope) + sut.captureEvent(SentryEvent()) + e + } + sut = fixture.getSut() + + sut.captureEvent(SentryEvent()) + + assertEquals(1, invocations) + verify(fixture.transport, times(1)).send(any(), anyOrNull()) + } + + @Test + fun `when beforeSendFeedback captures feedback again, the nested capture is dropped and does not recurse`() { + val scope = createScope() + var invocations = 0 + lateinit var sut: SentryClient + fixture.sentryOptions.setBeforeSendFeedback { e, _ -> + invocations++ + sut.captureFeedback(Feedback("nested"), null, scope) + e + } + sut = fixture.getSut() + + sut.captureFeedback(Feedback("outer"), null, scope) + + assertEquals(1, invocations) + } + @Test fun `when beforeSendLog is set, callback is invoked`() { val scope = createScope() From 4c2bc51e70b98c2c321a0f8e56f3a43410b47064 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 7 Jul 2026 17:22:28 +0200 Subject: [PATCH 4/4] fix(core): Harden re-entrancy guard with depth counter, wrap remaining callbacks Two robustness fixes for the callback re-entrancy guard: Replace the boolean flag with a depth counter so a nested exit() cannot disarm the guard while an outer callback is still running. The boolean relied on the "callbacks never nest" invariant, which every capture entry point must uphold by convention - a future capture path added without an entry check would silently re-open the recursion hole. The counter makes that failure mode structurally impossible, and lets exit() remove() the thread-local entry instead of parking a stale value on pooled threads. Wrap beforeErrorSampling and beforeEnvelopeCallback, the two remaining user callbacks in SentryClient without enter()/exit(). A capture from within beforeErrorSampling recursed unguarded (captureEvent -> beforeErrorSampling -> captureEvent -> ...). Both regression tests were verified to fail with StackOverflowError without the wraps. Co-Authored-By: Claude Fable 5 --- .../src/main/java/io/sentry/SentryClient.java | 6 +++ .../util/SentryCallbackReentrancyGuard.java | 23 +++++++++--- .../test/java/io/sentry/SentryClientTest.kt | 37 +++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index fb0233d93b6..be77e3a469f 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -246,6 +246,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul options.getSessionReplay().getBeforeErrorSampling(); if (beforeErrorSampling != null) { try { + SentryCallbackReentrancyGuard.enter(); shouldCaptureReplay = beforeErrorSampling.execute(event, hint); } catch (Throwable e) { options @@ -255,6 +256,8 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul "The beforeErrorSampling callback threw an exception. Proceeding with replay capture.", e); shouldCaptureReplay = true; + } finally { + SentryCallbackReentrancyGuard.exit(); } } if (shouldCaptureReplay) { @@ -947,11 +950,14 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint options.getBeforeEnvelopeCallback(); if (beforeEnvelopeCallback != null) { try { + SentryCallbackReentrancyGuard.enter(); beforeEnvelopeCallback.execute(envelope, hint); } catch (Throwable e) { options .getLogger() .log(SentryLevel.ERROR, "The BeforeEnvelope callback threw an exception.", e); + } finally { + SentryCallbackReentrancyGuard.exit(); } } diff --git a/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java b/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java index afd303731fa..50af3fbb07e 100644 --- a/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java +++ b/sentry/src/main/java/io/sentry/util/SentryCallbackReentrancyGuard.java @@ -1,6 +1,7 @@ package io.sentry.util; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Nullable; /** * Thread-local re-entrancy guard that marks whether a user-supplied {@code before*} callback @@ -13,29 +14,41 @@ * StackOverflowError}. Capture entry points consult {@link #isActive()} and drop the nested capture * while a callback is running. * - *

The flag is set ONLY around each callback's {@code execute(...)} invocation, never around the + *

The guard is set ONLY around each callback's {@code execute(...)} invocation, never around the * whole capture pipeline, so captures made by event processors (which run outside the callback) are * not affected. + * + *

The guard is a depth counter rather than a boolean so that nested {@link #exit()} calls cannot + * clear it while an outer callback is still running. Capture entry points drop while a callback is + * active, so callbacks should never nest — but a capture path lacking an entry check must not + * silently disarm the guard for the rest of the outer callback. */ @ApiStatus.Internal public final class SentryCallbackReentrancyGuard { - private static final ThreadLocal isRunning = new ThreadLocal<>(); + private static final ThreadLocal depth = new ThreadLocal<>(); private SentryCallbackReentrancyGuard() {} /** Whether a user callback is currently executing on this thread. */ public static boolean isActive() { - return Boolean.TRUE.equals(isRunning.get()); + final @Nullable Integer current = depth.get(); + return current != null && current > 0; } /** Marks that a user callback is starting to execute on this thread. */ public static void enter() { - isRunning.set(Boolean.TRUE); + final @Nullable Integer current = depth.get(); + depth.set(current == null ? 1 : current + 1); } /** Marks that the user callback finished executing on this thread. */ public static void exit() { - isRunning.set(Boolean.FALSE); + final @Nullable Integer current = depth.get(); + if (current == null || current <= 1) { + depth.remove(); + } else { + depth.set(current - 1); + } } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index cfe81ca8487..e7ef9ece193 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -3227,6 +3227,25 @@ class SentryClientTest { assertTrue(beforeEnvelopeCalled) } + @Test + fun `when beforeEnvelopeCallback captures another event, the nested capture is dropped and does not recurse`() { + var invocations = 0 + lateinit var sut: SentryClient + val options = { options: SentryOptions -> + options.beforeEnvelopeCallback = + SentryOptions.BeforeEnvelopeCallback { _, _ -> + invocations++ + sut.captureEvent(SentryEvent()) + } + } + sut = fixture.getSut(options) + + sut.captureEvent(SentryEvent(), Hint()) + + assertEquals(1, invocations) + verify(fixture.transport, times(1)).send(any(), anyOrNull()) + } + @Test fun `beforeEnvelopeCallback may fail, but the transport is still sends the envelope `() { val sut = @@ -3455,6 +3474,24 @@ class SentryClientTest { assertFalse(called) } + @Test + fun `when beforeErrorSampling captures another event, the nested capture is dropped and does not recurse`() { + var invocations = 0 + lateinit var sut: SentryClient + fixture.sentryOptions.sessionReplay.beforeErrorSampling = + SentryReplayOptions.BeforeErrorSamplingCallback { _, _ -> + invocations++ + sut.captureEvent(SentryEvent().apply { exceptions = listOf(SentryException()) }) + true + } + sut = fixture.getSut() + + sut.captureEvent(SentryEvent().apply { exceptions = listOf(SentryException()) }) + + assertEquals(1, invocations) + verify(fixture.transport, times(1)).send(any(), anyOrNull()) + } + @Test fun `beforeErrorSampling returning false skips captureReplay`() { var called = false