diff --git a/CHANGELOG.md b/CHANGELOG.md index cfd2e8d0f78..20603aa01d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- Fix background ANR when a session replay reaches its duration deadline: the resulting `stop()` (and its segment encoding) no longer runs on the replay worker thread while holding the replay lifecycle lock, which could block a foreground `start()` on the main thread ([#5826](https://github.com/getsentry/sentry-java/pull/5826)) - Release `MediaMuxer` when the replay video encoder fails to start to avoid a resource leak ([#5607](https://github.com/getsentry/sentry-java/pull/5607)) ### Performance diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt index d62efb534cc..37dd5b47d1e 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt @@ -10,6 +10,7 @@ import io.sentry.android.replay.ReplayCache import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment import io.sentry.android.replay.util.ReplayRunnable +import io.sentry.android.replay.util.submitSafely import io.sentry.protocol.SentryId import io.sentry.transport.ICurrentDateProvider import io.sentry.util.FileUtils @@ -134,8 +135,13 @@ internal class SessionCaptureStrategy( } if ((now - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration)) { - options.replayController.stop() options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording") + // dispatch off the replay worker thread, otherwise stop() would run the final segment + // encoding synchronously while holding the replay lifecycle lock, blocking a foreground + // start() on the main thread (ANR) + options.executorService.submitSafely(options, "$TAG.stop") { + options.replayController.stop() + } } } ) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt index fc2354eb1c0..6e7b00eaf5e 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/capture/SessionCaptureStrategyTest.kt @@ -29,6 +29,8 @@ import io.sentry.protocol.SentryId import io.sentry.rrweb.RRWebBreadcrumbEvent import io.sentry.rrweb.RRWebMetaEvent import io.sentry.rrweb.RRWebOptionsEvent +import io.sentry.test.DeferredExecutorService +import io.sentry.test.ImmediateExecutorService import io.sentry.transport.CurrentDateProvider import io.sentry.transport.ICurrentDateProvider import java.io.File @@ -66,6 +68,7 @@ class SessionCaptureStrategyTest { setReplayController( mock { on { breadcrumbConverter }.thenReturn(DefaultReplayBreadcrumbConverter()) } ) + executorService = ImmediateExecutorService() } val scope = Scope(options) val scopes = @@ -291,6 +294,35 @@ class SessionCaptureStrategyTest { verify(fixture.options.replayController).stop() } + @Test + fun `onScreenshotRecorded dispatches deadline stop off the calling thread`() { + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + var count = 0 + val strategy = + fixture.getSut( + dateProvider = { + if (count++ == 2) { + System.currentTimeMillis() + (fixture.options.sessionReplay.sessionDuration * 2) + } else { + System.currentTimeMillis() + } + } + ) + strategy.start() + strategy.onConfigurationChanged(mock()) + + strategy.onScreenshotRecorded(mock()) {} + + // stop must not run inline on the caller (replay worker) thread, otherwise it would encode the + // final segment while holding the lifecycle lock and block a foreground start() (ANR) + verify(fixture.options.replayController, never()).stop() + + deferredExecutor.runAll() + + verify(fixture.options.replayController).stop() + } + @Test fun `onConfigurationChanged creates new segment and updates config`() { val strategy = fixture.getSut()