[SR] Ensure bitmaps are recycled properly (#4820)

* Synchronize bitmap modifications

Requires to use synchronized blocks over AutoClosableReentrantLock, as
only the bitmap object is shared across Strategy <-> ReplayCache.
This ensure bitmap.compress is not called in parallel to bitmap.recycle

* Address PR comments

* Move recycling to background

* Address PR feedback

* Update Changelog

* Address PR comments

Author: markushi

CHANGELOG.md

@@ -41,6 +41,7 @@
   - This fixes "java.lang.IllegalArgumentException: The DSN is required" when combining WebFlux and OpenTelemetry
 - Session Replay: Do not use recycled screenshots for masking ([#4790](https://github.com/getsentry/sentry-java/pull/4790))
   - This fixes native crashes seen in `Canvas.<init>`/`ScreenshotRecorder.capture`
+- Session Replay: Ensure bitmaps are recycled properly ([#4820](https://github.com/getsentry/sentry-java/pull/4820))
 
 ### Miscellaneous
 

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt

@@ -79,12 +79,16 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
     replayCacheDir?.mkdirs()
 
     val screenshot = File(replayCacheDir, "$frameTimestamp.jpg").also { it.createNewFile() }
-    screenshot.outputStream().use {
-      bitmap.compress(JPEG, options.sessionReplay.quality.screenshotQuality, it)
-      it.flush()
+    synchronized(bitmap) {
+      if (bitmap.isRecycled) {
+        return
+      }
+      screenshot.outputStream().use {
+        bitmap.compress(JPEG, options.sessionReplay.quality.screenshotQuality, it)
+        it.flush()
+      }
+      addFrame(screenshot, frameTimestamp, screen)
     }
-
-    addFrame(screenshot, frameTimestamp, screen)
   }
 
   /**

sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt

@@ -142,10 +142,10 @@ internal class ScreenshotRecorder(
   }
 
   fun close() {
+    isCapturing.set(false)
     unbind(rootView?.get())
     rootView?.clear()
     screenshotStrategy.close()
-    isCapturing.set(false)
   }
 }
 

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/CanvasStrategy.kt

@@ -52,6 +52,7 @@ internal class CanvasStrategy(
 
   @Volatile private var screenshot: Bitmap? = null
 
+  // Lock to synchronize screenshot creation
   private val screenshotLock = AutoClosableReentrantLock()
   private val prescaledMatrix by
     lazy(NONE) { Matrix().apply { preScale(config.scaleFactorX, config.scaleFactorY) } }
@@ -71,19 +72,24 @@ internal class CanvasStrategy(
           if (image.planes.size > 0) {
             val plane = image.planes[0]
 
-            screenshotLock.acquire().use {
-              if (screenshot == null) {
-                screenshot =
-                  Bitmap.createBitmap(holder.width, holder.height, Bitmap.Config.ARGB_8888)
-              }
-              val bitmap = screenshot
-              if (bitmap == null || bitmap.isRecycled) {
-                return@use
+            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()
-              bitmap.copyPixelsFromBuffer(buffer)
-              lastCaptureSuccessful.set(true)
+              synchronized(bitmap) {
+                if (!bitmap.isRecycled) {
+                  bitmap.copyPixelsFromBuffer(buffer)
+                  lastCaptureSuccessful.set(true)
+                }
+              }
               screenshotRecorderCallback?.onScreenshotRecorded(bitmap)
             }
           }
@@ -183,14 +189,23 @@ internal class CanvasStrategy(
 
   override fun close() {
     isClosed.set(true)
-    screenshotLock.acquire().use {
-      screenshot?.apply {
-        if (!isRecycled) {
-          recycle()
-        }
-      }
-      screenshot = null
-    }
+    executor
+      .getExecutor()
+      .submit(
+        ReplayRunnable(
+          "CanvasStrategy.close",
+          {
+            screenshot?.let {
+              synchronized(it) {
+                if (!it.isRecycled) {
+                  it.recycle()
+                }
+              }
+            }
+          },
+        )
+      )
+
     // the image can be free, unprocessed or in transit
     freePictureRef.getAndSet(null)?.reader?.close()
     unprocessedPictureRef.getAndSet(null)?.reader?.close()

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt

@@ -185,12 +185,25 @@ internal class PixelCopyStrategy(
 
   override fun close() {
     isClosed.set(true)
-    if (!screenshot.isRecycled) {
-      screenshot.recycle()
-    }
-    if (!singlePixelBitmap.isRecycled) {
-      singlePixelBitmap.recycle()
-    }
+    executor.submit(
+      ReplayRunnable(
+        "PixelCopyStrategy.close",
+        {
+          if (!screenshot.isRecycled) {
+            synchronized(screenshot) {
+              if (!screenshot.isRecycled) {
+                screenshot.recycle()
+              }
+            }
+          }
+          // since singlePixelBitmap is only used in tasks within the single threaded executor
+          // there won't be any concurrent access
+          if (!singlePixelBitmap.isRecycled) {
+            singlePixelBitmap.recycle()
+          }
+        },
+      )
+    )
   }
 
   private fun Bitmap.dominantColorForRect(rect: Rect): Int {