Skip to content

fix(core): Make async event processing safe for crash and shutdown paths - #5878

Draft
runningcode wants to merge 4 commits into
mainfrom
no/async-event-processing-concurrency-fixes
Draft

fix(core): Make async event processing safe for crash and shutdown paths#5878
runningcode wants to merge 4 commits into
mainfrom
no/async-event-processing-concurrency-fixes

Conversation

@runningcode

Copy link
Copy Markdown
Contributor

📜 Description

Supersedes #5558. Same feature, rebased onto current main, with four concurrency fixes found in review.

The original PR moved the late processing stage to another thread without establishing what state that work is allowed to touch. The fixes narrow the async region to work that is actually safe to defer.

1. close() drained downstream before quiescing the async stage. Tasks finishing after the transport flush handed envelopes to a transport that was already closing, and anything still queued when the timeout expired was dropped by shutdownNow() with no client report. With isRestarting=true the timeout is 0, so the whole queue vanished silently. The async stage is now quiesced first, and undrained work is recorded as queue_overflow for its data category.

2. Crash captures went through the queue. UncaughtExceptionHandlerIntegration blocks on waitFlush() until the transport reports the event flushed to disk before letting the process die. Queueing put the fatal event behind everything already enqueued — with the default maxQueueSize of 30, the 15s flush timeout could expire before the crash was ever sent. Captures whose hint implements DiskFlushNotification, Backfillable, Cached, or TransactionEnd now run inline; the latter three also carry retry/delete state or force-finish a transaction that the caller depends on synchronously. This covers ANRv2, tombstones, and outbox replay as well.

3. The async task read a live scope. The scope handed to capture* is a CombinedScopeView over the caller's thread-local scope. By the time the async stage ran, the caller may have pushed or popped scopes, changed tags, or started a different transaction — so finalizeTransaction could force-finish a transaction unrelated to the event. The scope is now cloned at submit time.

4. Hint was shared mutable state. Its attachment list and five reference fields were unsynchronized while both threads could still reach them. The list is now guarded by the existing lock and the single-reference fields are volatile.

Also stops holding the executor monitor across awaitTermination, which could stall a capture on an app thread for the entire shutdown timeout — an ANR on Android's main thread.

💡 Motivation and Context

Issues 1 and 2 lose events outright; issue 3 can corrupt unrelated transactions. All three are reachable with enableAsyncProcessing=true on paths users do not control.

The feature remains opt-in and defaults to false, so behavior is unchanged unless enabled.

💚 How did you test it?

Nine tests across SentryClientTest and a new AsyncEventProcessingExecutorTest. Each was verified to fail against the pre-fix implementation and pass after — the two that initially passed either way were reworked until they discriminated.

./gradlew :sentry:check :sentry-android-core:testReleaseUnitTest passes.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Issues raised in review that this PR does not address, since they are design questions rather than correctness bugs:

  • One FIFO queue is shared by errors, transactions, replays, logs, and metrics, with capacity reused from maxQueueSize (default 30). A metrics-heavy app can fill it and starve errors. Worth a dedicated option or reserved headroom for DataCategory.Error.
  • The five process*Async methods are near-exact copies of their sync counterparts (~200 lines). Could be parameterized.
  • Migrating selected SDK-owned expensive processors to processAsync(...) where they do not capture stale request, thread-local, or UI state.

adinauer and others added 4 commits July 30, 2026 16:25
Document the proposed opt-in async processing pipeline for EventProcessor callbacks, including queue behavior, flush semantics, and testing coverage.

Co-Authored-By: Claude <noreply@anthropic.com>
Add EventProcessor.processAsync callbacks and an opt-in SentryOptions flag to run late event processing off the caller thread.

Use a dedicated bounded queue for async callbacks, record queue overflow drops, and make flush/close drain accepted async work before downstream queues.

Co-Authored-By: Claude <noreply@anthropic.com>
The async event processing stage moved work to another thread without
establishing what state that work may touch. Four issues followed.

close() flushed downstream queues before quiescing the async stage, so
tasks finishing afterwards handed envelopes to a transport that was
already closing, and anything still queued when the timeout expired was
discarded with no client report. Quiesce the async stage first and record
queue_overflow for work that could not be drained.

Captures whose hint coordinates across threads are no longer deferred.
DiskFlushNotification captures (uncaught exceptions, ANRv2, tombstones)
block the caller until the transport reports the event flushed to disk;
queueing put the fatal event behind everything already enqueued, so the
caller's flush timeout could expire before it was ever sent. Backfillable
and Cached captures carry retry/delete state the caller reads as soon as
capture returns, and TransactionEnd captures force-finish the running
transaction before the process dies.

The scope passed to capture* is a live view over the caller's
thread-local scope, so the async stage could read tags, sessions, and
transactions belonging to unrelated later work. It is now cloned at
submit time.

Hint left its attachment fields unsynchronized while both threads could
still reach them. The attachment list is now guarded by the existing lock
and the single-reference fields are volatile.

Also stop holding the executor monitor across awaitTermination, which
could stall a capture on an app thread for the whole shutdown timeout.
@github-actions

Copy link
Copy Markdown
Contributor
Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

### Fixes

- Make async event processing safe for crash and shutdown paths ([#5878](https://github.com/getsentry/sentry-java/pull/5878))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description or adding a skip-changelog label.

Generated by 🚫 dangerJS against 3a02b30

@sentry

sentry Bot commented Jul 30, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.51.0 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 321.13 ms 359.47 ms 38.34 ms
Size 0 B 0 B 0 B

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
6b019b7 343.31 ms 417.23 ms 73.91 ms
d15471f 342.08 ms 415.44 ms 73.35 ms
8687935 332.52 ms 362.23 ms 29.71 ms
5b1a06b 352.27 ms 413.70 ms 61.43 ms
91bb874 314.47 ms 440.00 ms 125.53 ms
0ee65e9 321.06 ms 361.24 ms 40.18 ms
e63ad34 323.67 ms 390.33 ms 66.67 ms
33a08cc 267.08 ms 340.45 ms 73.37 ms
27d7cf8 397.90 ms 498.65 ms 100.75 ms
ee747ae 405.43 ms 485.70 ms 80.28 ms

App size

Revision Plain With Sentry Diff
6b019b7 0 B 0 B 0 B
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
8687935 1.58 MiB 2.19 MiB 619.17 KiB
5b1a06b 0 B 0 B 0 B
91bb874 1.58 MiB 2.13 MiB 559.07 KiB
0ee65e9 0 B 0 B 0 B
e63ad34 0 B 0 B 0 B
33a08cc 1.58 MiB 2.12 MiB 555.28 KiB
27d7cf8 1.58 MiB 2.12 MiB 549.42 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants