From 45c33ca36b8d2d15b568f585f874c8cce6c655db Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 21 Jul 2026 18:23:24 +0200 Subject: [PATCH 1/5] fix(replay): Prevent concurrent PixelCopy frame access Keep PixelCopy, masking, compositing, and cleanup from accessing the shared bitmap concurrently. Fixes GH-5340 Co-Authored-By: Codex --- .../replay/screenshot/PixelCopyStrategy.kt | 123 ++++++++++++------ .../screenshot/PixelCopyStrategyTest.kt | 93 +++++++++++++ 2 files changed, 177 insertions(+), 39 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt index 4b9618df6ec..281b099ee79 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt @@ -59,6 +59,8 @@ internal class PixelCopyStrategy( private val contentChanged = AtomicBoolean(false) private val unstableCaptures = AtomicInteger(0) private val isClosed = AtomicBoolean(false) + private val frameInFlight = AtomicBoolean(false) + private val cleanupScheduled = AtomicBoolean(false) private val dstOverPaint by lazy(NONE) { Paint().apply { xfermode = PorterDuffXfermode(PorterDuff.Mode.DST_OVER) } } private val screenshotCanvas by lazy(NONE) { Canvas(screenshot) } @@ -77,8 +79,14 @@ internal class PixelCopyStrategy( return } + if (!frameInFlight.compareAndSet(false, true)) { + options.logger.log(DEBUG, "PixelCopyStrategy capture is already in flight, skipping") + return + } + if (isClosed.get()) { options.logger.log(DEBUG, "PixelCopyStrategy is closed, not capturing screenshot") + finishFrame() return } @@ -90,6 +98,7 @@ internal class PixelCopyStrategy( { copyResult: Int -> if (isClosed.get()) { options.logger.log(DEBUG, "PixelCopyStrategy is closed, ignoring capture result") + finishFrame() return@request } @@ -97,11 +106,13 @@ internal class PixelCopyStrategy( options.logger.log(INFO, "Failed to capture replay recording: %d", copyResult) unstableCaptures.set(0) lastCaptureSuccessful.set(false) + finishFrame() return@request } val changedDuringCapture = contentChanged.get() if (changedDuringCapture && shouldSkipUnstableCapture()) { + finishFrame() return@request } @@ -116,15 +127,23 @@ internal class PixelCopyStrategy( root.traverse(viewHierarchy, options.sessionReplay, options.logger, surfaceViewNodes) if (surfaceViewNodes.isNullOrEmpty()) { - executor.submit( - ReplayRunnable("screenshot_recorder.mask") { - applyMaskingAndNotify( - root, - viewHierarchy, - resetUnstableCaptures = !changedDuringCapture, - ) - } - ) + val submitted = + executor.submit( + ReplayRunnable("screenshot_recorder.mask") { + try { + applyMaskingAndNotify( + root, + viewHierarchy, + resetUnstableCaptures = !changedDuringCapture, + ) + } finally { + finishFrame() + } + } + ) + if (submitted == null) { + finishFrame() + } } else { // Re-arm the recorder's contentChanged gate; SurfaceView redraws don't trigger // ViewTreeObserver.OnDrawListener, so we'd otherwise emit the same frame forever. @@ -143,6 +162,7 @@ internal class PixelCopyStrategy( options.logger.log(WARNING, "Failed to capture replay recording", e) unstableCaptures.set(0) lastCaptureSuccessful.set(false) + finishFrame() } } @@ -272,37 +292,46 @@ internal class PixelCopyStrategy( windowY: Int, resetUnstableCaptures: Boolean, ) { - executor.submit( - ReplayRunnable("screenshot_recorder.composite") { - if (isClosed.get() || screenshot.isRecycled) { - options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping compositing") - recycleCaptures(captures) - return@ReplayRunnable - } + val submitted = + executor.submit( + ReplayRunnable("screenshot_recorder.composite") { + try { + if (isClosed.get() || screenshot.isRecycled) { + options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping compositing") + recycleCaptures(captures) + return@ReplayRunnable + } - for (capture in captures) { - if (capture == null) continue - if (capture.bitmap.isRecycled) continue - - compositeSurfaceViewInto( - screenshotCanvas, - dstOverPaint, - tmpSrcRect, - tmpDstRect, - capture.bitmap, - capture.x, - capture.y, - windowX, - windowY, - config.scaleFactorX, - config.scaleFactorY, - ) - capture.bitmap.recycle() - } + for (capture in captures) { + if (capture == null) continue + if (capture.bitmap.isRecycled) continue + + compositeSurfaceViewInto( + screenshotCanvas, + dstOverPaint, + tmpSrcRect, + tmpDstRect, + capture.bitmap, + capture.x, + capture.y, + windowX, + windowY, + config.scaleFactorX, + config.scaleFactorY, + ) + capture.bitmap.recycle() + } - applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures) - } - ) + applyMaskingAndNotify(root, viewHierarchy, resetUnstableCaptures) + } finally { + finishFrame() + } + } + ) + if (submitted == null) { + recycleCaptures(captures) + finishFrame() + } } private fun recycleCaptures(captures: Array) { @@ -322,7 +351,7 @@ internal class PixelCopyStrategy( } override fun emitLastScreenshot() { - if (lastCaptureSuccessful() && !screenshot.isRecycled) { + if (!frameInFlight.get() && lastCaptureSuccessful() && !screenshot.isRecycled) { screenshotRecorderCallback?.onScreenshotRecorded(screenshot) } } @@ -330,6 +359,22 @@ internal class PixelCopyStrategy( override fun close() { isClosed.set(true) unstableCaptures.set(0) + if (!frameInFlight.get()) { + scheduleCleanup() + } + } + + private fun finishFrame() { + frameInFlight.set(false) + if (isClosed.get()) { + scheduleCleanup() + } + } + + private fun scheduleCleanup() { + if (!cleanupScheduled.compareAndSet(false, true)) { + return + } executor.submit( ReplayRunnable( "PixelCopyStrategy.close", diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt index 779cf7d4311..14e0706407f 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt @@ -27,6 +27,7 @@ import io.sentry.android.replay.ScreenshotRecorderCallback import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.util.DebugOverlayDrawable import io.sentry.android.replay.util.MainLooperHandler +import java.util.concurrent.Future import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference @@ -140,6 +141,98 @@ class PixelCopyStrategyTest { if (failure.get() != null) throw failure.get() } + @Test + @Config(shadows = [DeferredWindowPixelCopyShadow::class]) + fun `capture drops frame while PixelCopy is in flight`() { + val activity = buildActivity(SimpleActivity::class.java).setup() + shadowOf(Looper.getMainLooper()).idle() + val root = activity.get().findViewById(android.R.id.content) + val strategy = fixture.getSut(executor = fixture.inlineExecutor()) + + strategy.capture(root) + strategy.capture(root) + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + + verify(fixture.callback).onScreenshotRecorded(any()) + + strategy.capture(root) + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + + verify(fixture.callback, times(2)).onScreenshotRecorded(any()) + } + + @Test + @Config(shadows = [DeferredWindowPixelCopyShadow::class]) + fun `capture drops frame while masking is in flight`() { + val activity = buildActivity(SimpleActivity::class.java).setup() + shadowOf(Looper.getMainLooper()).idle() + val root = activity.get().findViewById(android.R.id.content) + val tasks = mutableListOf() + val executor = mock() + whenever(executor.submit(any())).doAnswer { + tasks += it.getArgument(0) + mock>() + } + val strategy = fixture.getSut(executor) + + strategy.capture(root) + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + strategy.capture(root) + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + + assertEquals(1, tasks.size) + tasks.removeAt(0).run() + + strategy.capture(root) + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + + assertEquals(1, tasks.size) + } + + @Test + @Config(shadows = [DeferredWindowPixelCopyShadow::class]) + fun `emitLastScreenshot skips while frame is in flight`() { + val activity = buildActivity(SimpleActivity::class.java).setup() + shadowOf(Looper.getMainLooper()).idle() + val root = activity.get().findViewById(android.R.id.content) + val strategy = fixture.getSut(executor = fixture.inlineExecutor()) + captureStableFrame(strategy, root) + + strategy.capture(root) + strategy.emitLastScreenshot() + + verify(fixture.callback).onScreenshotRecorded(any()) + + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + verify(fixture.callback, times(2)).onScreenshotRecorded(any()) + } + + @Test + @Config(shadows = [DeferredWindowPixelCopyShadow::class]) + fun `close defers cleanup until PixelCopy completes`() { + val activity = buildActivity(SimpleActivity::class.java).setup() + shadowOf(Looper.getMainLooper()).idle() + val root = activity.get().findViewById(android.R.id.content) + val executor = mock() + val strategy = fixture.getSut(executor) + + strategy.capture(root) + strategy.close() + + verify(executor, never()).submit(any()) + + DeferredWindowPixelCopyShadow.flush() + shadowOf(Looper.getMainLooper()).idle() + + verify(executor).submit(any()) + } + @Test @Config(shadows = [DeferredWindowPixelCopyShadow::class]) fun `capture skips the first unstable PixelCopy result`() { From d6e09451728d85a88447a5443bc46c87eea328fc Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 22 Jul 2026 12:50:13 +0200 Subject: [PATCH 2/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d37da44d4b..48ce2126e61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Prevent concurrent PixelCopy access during Session Replay masking and bitmap cleanup ([#5808](https://github.com/getsentry/sentry-java/pull/5808)) - Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762)) - `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789)) - Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, `beforeSendLog`, or `beforeEnvelope` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737)) From 98aeb0ce87071baa3dc049ed953bfceae9065244 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 22 Jul 2026 13:57:05 +0200 Subject: [PATCH 3/5] fix(android-replay): Preserve pending PixelCopy capture Re-arm the recorder's content-change gate when a capture is skipped because another frame is still in flight. This ensures the latest UI state is retried on the next capture interval. Refs GH-5340 Co-Authored-By: OpenAI Codex --- .../io/sentry/android/replay/screenshot/PixelCopyStrategy.kt | 1 + .../sentry/android/replay/screenshot/PixelCopyStrategyTest.kt | 3 +++ 2 files changed, 4 insertions(+) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt index 281b099ee79..00a64dbfadc 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt @@ -81,6 +81,7 @@ internal class PixelCopyStrategy( if (!frameInFlight.compareAndSet(false, true)) { options.logger.log(DEBUG, "PixelCopyStrategy capture is already in flight, skipping") + markContentChanged() return } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt index 14e0706407f..834998da707 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt @@ -151,6 +151,9 @@ class PixelCopyStrategyTest { strategy.capture(root) strategy.capture(root) + + assertTrue(fixture.contentChangedMarked.get()) + DeferredWindowPixelCopyShadow.flush() shadowOf(Looper.getMainLooper()).idle() From 6b3a39db9c55384b2d6046fc75f4002852f84029 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 22 Jul 2026 17:45:28 +0200 Subject: [PATCH 4/5] perf(android-replay): Use RGB_565 when alpha is unnecessary Reduce replay screenshot memory while retaining ARGB_8888 for SurfaceView compositing. Refs GH-5340 Co-Authored-By: Codex --- .../sentry/android/replay/screenshot/CanvasStrategy.kt | 2 +- .../android/replay/screenshot/PixelCopyStrategy.kt | 10 +++++++++- .../android/replay/screenshot/PixelCopyStrategyTest.kt | 9 +++++++-- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt index 406b23a9e4e..c3deefbf71b 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt @@ -97,7 +97,7 @@ internal class CanvasStrategy( Bitmap.createBitmap( config.recordingWidth, config.recordingHeight, - Bitmap.Config.ARGB_8888, + Bitmap.Config.RGB_565, ) } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt index 00a64dbfadc..bea18384d0d 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt @@ -51,7 +51,15 @@ internal class PixelCopyStrategy( private val executor = executorProvider.getExecutor() private val mainLooperHandler = executorProvider.getMainLooperHandler() private val screenshot = - Bitmap.createBitmap(config.recordingWidth, config.recordingHeight, Bitmap.Config.ARGB_8888) + Bitmap.createBitmap( + config.recordingWidth, + config.recordingHeight, + if (options.sessionReplay.isCaptureSurfaceViews) { + Bitmap.Config.ARGB_8888 + } else { + Bitmap.Config.RGB_565 + }, + ) private val prescaledMatrix by lazy(NONE) { Matrix().apply { preScale(config.scaleFactorX, config.scaleFactorY) } } private val lastCaptureSuccessful = AtomicBoolean(false) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt index 834998da707..78168ee8644 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/screenshot/PixelCopyStrategyTest.kt @@ -38,6 +38,7 @@ import kotlin.test.assertFalse import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -310,7 +311,9 @@ class PixelCopyStrategyTest { assertFalse(fixture.contentChangedMarked.get()) assertTrue(strategy.lastCaptureSuccessful()) - verify(fixture.callback).onScreenshotRecorded(any()) + val screenshot = argumentCaptor() + verify(fixture.callback).onScreenshotRecorded(screenshot.capture()) + assertEquals(Bitmap.Config.RGB_565, screenshot.firstValue.config) } @Test @@ -342,7 +345,9 @@ class PixelCopyStrategyTest { shadowOf(Looper.getMainLooper()).idle() assertTrue(strategy.lastCaptureSuccessful()) - verify(fixture.callback).onScreenshotRecorded(any()) + val screenshot = argumentCaptor() + verify(fixture.callback).onScreenshotRecorded(screenshot.capture()) + assertEquals(Bitmap.Config.ARGB_8888, screenshot.firstValue.config) } @Test From 05221777721dc3b35c86bb592c05245489edd4cc Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 22 Jul 2026 17:48:58 +0200 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48ce2126e61..e2d5b44581b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Prevent concurrent PixelCopy access during Session Replay masking and bitmap cleanup ([#5808](https://github.com/getsentry/sentry-java/pull/5808)) +- Reduce Session Replay screenshot memory usage ([#5821](https://github.com/getsentry/sentry-java/pull/5821)) - Backfill release, environment, distribution, tags, and app version/build—and use the matching replay-on-error sample rate—for `ApplicationExitInfo` ANR and native crash events captured before SDK initialization, without reusing options cached by a later app update ([#5762](https://github.com/getsentry/sentry-java/pull/5762)) - `SentryTagModifierNode.isImportantForBounds` now matches the default behavior and returns `true` ([#5789](https://github.com/getsentry/sentry-java/pull/5789)) - Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, `beforeSendLog`, or `beforeEnvelope` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737))