fix(replay): Dispatch session-deadline stop off the replay worker thread#5826
Open
runningcode wants to merge 4 commits into
Open
fix(replay): Dispatch session-deadline stop off the replay worker thread#5826runningcode wants to merge 4 commits into
runningcode wants to merge 4 commits into
Conversation
…ead (DART-323) When a session replay reaches its duration deadline, the stop was invoked inline from the replay worker thread inside the frame-processing task. Because ReplayExecutorService.submit() runs tasks synchronously when already on that thread, stop() encoded the final segment and deleted the replay cache while holding the replay lifecycle lock. A foreground start() on the main thread then parked on that lock long enough to trigger a background ANR. Dispatch the deadline stop through options.executorService so the segment encoding is enqueued asynchronously and the lifecycle lock is only held for the fast state transition, matching the existing timer-executor stop path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 307.87 ms | 354.51 ms | 46.64 ms |
| 8c7718c | 307.42 ms | 374.84 ms | 67.42 ms |
| bb0ff41 | 321.00 ms | 378.28 ms | 57.28 ms |
| c8125f3 | 397.65 ms | 485.14 ms | 87.49 ms |
| eb95ded | 317.51 ms | 369.08 ms | 51.57 ms |
| f634d01 | 375.06 ms | 420.04 ms | 44.98 ms |
| 9fbb112 | 359.71 ms | 421.85 ms | 62.14 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| bb0ff41 | 317.76 ms | 384.66 ms | 66.90 ms |
| 9054d65 | 330.94 ms | 403.24 ms | 72.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| 8c7718c | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| c8125f3 | 1.58 MiB | 2.10 MiB | 532.32 KiB |
| eb95ded | 0 B | 0 B | 0 B |
| f634d01 | 1.58 MiB | 2.10 MiB | 533.40 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| bb0ff41 | 0 B | 0 B | 0 B |
| 9054d65 | 1.58 MiB | 2.29 MiB | 723.38 KiB |
romtsn
marked this pull request as ready for review
July 24, 2026 14:49
romtsn
requested review from
0xadam-brown,
adinauer,
markushi and
romtsn
as code owners
July 24, 2026 14:49
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 608c130. Configure here.
| // start() on the main thread (ANR) | ||
| options.executorService.submitSafely(options, "$TAG.stop") { | ||
| options.replayController.stop() | ||
| } |
There was a problem hiding this comment.
Stale deadline stop races new session
Low Severity
The deadline stop() is queued on options.executorService with no replay-id or generation check, and unlike the LifecycleWatcher end-session path it cannot be cancelled. If the current session is stopped and a new one starts before that task runs, the deferred stop() tears down the new session.
Reviewed by Cursor Bugbot for commit 608c130. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
When a session replay reaches its duration deadline (default 1h), the resulting
stop()was invoked inline from the replay worker thread, inside theadd_frametask inSessionCaptureStrategy.onScreenshotRecorded. BecauseReplayExecutorService.submit()runs tasks synchronously when the caller is already on aSentryReplayIntegration-*thread,stop()encoded the final video segment and recursively deleted the replay cache while holding the replay lifecycle lock (AutoClosableReentrantLock).A foreground
start()(LifecycleWatcher.onForeground→startSession) runs on the main thread and must acquire that same lock, so it parked long enough to trigger a background ANR.This change dispatches the deadline
stop()throughoptions.executorService(a non-worker thread).stop()then submits the segment encoding asynchronously onto the single-threaded replay executor (preserving ordering), and the lifecycle lock is held only for the fast state transition — matching the existing timer-executorendSessionstop path.💡 Motivation and Context
Fixes the background ANR reported in getsentry/sentry-dart#3556 (Linear: DART-323), whose stack shows the main thread parked on the replay lifecycle lock in
ReplayIntegration.start()on foreground. The deadline stop re-entering the lifecycle lock from the worker thread is the only path where the segment encode runs synchronously under the lock; every otherstart/stop/pause/resumecaller already submits the encode asynchronously.💚 How did you test it?
Added a regression test in
SessionCaptureStrategyTestusing a deferred executor that asserts the deadlinestop()is dispatched off the calling thread (not run inline) and only runs once the executor drains. Updated the existing deadline test's fixture to use an immediate executor so it still exercises the dispatched stop.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
None.