[SR] Fix ANRs and speed up (#4001)

* Get rid of the  lock on touch events

* pre-allocate some things for gesture converter

* have one less thread switch for re]play

* update

* Changelog

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Change TTFD timeout to 25 seconds ([#3984](https://github.com/getsentry/sentry-java/pull/3984))
 - Session Replay: Fix memory leak when masking Compose screens ([#3985](https://github.com/getsentry/sentry-java/pull/3985))
+- Session Replay: Fix potential ANRs in `GestureRecorder` ([#4001](https://github.com/getsentry/sentry-java/pull/4001))
 
 ## 7.19.0
 

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

@@ -29,6 +29,7 @@ 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.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
@@ -46,8 +47,9 @@ import io.sentry.util.Random
 import java.io.Closeable
 import java.io.File
 import java.util.LinkedList
+import java.util.concurrent.Executors
+import java.util.concurrent.ThreadFactory
 import java.util.concurrent.atomic.AtomicBoolean
-import kotlin.LazyThreadSafetyMode.NONE
 
 public class ReplayIntegration(
     private val context: Context,
@@ -93,7 +95,10 @@ public class ReplayIntegration(
     private var recorder: Recorder? = null
     private var gestureRecorder: GestureRecorder? = null
     private val random by lazy { Random() }
-    internal val rootViewsSpy by lazy(NONE) { RootViewsSpy.install() }
+    internal val rootViewsSpy by lazy { RootViewsSpy.install() }
+    private val replayExecutor by lazy {
+        Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
+    }
 
     // TODO: probably not everything has to be thread-safe here
     internal val isEnabled = AtomicBoolean(false)
@@ -123,7 +128,7 @@ public class ReplayIntegration(
         }
 
         this.hub = hub
-        recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, mainLooperHandler)
+        recorder = recorderProvider?.invoke() ?: WindowRecorder(options, this, mainLooperHandler, replayExecutor)
         gestureRecorder = gestureRecorderProvider?.invoke() ?: GestureRecorder(options, this)
         isEnabled.set(true)
 
@@ -166,9 +171,9 @@ public class ReplayIntegration(
 
         recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.experimental.sessionReplay)
         captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) {
-            SessionCaptureStrategy(options, hub, dateProvider, replayCacheProvider = replayCacheProvider)
+            SessionCaptureStrategy(options, hub, dateProvider, replayExecutor, replayCacheProvider)
         } else {
-            BufferCaptureStrategy(options, hub, dateProvider, random, replayCacheProvider = replayCacheProvider)
+            BufferCaptureStrategy(options, hub, dateProvider, random, replayExecutor, replayCacheProvider)
         }
 
         captureStrategy?.start(recorderConfig)
@@ -229,7 +234,6 @@ public class ReplayIntegration(
         gestureRecorder?.stop()
         captureStrategy?.stop()
         isRecording.set(false)
-        captureStrategy?.close()
         captureStrategy = null
     }
 
@@ -264,6 +268,7 @@ public class ReplayIntegration(
         recorder?.close()
         recorder = null
         rootViewsSpy.close()
+        replayExecutor.gracefullyShutdown(options)
     }
 
     override fun onConfigurationChanged(newConfig: Configuration) {
@@ -405,4 +410,13 @@ public class ReplayIntegration(
     private class PreviousReplayHint : Backfillable {
         override fun shouldEnrich(): Boolean = false
     }
+
+    private class ReplayExecutorServiceThreadFactory : ThreadFactory {
+        private var cnt = 0
+        override fun newThread(r: Runnable): Thread {
+            val ret = Thread(r, "SentryReplayIntegration-" + cnt++)
+            ret.setDaemon(true)
+            return ret
+        }
+    }
 }

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

@@ -25,7 +25,6 @@ import io.sentry.SentryReplayOptions
 import io.sentry.android.replay.util.MainLooperHandler
 import io.sentry.android.replay.util.addOnDrawListenerSafe
 import io.sentry.android.replay.util.getVisibleRects
-import io.sentry.android.replay.util.gracefullyShutdown
 import io.sentry.android.replay.util.removeOnDrawListenerSafe
 import io.sentry.android.replay.util.submitSafely
 import io.sentry.android.replay.util.traverse
@@ -34,7 +33,7 @@ import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarc
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.TextViewHierarchyNode
 import java.io.File
 import java.lang.ref.WeakReference
-import java.util.concurrent.Executors
+import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.ThreadFactory
 import java.util.concurrent.atomic.AtomicBoolean
 import kotlin.LazyThreadSafetyMode.NONE
@@ -44,13 +43,11 @@ import kotlin.math.roundToInt
 internal class ScreenshotRecorder(
     val config: ScreenshotRecorderConfig,
     val options: SentryOptions,
-    val mainLooperHandler: MainLooperHandler,
+    private val mainLooperHandler: MainLooperHandler,
+    private val recorder: ScheduledExecutorService,
     private val screenshotRecorderCallback: ScreenshotRecorderCallback?
 ) : ViewTreeObserver.OnDrawListener {
 
-    private val recorder by lazy {
-        Executors.newSingleThreadScheduledExecutor(RecorderExecutorServiceThreadFactory())
-    }
     private var rootView: WeakReference<View>? = null
     private val maskingPaint by lazy(NONE) { Paint() }
     private val singlePixelBitmap: Bitmap by lazy(NONE) {
@@ -231,7 +228,6 @@ internal class ScreenshotRecorder(
         rootView?.clear()
         lastScreenshot?.recycle()
         isCapturing.set(false)
-        recorder.gracefullyShutdown(options)
     }
 
     private fun Bitmap.dominantColorForRect(rect: Rect): Int {

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

@@ -8,6 +8,7 @@ import io.sentry.android.replay.util.gracefullyShutdown
 import io.sentry.android.replay.util.scheduleAtFixedRateSafely
 import java.lang.ref.WeakReference
 import java.util.concurrent.Executors
+import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.ScheduledFuture
 import java.util.concurrent.ThreadFactory
 import java.util.concurrent.TimeUnit.MILLISECONDS
@@ -17,7 +18,8 @@ import java.util.concurrent.atomic.AtomicBoolean
 internal class WindowRecorder(
     private val options: SentryOptions,
     private val screenshotRecorderCallback: ScreenshotRecorderCallback? = null,
-    private val mainLooperHandler: MainLooperHandler
+    private val mainLooperHandler: MainLooperHandler,
+    private val replayExecutor: ScheduledExecutorService
 ) : Recorder, OnRootViewsChangedListener {
 
     internal companion object {
@@ -57,7 +59,9 @@ internal class WindowRecorder(
             return
         }
 
-        recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, screenshotRecorderCallback)
+        recorder = ScreenshotRecorder(recorderConfig, options, mainLooperHandler, replayExecutor, screenshotRecorderCallback)
+        // TODO: change this to use MainThreadHandler and just post on the main thread with delay
+        // to avoid thread context switch every time
         capturingTask = capturer.scheduleAtFixedRateSafely(
             options,
             "$TAG.capture",

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

@@ -1,5 +1,6 @@
 package io.sentry.android.replay.capture
 
+import android.annotation.TargetApi
 import android.view.MotionEvent
 import io.sentry.Breadcrumb
 import io.sentry.DateUtils
@@ -14,25 +15,22 @@ import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_FRAME_RATE
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_HEIGHT
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_ID
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_ID
-import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_RECORDING
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_SCREEN_AT_START
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_REPLAY_TYPE
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_TIMESTAMP
 import io.sentry.android.replay.ReplayCache.Companion.SEGMENT_KEY_WIDTH
 import io.sentry.android.replay.ScreenshotRecorderConfig
 import io.sentry.android.replay.capture.CaptureStrategy.Companion.createSegment
-import io.sentry.android.replay.capture.CaptureStrategy.Companion.currentEventsLock
 import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
 import io.sentry.android.replay.gestures.ReplayGestureConverter
-import io.sentry.android.replay.util.PersistableLinkedList
-import io.sentry.android.replay.util.gracefullyShutdown
 import io.sentry.android.replay.util.submitSafely
 import io.sentry.protocol.SentryId
 import io.sentry.rrweb.RRWebEvent
 import io.sentry.transport.ICurrentDateProvider
 import java.io.File
 import java.util.Date
-import java.util.LinkedList
+import java.util.Deque
+import java.util.concurrent.ConcurrentLinkedDeque
 import java.util.concurrent.Executors
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.ThreadFactory
@@ -42,11 +40,12 @@ import java.util.concurrent.atomic.AtomicReference
 import kotlin.properties.ReadWriteProperty
 import kotlin.reflect.KProperty
 
+@TargetApi(26)
 internal abstract class BaseCaptureStrategy(
     private val options: SentryOptions,
     private val hub: IHub?,
     private val dateProvider: ICurrentDateProvider,
-    executor: ScheduledExecutorService? = null,
+    protected val replayExecutor: ScheduledExecutorService,
     private val replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
 ) : CaptureStrategy {
 
@@ -81,16 +80,8 @@ internal abstract class BaseCaptureStrategy(
     override val replayCacheDir: File? get() = cache?.replayCacheDir
 
     override var replayType by persistableAtomic<ReplayType>(propertyName = SEGMENT_KEY_REPLAY_TYPE)
-    protected val currentEvents: LinkedList<RRWebEvent> = PersistableLinkedList(
-        propertyName = SEGMENT_KEY_REPLAY_RECORDING,
-        options,
-        persistingExecutor,
-        cacheProvider = { cache }
-    )
-
-    protected val replayExecutor: ScheduledExecutorService by lazy {
-        executor ?: Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
-    }
+
+    protected val currentEvents: Deque<RRWebEvent> = ConcurrentLinkedDeque()
 
     override fun start(
         recorderConfig: ScreenshotRecorderConfig,
@@ -135,7 +126,7 @@ internal abstract class BaseCaptureStrategy(
         frameRate: Int = recorderConfig.frameRate,
         screenAtStart: String? = this.screenAtStart,
         breadcrumbs: List<Breadcrumb>? = null,
-        events: LinkedList<RRWebEvent> = this.currentEvents
+        events: Deque<RRWebEvent> = this.currentEvents
     ): ReplaySegment =
         createSegment(
             hub,
@@ -161,22 +152,7 @@ internal abstract class BaseCaptureStrategy(
     override fun onTouchEvent(event: MotionEvent) {
         val rrwebEvents = gestureConverter.convert(event, recorderConfig)
         if (rrwebEvents != null) {
-            synchronized(currentEventsLock) {
-                currentEvents += rrwebEvents
-            }
-        }
-    }
-
-    override fun close() {
-        replayExecutor.gracefullyShutdown(options)
-    }
-
-    private class ReplayExecutorServiceThreadFactory : ThreadFactory {
-        private var cnt = 0
-        override fun newThread(r: Runnable): Thread {
-            val ret = Thread(r, "SentryReplayIntegration-" + cnt++)
-            ret.setDaemon(true)
-            return ret
+            currentEvents += rrwebEvents
         }
     }
 

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

@@ -1,5 +1,6 @@
 package io.sentry.android.replay.capture
 
+import android.annotation.TargetApi
 import android.graphics.Bitmap
 import android.view.MotionEvent
 import io.sentry.DateUtils
@@ -23,14 +24,15 @@ import java.io.File
 import java.util.Date
 import java.util.concurrent.ScheduledExecutorService
 
+@TargetApi(26)
 internal class BufferCaptureStrategy(
     private val options: SentryOptions,
     private val hub: IHub?,
     private val dateProvider: ICurrentDateProvider,
     private val random: Random,
-    executor: ScheduledExecutorService? = null,
+    executor: ScheduledExecutorService,
     replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
-) : BaseCaptureStrategy(options, hub, dateProvider, executor = executor, replayCacheProvider = replayCacheProvider) {
+) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider = replayCacheProvider) {
 
     // TODO: capture envelopes for buffered segments instead, but don't send them until buffer is triggered
     private val bufferedSegments = mutableListOf<ReplaySegment.Created>()

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

@@ -19,6 +19,7 @@ import io.sentry.rrweb.RRWebMetaEvent
 import io.sentry.rrweb.RRWebVideoEvent
 import java.io.File
 import java.util.Date
+import java.util.Deque
 import java.util.LinkedList
 
 internal interface CaptureStrategy {
@@ -53,11 +54,8 @@ internal interface CaptureStrategy {
 
     fun convert(): CaptureStrategy
 
-    fun close()
-
     companion object {
         private const val BREADCRUMB_START_OFFSET = 100L
-        internal val currentEventsLock = Any()
 
         fun createSegment(
             hub: IHub?,
@@ -73,7 +71,7 @@ internal interface CaptureStrategy {
             frameRate: Int,
             screenAtStart: String?,
             breadcrumbs: List<Breadcrumb>?,
-            events: LinkedList<RRWebEvent>
+            events: Deque<RRWebEvent>
         ): ReplaySegment {
             val generatedVideo = cache?.createVideoOf(
                 duration,
@@ -127,7 +125,7 @@ internal interface CaptureStrategy {
             replayType: ReplayType,
             screenAtStart: String?,
             breadcrumbs: List<Breadcrumb>,
-            events: LinkedList<RRWebEvent>
+            events: Deque<RRWebEvent>
         ): ReplaySegment {
             val endTimestamp = DateUtils.getDateTime(segmentTimestamp.time + videoDuration)
             val replay = SentryReplayEvent().apply {
@@ -207,16 +205,16 @@ internal interface CaptureStrategy {
         }
 
         internal fun rotateEvents(
-            events: LinkedList<RRWebEvent>,
+            events: Deque<RRWebEvent>,
             until: Long,
             callback: ((RRWebEvent) -> Unit)? = null
         ) {
-            synchronized(currentEventsLock) {
-                var event = events.peek()
-                while (event != null && event.timestamp < until) {
+            val iter = events.iterator()
+            while (iter.hasNext()) {
+                val event = iter.next()
+                if (event.timestamp < until) {
                     callback?.invoke(event)
-                    events.remove()
-                    event = events.peek()
+                    iter.remove()
                 }
             }
         }

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

@@ -20,7 +20,7 @@ internal class SessionCaptureStrategy(
     private val options: SentryOptions,
     private val hub: IHub?,
     private val dateProvider: ICurrentDateProvider,
-    executor: ScheduledExecutorService? = null,
+    executor: ScheduledExecutorService,
     replayCacheProvider: ((replayId: SentryId, recorderConfig: ScreenshotRecorderConfig) -> ReplayCache)? = null
 ) : BaseCaptureStrategy(options, hub, dateProvider, executor, replayCacheProvider) {
 

sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/ReplayGestureConverter.kt

@@ -56,7 +56,7 @@ class ReplayGestureConverter(
 
                 val totalOffset = now - touchMoveBaseline
                 return if (totalOffset > CAPTURE_MOVE_EVENT_THRESHOLD) {
-                    val moveEvents = mutableListOf<RRWebInteractionMoveEvent>()
+                    val moveEvents = ArrayList<RRWebInteractionMoveEvent>(currentPositions.size)
                     for ((pointerId, positions) in currentPositions) {
                         if (positions.isNotEmpty()) {
                             moveEvents += RRWebInteractionMoveEvent().apply {
@@ -88,7 +88,7 @@ class ReplayGestureConverter(
                 }
 
                 // new finger down - add a new pointer for tracking movement
-                currentPositions[pId] = ArrayList()
+                currentPositions[pId] = ArrayList(10)
                 listOf(
                     RRWebInteractionEvent().apply {
                         timestamp = dateProvider.currentTimeMillis

sentry-android-replay/src/main/java/io/sentry/android/replay/util/Executors.kt

@@ -50,6 +50,11 @@ internal fun ExecutorService.submitSafely(
     taskName: String,
     task: Runnable
 ): Future<*>? {
+    if (Thread.currentThread().name.startsWith("SentryReplayIntegration")) {
+        // we're already on the worker thread, no need to submit
+        task.run()
+        return null
+    }
     return try {
         submit {
             try {