fix(replay): Do not use recycled screenshots (#4790)

Author: romtsn

CHANGELOG.md

@@ -37,6 +37,8 @@
 - Session Replay: Avoid deadlock when pausing replay if no connection ([#4788](https://github.com/getsentry/sentry-java/pull/4788))
 - Session Replay: Fix capturing roots with no windows ([#4805](https://github.com/getsentry/sentry-java/pull/4805))
 - Session Replay: Fix `java.lang.IllegalArgumentException: width and height must be > 0` ([#4805](https://github.com/getsentry/sentry-java/pull/4805))
+- 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`
 
 ### Miscellaneous
 

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

@@ -31,8 +31,8 @@ import io.sentry.android.replay.capture.SessionCaptureStrategy
 import io.sentry.android.replay.gestures.GestureRecorder
 import io.sentry.android.replay.gestures.TouchRecorderCallback
 import io.sentry.android.replay.util.MainLooperHandler
+import io.sentry.android.replay.util.ReplayExecutorService
 import io.sentry.android.replay.util.appContext
-import io.sentry.android.replay.util.gracefullyShutdown
 import io.sentry.android.replay.util.sample
 import io.sentry.android.replay.util.submitSafely
 import io.sentry.cache.PersistingScopeObserver.BREADCRUMBS_FILENAME
@@ -103,7 +103,8 @@ public class ReplayIntegration(
   private val random by lazy { Random() }
   internal val rootViewsSpy by lazy { RootViewsSpy.install() }
   private val replayExecutor by lazy {
-    Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
+    val delegate = Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
+    ReplayExecutorService(delegate, options)
   }
 
   internal val isEnabled = AtomicBoolean(false)
@@ -328,7 +329,7 @@ public class ReplayIntegration(
       recorder?.close()
       recorder = null
       rootViewsSpy.close()
-      replayExecutor.gracefullyShutdown(options)
+      replayExecutor.shutdown()
       lifecycle.currentState = CLOSED
     }
   }

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BaseCaptureStrategy.kt

@@ -24,7 +24,8 @@ import io.sentry.android.replay.ScreenshotRecorderConfig
 import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment
 import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
 import io.sentry.android.replay.gestures.ReplayGestureConverter
-import io.sentry.android.replay.util.submitSafely
+import io.sentry.android.replay.util.ReplayExecutorService
+import io.sentry.android.replay.util.ReplayRunnable
 import io.sentry.protocol.SentryId
 import io.sentry.rrweb.RRWebEvent
 import io.sentry.transport.ICurrentDateProvider
@@ -54,7 +55,9 @@ internal abstract class BaseCaptureStrategy(
   }
 
   private val persistingExecutor: ScheduledExecutorService by lazy {
-    Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory())
+    val delegate =
+      Executors.newSingleThreadScheduledExecutor(ReplayPersistingExecutorServiceThreadFactory())
+    ReplayExecutorService(delegate, options)
   }
   private val gestureConverter = ReplayGestureConverter(dateProvider)
 
@@ -185,7 +188,7 @@ internal abstract class BaseCaptureStrategy(
 
       private fun runInBackground(task: () -> Unit) {
         if (options.threadChecker.isMainThread) {
-          persistingExecutor.submitSafely(options, "$TAG.runInBackground") { task() }
+          persistingExecutor.submit(ReplayRunnable("$TAG.runInBackground") { task() })
         } else {
           try {
             task()

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/BufferCaptureStrategy.kt

@@ -14,8 +14,8 @@ import io.sentry.android.replay.ReplayCache
 import io.sentry.android.replay.ScreenshotRecorderConfig
 import io.sentry.android.replay.capture.CaptureStrategy.Companion.rotateEvents
 import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
+import io.sentry.android.replay.util.ReplayRunnable
 import io.sentry.android.replay.util.sample
-import io.sentry.android.replay.util.submitSafely
 import io.sentry.protocol.SentryId
 import io.sentry.transport.ICurrentDateProvider
 import io.sentry.util.FileUtils
@@ -62,10 +62,12 @@ internal class BufferCaptureStrategy(
 
   override fun stop() {
     val replayCacheDir = cache?.replayCacheDir
-    replayExecutor.submitSafely(options, "$TAG.stop") {
-      FileUtils.deleteRecursively(replayCacheDir)
-      currentSegment = -1
-    }
+    replayExecutor.submit(
+      ReplayRunnable("$TAG.stop") {
+        FileUtils.deleteRecursively(replayCacheDir)
+        currentSegment = -1
+      }
+    )
     super.stop()
   }
 
@@ -115,14 +117,16 @@ internal class BufferCaptureStrategy(
     // have to do it before submitting, otherwise if the queue is busy, the timestamp won't be
     // reflecting the exact time of when it was captured
     val frameTimestamp = dateProvider.currentTimeMillis
-    replayExecutor.submitSafely(options, "$TAG.add_frame") {
-      cache?.store(frameTimestamp)
-
-      val now = dateProvider.currentTimeMillis
-      val bufferLimit = now - options.sessionReplay.errorReplayDuration
-      screenAtStart = cache?.rotate(bufferLimit)
-      bufferedSegments.rotate(bufferLimit)
-    }
+    replayExecutor.submit(
+      ReplayRunnable("$TAG.add_frame") {
+        cache?.store(frameTimestamp)
+
+        val now = dateProvider.currentTimeMillis
+        val bufferLimit = now - options.sessionReplay.errorReplayDuration
+        screenAtStart = cache?.rotate(bufferLimit)
+        bufferedSegments.rotate(bufferLimit)
+      }
+    )
   }
 
   override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) {
@@ -225,19 +229,21 @@ internal class BufferCaptureStrategy(
     val duration = now - currentSegmentTimestamp.time
     val replayId = currentReplayId
 
-    replayExecutor.submitSafely(options, "$TAG.$taskName") {
-      val segment =
-        createSegmentInternal(
-          duration,
-          currentSegmentTimestamp,
-          replayId,
-          currentSegment,
-          currentConfig.recordingHeight,
-          currentConfig.recordingWidth,
-          currentConfig.frameRate,
-          currentConfig.bitRate,
-        )
-      onSegmentCreated(segment)
-    }
+    replayExecutor.submit(
+      ReplayRunnable("$TAG.$taskName") {
+        val segment =
+          createSegmentInternal(
+            duration,
+            currentSegmentTimestamp,
+            replayId,
+            currentSegment,
+            currentConfig.recordingHeight,
+            currentConfig.recordingWidth,
+            currentConfig.frameRate,
+            currentConfig.bitRate,
+          )
+        onSegmentCreated(segment)
+      }
+    )
   }
 }

sentry-android-replay/src/main/java/io/sentry/android/replay/capture/SessionCaptureStrategy.kt

@@ -9,7 +9,7 @@ import io.sentry.SentryReplayEvent.ReplayType
 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.submitSafely
+import io.sentry.android.replay.util.ReplayRunnable
 import io.sentry.protocol.SentryId
 import io.sentry.transport.ICurrentDateProvider
 import io.sentry.util.FileUtils
@@ -79,55 +79,57 @@ internal class SessionCaptureStrategy(
     // reflecting the exact time of when it was captured
     val currentConfig = recorderConfig
     val frameTimestamp = dateProvider.currentTimeMillis
-    replayExecutor.submitSafely(options, "$TAG.add_frame") {
-      cache?.store(frameTimestamp)
-
-      val currentSegmentTimestamp = segmentTimestamp
-      currentSegmentTimestamp
-        ?: run {
-          options.logger.log(DEBUG, "Segment timestamp is not set, not recording frame")
-          return@submitSafely
+    replayExecutor.submit(
+      ReplayRunnable("$TAG.add_frame") {
+        cache?.store(frameTimestamp)
+
+        val currentSegmentTimestamp = segmentTimestamp
+        currentSegmentTimestamp
+          ?: run {
+            options.logger.log(DEBUG, "Segment timestamp is not set, not recording frame")
+            return@ReplayRunnable
+          }
+
+        if (isTerminating.get()) {
+          options.logger.log(
+            DEBUG,
+            "Not capturing segment, because the app is terminating, will be captured on next launch",
+          )
+          return@ReplayRunnable
         }
 
-      if (isTerminating.get()) {
-        options.logger.log(
-          DEBUG,
-          "Not capturing segment, because the app is terminating, will be captured on next launch",
-        )
-        return@submitSafely
-      }
-
-      if (currentConfig == null) {
-        options.logger.log(DEBUG, "Recorder config is not set, not capturing a segment")
-        return@submitSafely
-      }
+        if (currentConfig == null) {
+          options.logger.log(DEBUG, "Recorder config is not set, not capturing a segment")
+          return@ReplayRunnable
+        }
 
-      val now = dateProvider.currentTimeMillis
-      if ((now - currentSegmentTimestamp.time >= options.sessionReplay.sessionSegmentDuration)) {
-        val segment =
-          createSegmentInternal(
-            options.sessionReplay.sessionSegmentDuration,
-            currentSegmentTimestamp,
-            currentReplayId,
-            currentSegment,
-            currentConfig.recordingHeight,
-            currentConfig.recordingWidth,
-            currentConfig.frameRate,
-            currentConfig.bitRate,
-          )
-        if (segment is ReplaySegment.Created) {
-          segment.capture(scopes)
-          currentSegment++
-          // set next segment timestamp as close to the previous one as possible to avoid gaps
-          segmentTimestamp = segment.replay.timestamp
+        val now = dateProvider.currentTimeMillis
+        if ((now - currentSegmentTimestamp.time >= options.sessionReplay.sessionSegmentDuration)) {
+          val segment =
+            createSegmentInternal(
+              options.sessionReplay.sessionSegmentDuration,
+              currentSegmentTimestamp,
+              currentReplayId,
+              currentSegment,
+              currentConfig.recordingHeight,
+              currentConfig.recordingWidth,
+              currentConfig.frameRate,
+              currentConfig.bitRate,
+            )
+          if (segment is ReplaySegment.Created) {
+            segment.capture(scopes)
+            currentSegment++
+            // set next segment timestamp as close to the previous one as possible to avoid gaps
+            segmentTimestamp = segment.replay.timestamp
+          }
         }
-      }
 
-      if ((now - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration)) {
-        options.replayController.stop()
-        options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording")
+        if ((now - replayStartTimestamp.get() >= options.sessionReplay.sessionDuration)) {
+          options.replayController.stop()
+          options.logger.log(INFO, "Session replay deadline exceeded (1h), stopping recording")
+        }
       }
-    }
+    )
   }
 
   override fun onConfigurationChanged(recorderConfig: ScreenshotRecorderConfig) {
@@ -161,19 +163,21 @@ internal class SessionCaptureStrategy(
     val currentSegmentTimestamp = segmentTimestamp ?: return
     val duration = now - currentSegmentTimestamp.time
     val replayId = currentReplayId
-    replayExecutor.submitSafely(options, "$TAG.$taskName") {
-      val segment =
-        createSegmentInternal(
-          duration,
-          currentSegmentTimestamp,
-          replayId,
-          currentSegment,
-          currentConfig.recordingHeight,
-          currentConfig.recordingWidth,
-          currentConfig.frameRate,
-          currentConfig.bitRate,
-        )
-      onSegmentCreated(segment)
-    }
+    replayExecutor.submit(
+      ReplayRunnable("$TAG.$taskName") {
+        val segment =
+          createSegmentInternal(
+            duration,
+            currentSegmentTimestamp,
+            replayId,
+            currentSegment,
+            currentConfig.recordingHeight,
+            currentConfig.recordingWidth,
+            currentConfig.frameRate,
+            currentConfig.bitRate,
+          )
+        onSegmentCreated(segment)
+      }
+    )
   }
 }

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

@@ -32,7 +32,7 @@ import io.sentry.SentryOptions
 import io.sentry.android.replay.ExecutorProvider
 import io.sentry.android.replay.ScreenshotRecorderCallback
 import io.sentry.android.replay.ScreenshotRecorderConfig
-import io.sentry.android.replay.util.submitSafely
+import io.sentry.android.replay.util.ReplayRunnable
 import io.sentry.util.AutoClosableReentrantLock
 import io.sentry.util.IntegrationUtils
 import java.io.Closeable
@@ -173,7 +173,7 @@ internal class CanvasStrategy(
       holder.close()
     } else {
       unprocessedPictureRef.set(holder)
-      executor.getExecutor().submitSafely(options, "screenshot_recorder.canvas", pictureRenderTask)
+      executor.getExecutor().submit(ReplayRunnable("screenshot_recorder.canvas", pictureRenderTask))
     }
   }