From 5dd2b608cb31aff83dac27f4b182e5fceeafaf32 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 4 Nov 2025 15:13:54 +0100 Subject: [PATCH 1/8] fix(replay): Fix visual artifacts for the Canvas strategy on some devices --- .../replay/screenshot/CanvasStrategy.kt | 203 +++++------------- 1 file changed, 59 insertions(+), 144 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 a5c76778cff..e5652347292 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 @@ -15,16 +15,17 @@ import android.graphics.NinePatch import android.graphics.Paint import android.graphics.Path import android.graphics.Picture -import android.graphics.PixelFormat import android.graphics.PorterDuff import android.graphics.Rect import android.graphics.RectF import android.graphics.Region import android.graphics.RenderNode +import android.graphics.SurfaceTexture import android.graphics.fonts.Font import android.graphics.text.MeasuredText -import android.media.ImageReader import android.os.Build +import android.view.PixelCopy +import android.view.Surface import android.view.View import androidx.annotation.RequiresApi import io.sentry.SentryLevel @@ -35,14 +36,12 @@ import io.sentry.android.replay.ScreenshotRecorderConfig import io.sentry.android.replay.util.ReplayRunnable import io.sentry.util.AutoClosableReentrantLock import io.sentry.util.IntegrationUtils -import java.io.Closeable import java.util.WeakHashMap import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference import kotlin.LazyThreadSafetyMode.NONE -import kotlin.use -@SuppressLint("UseKtx") +@SuppressLint("NewApi", "UseKtx") internal class CanvasStrategy( private val executor: ExecutorProvider, private val screenshotRecorderCallback: ScreenshotRecorderCallback?, @@ -51,73 +50,19 @@ internal class CanvasStrategy( ) : ScreenshotStrategy { @Volatile private var screenshot: Bitmap? = null - - // Lock to synchronize screenshot creation + private var unprocessedPictureRef = AtomicReference(null) private val screenshotLock = AutoClosableReentrantLock() private val prescaledMatrix by lazy(NONE) { Matrix().apply { preScale(config.scaleFactorX, config.scaleFactorY) } } private val lastCaptureSuccessful = AtomicBoolean(false) private val textIgnoringCanvas = TextIgnoringDelegateCanvas() - private val isClosed = AtomicBoolean(false) - private val onImageAvailableListener: (holder: PictureReaderHolder) -> Unit = { holder -> - if (isClosed.get()) { - options.logger.log(SentryLevel.ERROR, "CanvasStrategy already closed, skipping image") - holder.close() - } else { - try { - val image = holder.reader.acquireLatestImage() - try { - if (image.planes.size > 0) { - val plane = image.planes[0] - - if (screenshot == null) { - screenshotLock.acquire().use { - if (screenshot == null) { - screenshot = - Bitmap.createBitmap(holder.width, holder.height, Bitmap.Config.ARGB_8888) - } - } - } - - val bitmap = screenshot - if (bitmap != null) { - val buffer = plane.buffer.rewind() - synchronized(bitmap) { - if (!bitmap.isRecycled) { - bitmap.copyPixelsFromBuffer(buffer) - lastCaptureSuccessful.set(true) - } - } - screenshotRecorderCallback?.onScreenshotRecorded(bitmap) - } - } - } finally { - try { - image.close() - } catch (_: Throwable) { - // ignored - } - } - } catch (e: Throwable) { - options.logger.log(SentryLevel.ERROR, "CanvasStrategy: image processing failed", e) - } finally { - if (isClosed.get()) { - holder.close() - } else { - freePictureRef.set(holder) - } - } + private val surfaceTexture = + SurfaceTexture(false).apply { + setDefaultBufferSize(config.recordingWidth, config.recordingHeight) } - } - - private var freePictureRef = - AtomicReference( - PictureReaderHolder(config.recordingWidth, config.recordingHeight, onImageAvailableListener) - ) - - private var unprocessedPictureRef = AtomicReference(null) + private val surface = Surface(surfaceTexture) init { IntegrationUtils.addIntegrationToSdkVersion("ReplayCanvasStrategy") @@ -132,54 +77,64 @@ internal class CanvasStrategy( ) return@Runnable } - val holder = unprocessedPictureRef.getAndSet(null) ?: return@Runnable + val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable + // Draw picture to the Surface for PixelCopy + val surfaceCanvas = surface.lockHardwareCanvas() try { - if (!holder.setup.getAndSet(true)) { - holder.reader.setOnImageAvailableListener(holder, executor.getBackgroundHandler()) - } + surfaceCanvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) + picture.draw(surfaceCanvas) + } finally { + surface.unlockCanvasAndPost(surfaceCanvas) + } - val surface = holder.reader.surface - val canvas = surface.lockHardwareCanvas() - try { - canvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) - holder.picture.draw(canvas) - } finally { - surface.unlockCanvasAndPost(canvas) - } - } catch (t: Throwable) { - if (isClosed.get()) { - holder.close() - } else { - freePictureRef.set(holder) + if (screenshot == null) { + screenshotLock.acquire().use { + if (screenshot == null) { + screenshot = Bitmap.createBitmap(picture.width, picture.height, Bitmap.Config.ARGB_8888) + } } - options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed", t) } + + // Trigger PixelCopy capture + PixelCopy.request( + surface, + screenshot!!, + { result -> + if (result == PixelCopy.SUCCESS) { + lastCaptureSuccessful.set(true) + val bitmap = screenshot + if (bitmap != null && !bitmap.isRecycled) { + screenshotRecorderCallback?.onScreenshotRecorded(bitmap) + } + } else { + options.logger.log(SentryLevel.ERROR, "PixelCopy failed with code $result") + lastCaptureSuccessful.set(false) + } + }, + executor.getBackgroundHandler(), + ) } - @SuppressLint("UnclosedTrace") + @SuppressLint("NewApi") override fun capture(root: View) { if (isClosed.get()) { return } - val holder = freePictureRef.getAndSet(null) - if (holder == null) { - options.logger.log(SentryLevel.DEBUG, "No free Picture available, skipping capture") - lastCaptureSuccessful.set(false) - return - } - val pictureCanvas = holder.picture.beginRecording(config.recordingWidth, config.recordingHeight) - textIgnoringCanvas.delegate = pictureCanvas + val picture = Picture() + val canvas = picture.beginRecording(config.recordingWidth, config.recordingHeight) + textIgnoringCanvas.delegate = canvas textIgnoringCanvas.setMatrix(prescaledMatrix) root.draw(textIgnoringCanvas) - holder.picture.endRecording() - - if (isClosed.get()) { - holder.close() - } else { - unprocessedPictureRef.set(holder) - executor.getExecutor().submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) + picture.endRecording() + + if (!isClosed.get()) { + unprocessedPictureRef.set(picture) + // use the same handler for PixelCopy and pictureRenderTask + executor + .getBackgroundHandler() + .post(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) } } @@ -192,28 +147,15 @@ internal class CanvasStrategy( executor .getExecutor() .submit( - ReplayRunnable( - "CanvasStrategy.close", - { - screenshot?.let { - synchronized(it) { - if (!it.isRecycled) { - it.recycle() - } - } - } - }, - ) + ReplayRunnable("CanvasStrategy.close") { + screenshot?.let { synchronized(it) { if (!it.isRecycled) it.recycle() } } + surface.release() + surfaceTexture.release() + } ) - - // the image can be free, unprocessed or in transit - freePictureRef.getAndSet(null)?.reader?.close() - unprocessedPictureRef.getAndSet(null)?.reader?.close() } - override fun lastCaptureSuccessful(): Boolean { - return lastCaptureSuccessful.get() - } + override fun lastCaptureSuccessful(): Boolean = lastCaptureSuccessful.get() override fun emitLastScreenshot() { if (lastCaptureSuccessful()) { @@ -1031,30 +973,3 @@ private class TextIgnoringDelegateCanvas : Canvas() { } } } - -private class PictureReaderHolder( - val width: Int, - val height: Int, - val listener: (holder: PictureReaderHolder) -> Unit, -) : ImageReader.OnImageAvailableListener, Closeable { - val picture = Picture() - - @SuppressLint("InlinedApi") - val reader: ImageReader = ImageReader.newInstance(width, height, PixelFormat.RGBA_8888, 1) - - var setup = AtomicBoolean(false) - - override fun onImageAvailable(reader: ImageReader?) { - if (reader != null) { - listener(this) - } - } - - override fun close() { - try { - reader.close() - } catch (_: Throwable) { - // ignored - } - } -} From ae3dda4d1eec57adfed384fcf820500329dc1bd8 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 4 Nov 2025 15:19:26 +0100 Subject: [PATCH 2/8] Wrap into try-catch --- .../replay/screenshot/CanvasStrategy.kt | 66 +++++++++++-------- 1 file changed, 37 insertions(+), 29 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 e5652347292..bd62f196945 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 @@ -79,41 +79,49 @@ internal class CanvasStrategy( } val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable - // Draw picture to the Surface for PixelCopy - val surfaceCanvas = surface.lockHardwareCanvas() try { - surfaceCanvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) - picture.draw(surfaceCanvas) - } finally { - surface.unlockCanvasAndPost(surfaceCanvas) - } + // Draw picture to the Surface for PixelCopy + val surfaceCanvas = surface.lockHardwareCanvas() + try { + surfaceCanvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) + picture.draw(surfaceCanvas) + } finally { + surface.unlockCanvasAndPost(surfaceCanvas) + } - if (screenshot == null) { - screenshotLock.acquire().use { - if (screenshot == null) { - screenshot = Bitmap.createBitmap(picture.width, picture.height, Bitmap.Config.ARGB_8888) + if (screenshot == null) { + screenshotLock.acquire().use { + if (screenshot == null) { + screenshot = Bitmap.createBitmap(picture.width, picture.height, Bitmap.Config.ARGB_8888) + } } } - } - // Trigger PixelCopy capture - PixelCopy.request( - surface, - screenshot!!, - { result -> - if (result == PixelCopy.SUCCESS) { - lastCaptureSuccessful.set(true) - val bitmap = screenshot - if (bitmap != null && !bitmap.isRecycled) { - screenshotRecorderCallback?.onScreenshotRecorded(bitmap) + // Trigger PixelCopy capture + PixelCopy.request( + surface, + screenshot!!, + { result -> + if (result == PixelCopy.SUCCESS) { + lastCaptureSuccessful.set(true) + val bitmap = screenshot + if (bitmap != null && !bitmap.isRecycled) { + screenshotRecorderCallback?.onScreenshotRecorded(bitmap) + } + } else { + options.logger.log( + SentryLevel.ERROR, + "Canvas Strategy: PixelCopy failed with code $result", + ) + lastCaptureSuccessful.set(false) } - } else { - options.logger.log(SentryLevel.ERROR, "PixelCopy failed with code $result") - lastCaptureSuccessful.set(false) - } - }, - executor.getBackgroundHandler(), - ) + }, + executor.getBackgroundHandler(), + ) + } catch (t: Throwable) { + options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed") + lastCaptureSuccessful.set(false) + } } @SuppressLint("NewApi") From 8ad5eb0275aa3a8c0cb13338e04deb1195c57fc3 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Nov 2025 15:49:24 +0100 Subject: [PATCH 3/8] Update Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5206f8e4f77..bb38482fadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Fix visual artifacts for the Canvas strategy on some devices ([#4861](https://github.com/getsentry/sentry-java/pull/4861)) + ### Improvements - Fallback to distinct-id as user.id logging attribute when user is not set ([#4847](https://github.com/getsentry/sentry-java/pull/4847)) From 835d1aa60321ae9ae1306e300876278bcc11ac76 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Tue, 4 Nov 2025 16:26:11 +0100 Subject: [PATCH 4/8] Remove extra background executor, as it's not needed anymore --- .../sentry/android/replay/WindowRecorder.kt | 18 ------ .../replay/screenshot/CanvasStrategy.kt | 60 +++++++++++-------- .../android/replay/ScreenshotRecorderTest.kt | 3 - .../screenshot/PixelCopyStrategyTest.kt | 3 - 4 files changed, 36 insertions(+), 48 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt index 654fb43ab18..be6a73e8e08 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt @@ -244,21 +244,6 @@ internal class WindowRecorder( override fun getExecutor(): ScheduledExecutorService = replayExecutor override fun getMainLooperHandler(): MainLooperHandler = mainLooperHandler - - override fun getBackgroundHandler(): Handler { - // only start the background thread if it's actually needed, as it's only used by Canvas Capture - // Strategy - if (backgroundProcessingHandler == null) { - backgroundProcessingHandlerLock.acquire().use { - if (backgroundProcessingHandler == null) { - backgroundProcessingHandlerThread = HandlerThread("SentryReplayBackgroundProcessing") - backgroundProcessingHandlerThread?.start() - backgroundProcessingHandler = Handler(backgroundProcessingHandlerThread!!.looper) - } - } - } - return backgroundProcessingHandler!! - } } internal interface ExecutorProvider { @@ -267,7 +252,4 @@ internal interface ExecutorProvider { /** Returns a handler associated with the main thread looper. */ fun getMainLooperHandler(): MainLooperHandler - - /** Returns a handler associated with a background thread looper. */ - fun getBackgroundHandler(): Handler } 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 bd62f196945..d4656a13c05 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 @@ -29,6 +29,7 @@ import android.view.Surface import android.view.View import androidx.annotation.RequiresApi import io.sentry.SentryLevel +import io.sentry.SentryLevel.DEBUG import io.sentry.SentryOptions import io.sentry.android.replay.ExecutorProvider import io.sentry.android.replay.ScreenshotRecorderCallback @@ -71,15 +72,14 @@ internal class CanvasStrategy( @SuppressLint("NewApi") private val pictureRenderTask = Runnable { if (isClosed.get()) { - options.logger.log( - SentryLevel.DEBUG, - "Canvas Strategy already closed, skipping picture render", - ) + options.logger.log(DEBUG, "Canvas Strategy already closed, skipping picture render") return@Runnable } val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable try { + // It's safe to access the surface because the render task, + // as well as surface release are executed on the same single threaded executor // Draw picture to the Surface for PixelCopy val surfaceCanvas = surface.lockHardwareCanvas() try { @@ -96,30 +96,45 @@ internal class CanvasStrategy( } } } - - // Trigger PixelCopy capture PixelCopy.request( surface, screenshot!!, { result -> - if (result == PixelCopy.SUCCESS) { - lastCaptureSuccessful.set(true) - val bitmap = screenshot - if (bitmap != null && !bitmap.isRecycled) { - screenshotRecorderCallback?.onScreenshotRecorded(bitmap) - } - } else { - options.logger.log( - SentryLevel.ERROR, - "Canvas Strategy: PixelCopy failed with code $result", - ) - lastCaptureSuccessful.set(false) + if (isClosed.get()) { + options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result") + return@request } + executor + .getExecutor() + .submit( + ReplayRunnable("screenshot_recorder.mask") { + if (isClosed.get()) { + options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result") + return@ReplayRunnable + } + if (result == PixelCopy.SUCCESS) { + lastCaptureSuccessful.set(true) + val bitmap = screenshot + if (bitmap != null) { + synchronized(bitmap) { + if (!bitmap.isRecycled) + screenshotRecorderCallback?.onScreenshotRecorded(bitmap) + } + } + } else { + options.logger.log( + SentryLevel.ERROR, + "Canvas Strategy: PixelCopy failed with code $result", + ) + lastCaptureSuccessful.set(false) + } + } + ) }, - executor.getBackgroundHandler(), + executor.getMainLooperHandler().handler, ) } catch (t: Throwable) { - options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed") + options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed", t) lastCaptureSuccessful.set(false) } } @@ -139,10 +154,7 @@ internal class CanvasStrategy( if (!isClosed.get()) { unprocessedPictureRef.set(picture) - // use the same handler for PixelCopy and pictureRenderTask - executor - .getBackgroundHandler() - .post(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) + executor.getExecutor().submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) } } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt index 0a5c73f8a5c..5a7338e82d2 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt @@ -1,6 +1,5 @@ package io.sentry.android.replay -import android.os.Handler import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ScreenshotStrategyType import io.sentry.SentryOptions @@ -30,8 +29,6 @@ class ScreenshotRecorderTest { override fun getExecutor(): ScheduledExecutorService = mock() override fun getMainLooperHandler(): MainLooperHandler = mock() - - override fun getBackgroundHandler(): Handler = mock() }, null, ) 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 29a3089e686..9be71bd363b 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 @@ -2,7 +2,6 @@ package io.sentry.android.replay.screenshot import android.app.Activity import android.os.Bundle -import android.os.Handler import android.os.Looper import android.widget.LinearLayout import android.widget.LinearLayout.LayoutParams @@ -45,8 +44,6 @@ class PixelCopyStrategyTest { override fun getExecutor(): ScheduledExecutorService = executor override fun getMainLooperHandler(): MainLooperHandler = MainLooperHandler() - - override fun getBackgroundHandler(): Handler = mock() }, callback, options, From 3ee6ab67c296c0141565e6240541c95636ecedd2 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 5 Nov 2025 07:40:39 +0100 Subject: [PATCH 5/8] Revert "Remove extra background executor, as it's not needed anymore" This reverts commit 835d1aa60321ae9ae1306e300876278bcc11ac76. --- .../sentry/android/replay/WindowRecorder.kt | 18 ++++++ .../replay/screenshot/CanvasStrategy.kt | 60 ++++++++----------- .../android/replay/ScreenshotRecorderTest.kt | 3 + .../screenshot/PixelCopyStrategyTest.kt | 3 + 4 files changed, 48 insertions(+), 36 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt index be6a73e8e08..654fb43ab18 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt @@ -244,6 +244,21 @@ internal class WindowRecorder( override fun getExecutor(): ScheduledExecutorService = replayExecutor override fun getMainLooperHandler(): MainLooperHandler = mainLooperHandler + + override fun getBackgroundHandler(): Handler { + // only start the background thread if it's actually needed, as it's only used by Canvas Capture + // Strategy + if (backgroundProcessingHandler == null) { + backgroundProcessingHandlerLock.acquire().use { + if (backgroundProcessingHandler == null) { + backgroundProcessingHandlerThread = HandlerThread("SentryReplayBackgroundProcessing") + backgroundProcessingHandlerThread?.start() + backgroundProcessingHandler = Handler(backgroundProcessingHandlerThread!!.looper) + } + } + } + return backgroundProcessingHandler!! + } } internal interface ExecutorProvider { @@ -252,4 +267,7 @@ internal interface ExecutorProvider { /** Returns a handler associated with the main thread looper. */ fun getMainLooperHandler(): MainLooperHandler + + /** Returns a handler associated with a background thread looper. */ + fun getBackgroundHandler(): Handler } 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 d4656a13c05..bd62f196945 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 @@ -29,7 +29,6 @@ import android.view.Surface import android.view.View import androidx.annotation.RequiresApi import io.sentry.SentryLevel -import io.sentry.SentryLevel.DEBUG import io.sentry.SentryOptions import io.sentry.android.replay.ExecutorProvider import io.sentry.android.replay.ScreenshotRecorderCallback @@ -72,14 +71,15 @@ internal class CanvasStrategy( @SuppressLint("NewApi") private val pictureRenderTask = Runnable { if (isClosed.get()) { - options.logger.log(DEBUG, "Canvas Strategy already closed, skipping picture render") + options.logger.log( + SentryLevel.DEBUG, + "Canvas Strategy already closed, skipping picture render", + ) return@Runnable } val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable try { - // It's safe to access the surface because the render task, - // as well as surface release are executed on the same single threaded executor // Draw picture to the Surface for PixelCopy val surfaceCanvas = surface.lockHardwareCanvas() try { @@ -96,45 +96,30 @@ internal class CanvasStrategy( } } } + + // Trigger PixelCopy capture PixelCopy.request( surface, screenshot!!, { result -> - if (isClosed.get()) { - options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result") - return@request - } - executor - .getExecutor() - .submit( - ReplayRunnable("screenshot_recorder.mask") { - if (isClosed.get()) { - options.logger.log(DEBUG, "CanvasStrategy is closed, ignoring capture result") - return@ReplayRunnable - } - if (result == PixelCopy.SUCCESS) { - lastCaptureSuccessful.set(true) - val bitmap = screenshot - if (bitmap != null) { - synchronized(bitmap) { - if (!bitmap.isRecycled) - screenshotRecorderCallback?.onScreenshotRecorded(bitmap) - } - } - } else { - options.logger.log( - SentryLevel.ERROR, - "Canvas Strategy: PixelCopy failed with code $result", - ) - lastCaptureSuccessful.set(false) - } - } + if (result == PixelCopy.SUCCESS) { + lastCaptureSuccessful.set(true) + val bitmap = screenshot + if (bitmap != null && !bitmap.isRecycled) { + screenshotRecorderCallback?.onScreenshotRecorded(bitmap) + } + } else { + options.logger.log( + SentryLevel.ERROR, + "Canvas Strategy: PixelCopy failed with code $result", ) + lastCaptureSuccessful.set(false) + } }, - executor.getMainLooperHandler().handler, + executor.getBackgroundHandler(), ) } catch (t: Throwable) { - options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed", t) + options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed") lastCaptureSuccessful.set(false) } } @@ -154,7 +139,10 @@ internal class CanvasStrategy( if (!isClosed.get()) { unprocessedPictureRef.set(picture) - executor.getExecutor().submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) + // use the same handler for PixelCopy and pictureRenderTask + executor + .getBackgroundHandler() + .post(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) } } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt index 5a7338e82d2..0a5c73f8a5c 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ScreenshotRecorderTest.kt @@ -1,5 +1,6 @@ package io.sentry.android.replay +import android.os.Handler import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ScreenshotStrategyType import io.sentry.SentryOptions @@ -29,6 +30,8 @@ class ScreenshotRecorderTest { override fun getExecutor(): ScheduledExecutorService = mock() override fun getMainLooperHandler(): MainLooperHandler = mock() + + override fun getBackgroundHandler(): Handler = mock() }, null, ) 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 9be71bd363b..29a3089e686 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 @@ -2,6 +2,7 @@ package io.sentry.android.replay.screenshot import android.app.Activity import android.os.Bundle +import android.os.Handler import android.os.Looper import android.widget.LinearLayout import android.widget.LinearLayout.LayoutParams @@ -44,6 +45,8 @@ class PixelCopyStrategyTest { override fun getExecutor(): ScheduledExecutorService = executor override fun getMainLooperHandler(): MainLooperHandler = MainLooperHandler() + + override fun getBackgroundHandler(): Handler = mock() }, callback, options, From 2bd3de4894bc041e216505d9c28b2dcb90d886db Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 5 Nov 2025 07:52:15 +0100 Subject: [PATCH 6/8] Use same background handler everywhere --- .../replay/screenshot/CanvasStrategy.kt | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 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 bd62f196945..0d1adedfee6 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 @@ -78,9 +78,9 @@ internal class CanvasStrategy( return@Runnable } val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable - try { - // Draw picture to the Surface for PixelCopy + // It's safe to access the surface because the + // surface release within close() is executed on the same background handler val surfaceCanvas = surface.lockHardwareCanvas() try { surfaceCanvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR) @@ -92,16 +92,27 @@ internal class CanvasStrategy( if (screenshot == null) { screenshotLock.acquire().use { if (screenshot == null) { - screenshot = Bitmap.createBitmap(picture.width, picture.height, Bitmap.Config.ARGB_8888) + screenshot = + Bitmap.createBitmap( + config.recordingWidth, + config.recordingHeight, + Bitmap.Config.ARGB_8888, + ) } } } - // Trigger PixelCopy capture PixelCopy.request( surface, screenshot!!, { result -> + if (isClosed.get()) { + options.logger.log( + SentryLevel.DEBUG, + "CanvasStrategy is closed, ignoring capture result", + ) + return@request + } if (result == PixelCopy.SUCCESS) { lastCaptureSuccessful.set(true) val bitmap = screenshot @@ -119,7 +130,7 @@ internal class CanvasStrategy( executor.getBackgroundHandler(), ) } catch (t: Throwable) { - options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed") + options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed", t) lastCaptureSuccessful.set(false) } } @@ -153,8 +164,8 @@ internal class CanvasStrategy( override fun close() { isClosed.set(true) executor - .getExecutor() - .submit( + .getBackgroundHandler() + .post( ReplayRunnable("CanvasStrategy.close") { screenshot?.let { synchronized(it) { if (!it.isRecycled) it.recycle() } } surface.release() From 3eb8f207f87438559631f621d3abb81ae5f43baf Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Wed, 5 Nov 2025 08:00:19 +0100 Subject: [PATCH 7/8] Ensure handler.post is caught gracefully --- .../replay/screenshot/CanvasStrategy.kt | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 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 0d1adedfee6..c03b8f0f656 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 @@ -24,6 +24,7 @@ import android.graphics.SurfaceTexture import android.graphics.fonts.Font import android.graphics.text.MeasuredText import android.os.Build +import android.os.Handler import android.view.PixelCopy import android.view.Surface import android.view.View @@ -102,6 +103,13 @@ internal class CanvasStrategy( } } + if (isClosed.get()) { + options.logger.log( + SentryLevel.DEBUG, + "Canvas Strategy already closed, skipping pixel copy request", + ) + return@Runnable + } PixelCopy.request( surface, screenshot!!, @@ -150,10 +158,9 @@ internal class CanvasStrategy( if (!isClosed.get()) { unprocessedPictureRef.set(picture) - // use the same handler for PixelCopy and pictureRenderTask executor .getBackgroundHandler() - .post(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) + .postSafely(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask)) } } @@ -165,7 +172,7 @@ internal class CanvasStrategy( isClosed.set(true) executor .getBackgroundHandler() - .post( + .postSafely( ReplayRunnable("CanvasStrategy.close") { screenshot?.let { synchronized(it) { if (!it.isRecycled) it.recycle() } } surface.release() @@ -184,6 +191,18 @@ internal class CanvasStrategy( } } } + + fun Handler.postSafely(runnable: ReplayRunnable) { + try { + post(runnable) + } catch (t: Throwable) { + options.logger.log( + SentryLevel.ERROR, + "Canvas Strategy: failed to post runnable ${runnable.taskName}", + t, + ) + } + } } @SuppressLint("UseKtx") From c470a4be1f00b05b190cc58fde50856c543a0c23 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 5 Nov 2025 09:41:59 +0100 Subject: [PATCH 8/8] Clean up unprocessedPictureRef --- .../java/io/sentry/android/replay/screenshot/CanvasStrategy.kt | 1 + 1 file changed, 1 insertion(+) 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 c03b8f0f656..406b23a9e4e 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 @@ -179,6 +179,7 @@ internal class CanvasStrategy( surfaceTexture.release() } ) + unprocessedPictureRef.getAndSet(null) } override fun lastCaptureSuccessful(): Boolean = lastCaptureSuccessful.get()