Skip to content

fix(replay): Fix visual artifacts for the Canvas strategy on some devices #4861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 5, 2025
+97 −142
Merged

fix(replay): Fix visual artifacts for the Canvas strategy on some devices #4861

Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5dd2b60
fix(replay): Fix visual artifacts for the Canvas strategy on some dev…
romtsn Nov 4, 2025
ae3dda4
Wrap into try-catch
romtsn Nov 4, 2025
8ad5eb0
Update Changelog
markushi Nov 4, 2025
835d1aa
Remove extra background executor, as it's not needed anymore
markushi Nov 4, 2025
3ee6ab6
Revert "Remove extra background executor, as it's not needed anymore"
markushi Nov 5, 2025
2bd3de4
Use same background handler everywhere
markushi Nov 5, 2025
3eb8f20
Ensure handler.post is caught gracefully
markushi Nov 5, 2025
03a7871
Merge branch 'main' into rz/fix/session-replay-canvas-artifacts
romtsn Nov 5, 2025
c470a4b
Clean up unprocessedPictureRef
romtsn Nov 5, 2025
1c9adf7
Merge branch 'main' into rz/fix/session-replay-canvas-artifacts
markushi Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 65 additions & 142 deletions sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?,
Expand All @@ -51,73 +50,19 @@ internal class CanvasStrategy(
) : ScreenshotStrategy {

@Volatile private var screenshot: Bitmap? = null

// Lock to synchronize screenshot creation
private var unprocessedPictureRef = AtomicReference<Picture>(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<PictureReaderHolder>(null)
private val surface = Surface(surfaceTexture)

init {
IntegrationUtils.addIntegrationToSdkVersion("ReplayCanvasStrategy")
Expand All @@ -132,54 +77,72 @@ internal class CanvasStrategy(
)
return@Runnable
}
val holder = unprocessedPictureRef.getAndSet(null) ?: return@Runnable
val picture = unprocessedPictureRef.getAndSet(null) ?: return@Runnable

try {
if (!holder.setup.getAndSet(true)) {
holder.reader.setOnImageAvailableListener(holder, executor.getBackgroundHandler())
}

val surface = holder.reader.surface
val canvas = surface.lockHardwareCanvas()
// Draw picture to the Surface for PixelCopy
val surfaceCanvas = surface.lockHardwareCanvas()
try {
canvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR)
holder.picture.draw(canvas)
surfaceCanvas.drawColor(Color.BLACK, PorterDuff.Mode.CLEAR)
picture.draw(surfaceCanvas)
} finally {
surface.unlockCanvasAndPost(canvas)
surface.unlockCanvasAndPost(surfaceCanvas)
}
} 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)
}
Comment on lines 116 to 129
Copy link

@sentry sentry bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unsynchronized access to screenshot Bitmap by multiple threads while another thread recycles it.
Severity: CRITICAL | Confidence: 1.00

🔍 Detailed Analysis

The screenshot Bitmap is accessed without synchronization by emitLastScreenshot() on the main thread and by the PixelCopy callback on a background handler thread. Concurrently, the close() method, executing on a thread pool, recycles the same screenshot Bitmap. This creates a data race where the Bitmap can be accessed after it has been recycled, which can lead to native crashes according to repository guidelines.

💡 Suggested Fix

Protect all accesses to the screenshot Bitmap with synchronized(bitmap) blocks to prevent concurrent access during recycling or other operations.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt#L104-L110

Potential issue: The `screenshot` Bitmap is accessed without synchronization by
`emitLastScreenshot()` on the main thread and by the PixelCopy callback on a background
handler thread. Concurrently, the `close()` method, executing on a thread pool, recycles
the same `screenshot` Bitmap. This creates a data race where the Bitmap can be accessed
after it has been recycled, which can lead to native crashes according to repository
guidelines.

Did we get this right? 👍 / 👎 to inform future reviews.

} else {
options.logger.log(
SentryLevel.ERROR,
"Canvas Strategy: PixelCopy failed with code $result",
)
lastCaptureSuccessful.set(false)
}
},
executor.getBackgroundHandler(),
)
} catch (t: Throwable) {
options.logger.log(SentryLevel.ERROR, "Canvas Strategy: picture render failed")
lastCaptureSuccessful.set(false)
}
}

@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))
}
}

Expand All @@ -192,28 +155,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()) {
Expand Down Expand Up @@ -1031,30 +981,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
}
}
}
Loading