Skip to content

Commit 2b15346

Browse files
runningcodeclaude
andcommitted
fix(core): Prevent recursion when a callback triggers another capture
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 <noreply@anthropic.com>
1 parent 61ba1d5 commit 2b15346

7 files changed

Lines changed: 231 additions & 0 deletions

File tree

sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberIntegrationTest.kt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
package io.sentry.android.timber
22

33
import io.sentry.IScopes
4+
import io.sentry.ITransportFactory
5+
import io.sentry.ScopesAdapter
6+
import io.sentry.Sentry
47
import io.sentry.SentryLevel
58
import io.sentry.SentryLogLevel
69
import io.sentry.SentryOptions
710
import io.sentry.protocol.SdkVersion
11+
import io.sentry.transport.ITransport
812
import kotlin.test.BeforeTest
913
import kotlin.test.Test
1014
import kotlin.test.assertEquals
1115
import kotlin.test.assertTrue
1216
import org.mockito.kotlin.any
1317
import org.mockito.kotlin.mock
1418
import org.mockito.kotlin.verify
19+
import org.mockito.kotlin.whenever
1520
import timber.log.Timber
1621

1722
class SentryTimberIntegrationTest {
@@ -112,4 +117,44 @@ class SentryTimberIntegrationTest {
112117

113118
assertTrue(fixture.options.sdkVersion!!.integrationSet.contains("Timber"))
114119
}
120+
121+
@Test
122+
fun `a beforeSend callback that logs via Timber does not recurse`() {
123+
// End-to-end guard against SDK-CRASHES-JAVA-3T3H style recursion: with a real Sentry instance,
124+
// a beforeSend callback that logs through the planted SentryTimberTree must not loop back into
125+
// capture forever.
126+
val transport = mock<ITransport>()
127+
val transportFactory = mock<ITransportFactory>()
128+
whenever(transportFactory.create(any(), any())).thenReturn(transport)
129+
130+
var beforeSendInvocations = 0
131+
Sentry.init { options ->
132+
options.dsn = "https://key@sentry.io/123"
133+
options.setTransportFactory(transportFactory)
134+
options.beforeSend =
135+
SentryOptions.BeforeSendCallback { event, _ ->
136+
beforeSendInvocations++
137+
Timber.e("logging from beforeSend")
138+
event
139+
}
140+
}
141+
Timber.plant(
142+
SentryTimberTree(
143+
ScopesAdapter.getInstance(),
144+
SentryLevel.ERROR,
145+
SentryLevel.INFO,
146+
SentryLogLevel.INFO,
147+
)
148+
)
149+
150+
try {
151+
Timber.e("outer error")
152+
153+
// Without the core re-entrancy guard this recurses until a StackOverflowError. The nested
154+
// Timber.e is dropped before its own beforeSend, so the callback runs exactly once.
155+
assertEquals(1, beforeSendInvocations)
156+
} finally {
157+
Sentry.close()
158+
}
159+
}
115160
}

sentry/api/sentry.api

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7893,6 +7893,12 @@ public final class io/sentry/util/ScopesUtil {
78937893
public static fun printScopesChain (Lio/sentry/IScopes;)V
78947894
}
78957895

7896+
public final class io/sentry/util/SentryCallbackReentrancyGuard {
7897+
public static fun enter ()V
7898+
public static fun exit ()V
7899+
public static fun isActive ()Z
7900+
}
7901+
78967902
public final class io/sentry/util/SentryRandom {
78977903
public fun <init> ()V
78987904
public static fun current ()Lio/sentry/util/Random;

sentry/src/main/java/io/sentry/Scope.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import io.sentry.util.ExceptionUtils;
1717
import io.sentry.util.Objects;
1818
import io.sentry.util.Pair;
19+
import io.sentry.util.SentryCallbackReentrancyGuard;
1920
import java.lang.ref.WeakReference;
2021
import java.util.ArrayList;
2122
import java.util.Collection;
@@ -465,6 +466,7 @@ public Queue<Breadcrumb> getBreadcrumbs() {
465466
@NotNull Breadcrumb breadcrumb,
466467
final @NotNull Hint hint) {
467468
try {
469+
SentryCallbackReentrancyGuard.enter();
468470
breadcrumb = callback.execute(breadcrumb, hint);
469471
} catch (Throwable e) {
470472
options
@@ -477,6 +479,8 @@ public Queue<Breadcrumb> getBreadcrumbs() {
477479
if (e.getMessage() != null) {
478480
breadcrumb.setData("sentry:message", e.getMessage());
479481
}
482+
} finally {
483+
SentryCallbackReentrancyGuard.exit();
480484
}
481485
return breadcrumb;
482486
}
@@ -493,6 +497,10 @@ public void addBreadcrumb(@NotNull Breadcrumb breadcrumb, @Nullable Hint hint) {
493497
if (breadcrumb == null || breadcrumbs instanceof DisabledQueue) {
494498
return;
495499
}
500+
// Drop breadcrumbs added from within a user callback to prevent recursion.
501+
if (SentryCallbackReentrancyGuard.isActive()) {
502+
return;
503+
}
496504
SentryOptions.BeforeBreadcrumbCallback callback = options.getBeforeBreadcrumb();
497505
if (callback != null) {
498506
if (hint == null) {

sentry/src/main/java/io/sentry/SentryClient.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,15 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
107107
@NotNull SentryEvent event, final @Nullable IScope scope, @Nullable Hint hint) {
108108
Objects.requireNonNull(event, "SentryEvent is required.");
109109

110+
if (SentryCallbackReentrancyGuard.isActive()) {
111+
options
112+
.getLogger()
113+
.log(
114+
SentryLevel.DEBUG,
115+
"Event captured from within a callback (beforeSend/beforeBreadcrumb/beforeSendLog) was dropped to prevent recursion.");
116+
return SentryId.EMPTY_ID;
117+
}
118+
110119
if (hint == null) {
111120
hint = new Hint();
112121
}
@@ -957,6 +966,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
957966
final @Nullable ProfilingTraceData profilingTraceData) {
958967
Objects.requireNonNull(transaction, "Transaction is required.");
959968

969+
if (SentryCallbackReentrancyGuard.isActive()) {
970+
options
971+
.getLogger()
972+
.log(
973+
SentryLevel.DEBUG,
974+
"Transaction captured from within a callback was dropped to prevent recursion.");
975+
return SentryId.EMPTY_ID;
976+
}
977+
960978
if (hint == null) {
961979
hint = new Hint();
962980
}
@@ -1281,6 +1299,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
12811299
@ApiStatus.Experimental
12821300
@Override
12831301
public void captureLog(@Nullable SentryLogEvent logEvent, @Nullable IScope scope) {
1302+
if (SentryCallbackReentrancyGuard.isActive()) {
1303+
options
1304+
.getLogger()
1305+
.log(
1306+
SentryLevel.DEBUG,
1307+
"Log captured from within a callback was dropped to prevent recursion.");
1308+
return;
1309+
}
1310+
12841311
if (logEvent != null && scope != null) {
12851312
logEvent = processLogEvent(logEvent, scope.getEventProcessors());
12861313
if (logEvent == null) {
@@ -1595,6 +1622,7 @@ private void sortBreadcrumbsByDate(
15951622
final SentryOptions.BeforeSendCallback beforeSend = options.getBeforeSend();
15961623
if (beforeSend != null) {
15971624
try {
1625+
SentryCallbackReentrancyGuard.enter();
15981626
event = beforeSend.execute(event, hint);
15991627
} catch (Throwable e) {
16001628
options
@@ -1606,6 +1634,8 @@ private void sortBreadcrumbsByDate(
16061634

16071635
// drop event in case of an error in beforeSend due to PII concerns
16081636
event = null;
1637+
} finally {
1638+
SentryCallbackReentrancyGuard.exit();
16091639
}
16101640
}
16111641
return event;
@@ -1617,6 +1647,7 @@ private void sortBreadcrumbsByDate(
16171647
options.getBeforeSendTransaction();
16181648
if (beforeSendTransaction != null) {
16191649
try {
1650+
SentryCallbackReentrancyGuard.enter();
16201651
transaction = beforeSendTransaction.execute(transaction, hint);
16211652
} catch (Throwable e) {
16221653
options
@@ -1628,6 +1659,8 @@ private void sortBreadcrumbsByDate(
16281659

16291660
// drop transaction in case of an error in beforeSend due to PII concerns
16301661
transaction = null;
1662+
} finally {
1663+
SentryCallbackReentrancyGuard.exit();
16311664
}
16321665
}
16331666
return transaction;
@@ -1638,6 +1671,7 @@ private void sortBreadcrumbsByDate(
16381671
final SentryOptions.BeforeSendCallback beforeSendFeedback = options.getBeforeSendFeedback();
16391672
if (beforeSendFeedback != null) {
16401673
try {
1674+
SentryCallbackReentrancyGuard.enter();
16411675
event = beforeSendFeedback.execute(event, hint);
16421676
} catch (Throwable e) {
16431677
options
@@ -1646,6 +1680,8 @@ private void sortBreadcrumbsByDate(
16461680

16471681
// drop feedback in case of an error in beforeSend due to PII concerns
16481682
event = null;
1683+
} finally {
1684+
SentryCallbackReentrancyGuard.exit();
16491685
}
16501686
}
16511687
return event;
@@ -1656,6 +1692,7 @@ private void sortBreadcrumbsByDate(
16561692
final SentryOptions.BeforeSendReplayCallback beforeSendReplay = options.getBeforeSendReplay();
16571693
if (beforeSendReplay != null) {
16581694
try {
1695+
SentryCallbackReentrancyGuard.enter();
16591696
event = beforeSendReplay.execute(event, hint);
16601697
} catch (Throwable e) {
16611698
options
@@ -1667,6 +1704,8 @@ private void sortBreadcrumbsByDate(
16671704

16681705
// drop event in case of an error in beforeSend due to PII concerns
16691706
event = null;
1707+
} finally {
1708+
SentryCallbackReentrancyGuard.exit();
16701709
}
16711710
}
16721711
return event;
@@ -1677,6 +1716,7 @@ private void sortBreadcrumbsByDate(
16771716
options.getLogs().getBeforeSend();
16781717
if (beforeSendLog != null) {
16791718
try {
1719+
SentryCallbackReentrancyGuard.enter();
16801720
event = beforeSendLog.execute(event);
16811721
} catch (Throwable e) {
16821722
options
@@ -1688,6 +1728,8 @@ private void sortBreadcrumbsByDate(
16881728

16891729
// drop event in case of an error in beforeSendLog due to PII concerns
16901730
event = null;
1731+
} finally {
1732+
SentryCallbackReentrancyGuard.exit();
16911733
}
16921734
}
16931735
return event;
@@ -1699,6 +1741,7 @@ private void sortBreadcrumbsByDate(
16991741
options.getMetrics().getBeforeSend();
17001742
if (beforeSendMetric != null) {
17011743
try {
1744+
SentryCallbackReentrancyGuard.enter();
17021745
event = beforeSendMetric.execute(event, hint);
17031746
} catch (Throwable e) {
17041747
options
@@ -1710,6 +1753,8 @@ private void sortBreadcrumbsByDate(
17101753

17111754
// drop event in case of an error in beforeSendMetric due to PII concerns
17121755
event = null;
1756+
} finally {
1757+
SentryCallbackReentrancyGuard.exit();
17131758
}
17141759
}
17151760
return event;
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package io.sentry.util;
2+
3+
import org.jetbrains.annotations.ApiStatus;
4+
5+
/**
6+
* Thread-local re-entrancy guard that marks whether a user-supplied {@code before*} callback
7+
* ({@code beforeSend}, {@code beforeBreadcrumb}, {@code beforeSendLog}, ...) is currently executing
8+
* on the current thread.
9+
*
10+
* <p>A callback that itself triggers another SDK capture on the same thread — directly, or
11+
* transitively through a logging integration that routes back into Sentry (e.g. Timber or the
12+
* Gradle plugin's logcat instrumentation) — would otherwise recurse indefinitely and throw {@link
13+
* StackOverflowError}. Capture entry points consult {@link #isActive()} and drop the nested capture
14+
* while a callback is running.
15+
*
16+
* <p>The flag is set ONLY around each callback's {@code execute(...)} invocation, never around the
17+
* whole capture pipeline, so captures made by event processors (which run outside the callback) are
18+
* not affected.
19+
*/
20+
@ApiStatus.Internal
21+
public final class SentryCallbackReentrancyGuard {
22+
23+
private static final ThreadLocal<Boolean> isRunning = new ThreadLocal<>();
24+
25+
private SentryCallbackReentrancyGuard() {}
26+
27+
/** Whether a user callback is currently executing on this thread. */
28+
public static boolean isActive() {
29+
return Boolean.TRUE.equals(isRunning.get());
30+
}
31+
32+
/** Marks that a user callback is starting to execute on this thread. */
33+
public static void enter() {
34+
isRunning.set(Boolean.TRUE);
35+
}
36+
37+
/** Marks that the user callback finished executing on this thread. */
38+
public static void exit() {
39+
isRunning.set(Boolean.FALSE);
40+
}
41+
}

sentry/src/test/java/io/sentry/ScopeTest.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,28 @@ class ScopeTest {
372372
assertFalse(called)
373373
}
374374

375+
@Test
376+
fun `when beforeBreadcrumb adds another breadcrumb, the nested breadcrumb is dropped and does not recurse`() {
377+
var invocations = 0
378+
lateinit var scope: Scope
379+
val options =
380+
SentryOptions().apply {
381+
beforeBreadcrumb =
382+
SentryOptions.BeforeBreadcrumbCallback { breadcrumb, _ ->
383+
invocations++
384+
scope.addBreadcrumb(Breadcrumb())
385+
breadcrumb
386+
}
387+
}
388+
389+
scope = Scope(options)
390+
scope.addBreadcrumb(Breadcrumb())
391+
392+
// Callback runs only for the outer breadcrumb; the nested one is dropped before its callback.
393+
assertEquals(1, invocations)
394+
assertEquals(1, scope.breadcrumbs.count())
395+
}
396+
375397
@Test
376398
fun `when adding breadcrumb and maxBreadcrumb is not 0, beforeBreadcrumb is executed`() {
377399
var called = false

0 commit comments

Comments
 (0)