You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(core): Prevent recursion when a callback triggers another capture (#5737)
* 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>
* docs(changelog): Add entry for callback re-entrancy guard
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
* refactor(core): Hand out AutoCloseable token from re-entrancy guard
Replace the manual enter()/finally-exit() pattern at every callback site
with an ISentryLifecycleToken returned from enter(), used via
try-with-resources. This removes nine copies of the finally boilerplate
and the risk of forgetting the exit() call.
The depth counter stays as the guard's state model - the token's close()
just decrements it - so the nesting correctness guarantee is unchanged.
The token is a shared static singleton, so no allocation happens per
callback, matching the AutoClosableReentrantLock idiom already in the
codebase.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Format code
* docs(changelog): Move re-entrancy entry to Unreleased
The rebase folded the previous Unreleased section into the released
8.48.0, stranding the callback re-entrancy entry there. Move it back
under Unreleased.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(core): Guard sendEnvelope against callback re-entrancy
captureEnvelope and captureCheckIn route into sendEnvelope without the
isActive() entry check every other capture path has. A beforeEnvelope
callback that itself captured an envelope or check-in re-entered
sendEnvelope, re-ran the callback, and recursed to a StackOverflowError -
the same failure class the rest of this PR fixes.
Check the guard at the top of sendEnvelope, the single choke point every
send flows through, rather than adding a check to each public entry
point. In normal flow the guard is already inactive there (the before*
callback has exited), so an active guard can only mean a callback
triggered the send, which is dropped. This also closes the hole for any
future sender routed through sendEnvelope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(core): Drop redundant check-in re-entrancy test
captureCheckIn and captureEnvelope both funnel into sendEnvelope and hit
the same isActive() guard, so the check-in test exercised no code path
the envelope test did not already cover.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(core): Drop callback-recursion captures silently
The re-entrancy guard dropped nested captures but logged a DEBUG line at
seven of the eight drop sites. That log is itself routed back into Sentry
by the same logging integrations this guard protects against (the Gradle
plugin's logcat instrumentation feeds captureLog, whose own drop logs
again), so logging on the drop path re-opens the very recursion the guard
breaks. It only bites with SDK debug logging enabled, since the internal
logger is otherwise a no-op, but dropping silently removes the failure
mode outright.
Drop without logging everywhere and document why in the guard's Javadoc
and at each drop site.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(core): Document that captures inside before* callbacks are dropped
A customer implementing beforeSend/beforeBreadcrumb/beforeSendLog/etc.
has no way to know that capturing from within the callback — directly or
through a logging integration — is silently dropped to prevent recursion.
State the contract on each customer-facing callback interface.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* build(core): Update API dump for re-entrancy guard token
The token refactor changed SentryCallbackReentrancyGuard.enter() to
return an ISentryLifecycleToken and made exit() private, but the API dump
was not regenerated at the time. The guard is @ApiStatus.Internal.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
Copy file name to clipboardExpand all lines: CHANGELOG.md
+2Lines changed: 2 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -6,6 +6,8 @@
6
6
7
7
- Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762))
8
8
- `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789))
9
+
- Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, `beforeSendLog`, or `beforeEnvelope` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737))
10
+
- Captures made from within a user callback (event, transaction, breadcrumb, log, envelope, or check-in) are now dropped while that callback runs, instead of recursing. Captures made by event processors are unaffected.
0 commit comments