fix(replay): Post checkCanRecord to main thread to prevent deadlock

[romtsn]

Summary

• onScreenshotRecorded can run on the replay executor thread (PixelCopy masked-capture and emit paths). checkCanRecord() → pauseInternal() acquires lifecycleLock — if another thread holds that lock and submits to the same single-threaded executor, we deadlock.
• Post checkCanRecord() to the main looper so it never runs on the executor thread.

This was a pre-existing issue introduced with SurfaceView capturing (masked-capture path calls onScreenshotRecorded from executor at PixelCopyStrategy.kt:209), and is also relevant for PR #5808 which adds the emitLastScreenshot executor path.

Test plan

• Existing onScreenshotRecorded pauses replay when rate-limited for sessions test updated to idle looper
• ./gradlew :sentry-android-replay:testReleaseUnitTest --tests="*ReplayIntegrationTest*" passes

🤖 Generated with Claude Code

[sentry]

📲 Install Builds

Android

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

⚙️ sentry-android Build Distribution Settings

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	318.04 ms	367.28 ms	49.24 ms
Size	0 B	0 B	0 B

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
05aa61d	326.06 ms	385.46 ms	59.40 ms
bb0ff41	315.84 ms	350.76 ms	34.92 ms
806307f	357.85 ms	424.64 ms	66.79 ms
d501a7e	307.33 ms	341.94 ms	34.61 ms
0ee65e9	321.06 ms	361.24 ms	40.18 ms
ed33deb	334.19 ms	362.30 ms	28.11 ms
9fbb112	401.87 ms	515.87 ms	114.00 ms
b8bd880	314.56 ms	336.50 ms	21.94 ms
5b1a06b	315.40 ms	353.33 ms	37.94 ms
6edfca2	316.43 ms	398.90 ms	82.46 ms

App size

Revision	Plain	With Sentry	Diff
05aa61d	0 B	0 B	0 B
bb0ff41	0 B	0 B	0 B
806307f	1.58 MiB	2.10 MiB	533.42 KiB
d501a7e	0 B	0 B	0 B
0ee65e9	0 B	0 B	0 B
ed33deb	1.58 MiB	2.13 MiB	559.52 KiB
9fbb112	1.58 MiB	2.11 MiB	539.18 KiB
b8bd880	1.58 MiB	2.29 MiB	722.92 KiB
5b1a06b	0 B	0 B	0 B
6edfca2	1.58 MiB	2.13 MiB	559.07 KiB

Previous results on branch: rz/fix/replay-checkcanrecord-deadlock

Startup times

Revision	Plain	With Sentry	Diff
1922e86	319.80 ms	359.07 ms	39.27 ms
03ae43b	327.07 ms	408.58 ms	81.52 ms
ba152d2	346.75 ms	440.34 ms	93.59 ms
33731c0	351.60 ms	422.48 ms	70.88 ms

App size

Revision	Plain	With Sentry	Diff
1922e86	0 B	0 B	0 B
03ae43b	0 B	0 B	0 B
ba152d2	0 B	0 B	0 B
33731c0	0 B	0 B	0 B

[cursor]

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.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit dabe4b2. Configure here.}

[0xadam-brown]

m: Looks like we have (at least) two choices for how we fix our deadlock:

1. never allow code that may acquire lifecycleLock to run on the replay executor thread (ie, this PR's current approach), or
2. don't allow code holding lifecycleLock to try to shut down the replay executor.

I like (2) because it's easier to confine to a single place (the close() method), both now and over time; it's reasonable to have a policy that prohibits trying to shut down executors (or do other potentially blocking things) while holding locks (because of deadlocks like this one); and tracking down exactly which code is actually executing onScreenshotRecorded() is...tricky.

That said, maybe (2) is a pain to implement? If so, I'm okay with (1), but my vote would be to rename postOnMainThread() -> dontBlockExecutorShutdown() (or something like that). The rename would put us on notice that we're being clever, and it'd also avoid the suggestion that use of the main thread is itself significant.

---

Background explaining why I think releasing lifecycleLock before shutting down the replay executor will prevent our deadlock:

AFAICT, our deadlock emerges when:

1. Thread A holds lifecycleLock.
2. Replay executor on thread B runs onScreenshotRecorded(), which calls checkCanRecord().
3. checkCanRecord() tries to acquire lifecycleLock via pauseInternal() but can't because thread A holds it.
4. Thread A finishes its work and – before releasing lifecycleLock – calls replayExecutor.shutdown().
5. But shutdown() only completes if all tasks managed by the replayExecutor have terminated.
6. The onScreenshotRecorded() task hasn't completed because its waiting for the lifecycleLock. --> and our deadlock is born.

[romtsn]

thanks, that's a good point, however I don't think replayExecutor.shutdown() alone is the problem. There's also this scenario (as we'd attempted to fix earlier: #4788):

1. Thread A holds lifecycleLock (in pause(), stop(), start(), whatever)
2. Executor thread runs onScreenshotRecorded → checkCanRecord() → pauseInternal() → tries to acquire lifecycleLock → blocks
3. Thread A (still holding the lock) submits something to the same single-threaded executor → blocks waiting for a slot → deadlock

So I think to satisfy both, we actually need both option 1 and 2. I can add the 2nd one 👍

[0xadam-brown]

l: If we keep approach (1), worth explicitly mentioning that the invariant we need to maintain is never to allow the replay executor to try to acquire lifecycleLock, as it could prevent the executor from being shut down.