ref(android): Confine replay lifecycle to main thread - #5965
Conversation
Serialize replay lifecycle mutations on Android's main thread and keep replay cache cleanup ordered on the replay executor. Remove locks that could block lifecycle callbacks while preserving shutdown ordering. Refs JAVA-665 Co-Authored-By: OpenAI Codex <noreply@openai.com>
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9e60aca | 316.18 ms | 345.04 ms | 28.86 ms |
| 6b019b7 | 403.90 ms | 546.09 ms | 142.19 ms |
| d15471f | 310.66 ms | 368.19 ms | 57.53 ms |
| d217708 | 375.27 ms | 415.68 ms | 40.41 ms |
| 22f4345 | 314.79 ms | 375.02 ms | 60.23 ms |
| fcec2f2 | 328.91 ms | 387.75 ms | 58.84 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| 7414e9b | 315.69 ms | 367.66 ms | 51.97 ms |
| fcec2f2 | 314.96 ms | 373.66 ms | 58.70 ms |
| e2dce0b | 308.96 ms | 360.10 ms | 51.14 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9e60aca | 0 B | 0 B | 0 B |
| 6b019b7 | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| 7414e9b | 0 B | 0 B | 0 B |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| e2dce0b | 0 B | 0 B | 0 B |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } catch (t: Throwable) { | ||
| release() | ||
| throw t | ||
| } |
There was a problem hiding this comment.
do you know why we catch and rethrow the throwable here instead of a finally ?
| } catch (t: Throwable) { | |
| release() | |
| throw t | |
| } | |
| } finally { | |
| release() | |
| } |
| recorder = null | ||
| rootViewsSpy.close() | ||
| lifecycle.currentState = CLOSED | ||
| val isMainThread = Looper.myLooper() == Looper.getMainLooper() |
There was a problem hiding this comment.
how come we don't use options.threadChecker.isMainThread here ?
| ) | ||
| return | ||
| } | ||
| postOnMainThread { startInternal() } |
There was a problem hiding this comment.
postOnMainThread is a bit of a misnomer since it doesn't post if we're already on the main thread.
Two issues:
- when called during startup, it doesn't post it for later meaning it doesn't improve the startup time.
- calls can get out of order. not sure if this was an explicit goal, if we always post to the main thread, then the events are queue in order, if we sometimes post and sometimes execute inline then we dno't have any ordering guarantee if these are called from different threads.
Keep replay lifecycle state in one atomic value and serialize lifecycle transitions through the main looper. Return the replay ID synchronously so triggering events remain correlated while capture is deferred. Refs JAVA-665 Refs JAVA-656 Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c9a7891. Configure here.
| scopes?.configureScope { it.replayId = current.replayId } | ||
| enqueueOnMainThread { | ||
| captureReplayInternal(current.generation, current.replayId, isTerminating == true) | ||
| } |
There was a problem hiding this comment.
Crash capture never runs on main
High Severity
captureReplay always posts captureReplayInternal instead of running it on the current main-thread turn. A main-thread uncaught exception captures the event and then blocks the looper in waitFlush(), so the queued work never runs. isTerminating is never set and buffer-to-session conversion never happens before the process dies.
Reviewed by Cursor Bugbot for commit c9a7891. Configure here.
| replayId = replayId ?: SentryId.EMPTY_ID, | ||
| captureStrategy = strategy, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Replay state published after start
Medium Severity
startInternal starts the recorder and capture strategy before publishing ReplayState. Screenshot, touch, and captureReplay readers still see the previous generation with a null strategy, so early frames and a concurrent capture can be dropped even though start has already begun.
Reviewed by Cursor Bugbot for commit c9a7891. Configure here.
2 participants
📜 Description
Confine Session Replay lifecycle mutations to Android's main thread and keep replay cache cleanup ordered on the replay executor. Background shutdown waits only for main-thread teardown to queue cleanup before stopping the executors.
This also removes the lifecycle and encoder locks that are no longer needed, together with the unused
tryAcquire()helper.💡 Motivation and Context
Prepare Session Replay for public start and stop APIs without allowing lifecycle calls from arbitrary threads to race or block Android lifecycle callbacks.
JAVA-656 remains related follow-up work because automatic replay startup is still synchronous when SDK initialization already runs on the main thread.
Refs JAVA-665
Refs JAVA-656
💚 How did you test it?
./gradlew spotlessApply apiDump.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps