Merge a2c064c into c7cf756

CHANGELOG.md

@@ -11,6 +11,10 @@
 - Do not log if `OtelContextScopesStorage` cannot be found ([#4127](https://github.com/getsentry/sentry-java/pull/4127))
   - Previously `java.lang.ClassNotFoundException: io.sentry.opentelemetry.OtelContextScopesStorage` was shown in the log if the class could not be found.
   - This is just a lookup the SDK performs to configure itself. The SDK also works without OpenTelemetry.
+- Session Replay: Fix various crashes and issues ([#4135](https://github.com/getsentry/sentry-java/pull/4135))
+  - Fix `FileNotFoundException` when trying to read/write `.ongoing_segment` file
+  - Fix `IllegalStateException` when registering `onDrawListener`
+  - Fix SIGABRT native crashes on Motorola devices when encoding a video
 - Mention javadoc and sources for published artifacts in Gradle `.module` metadata ([#3936](https://github.com/getsentry/sentry-java/pull/3936))
 
 ### Dependencies

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

@@ -55,7 +55,7 @@ public class ReplayCache(
     internal val frames = mutableListOf<ReplayFrame>()
 
     private val ongoingSegment = LinkedHashMap<String, String>()
-    private val ongoingSegmentFile: File? by lazy {
+    internal val ongoingSegmentFile: File? by lazy {
         if (replayCacheDir == null) {
             return@lazy null
         }
@@ -275,6 +275,9 @@ public class ReplayCache(
             if (isClosed.get()) {
                 return
             }
+            if (ongoingSegmentFile?.exists() != true) {
+                ongoingSegmentFile?.createNewFile()
+            }
             if (ongoingSegment.isEmpty()) {
                 ongoingSegmentFile?.useLines { lines ->
                     lines.associateTo(ongoingSegment) {

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

@@ -21,6 +21,11 @@ import io.sentry.SentryIntegrationPackageStorage
 import io.sentry.SentryLevel.DEBUG
 import io.sentry.SentryLevel.INFO
 import io.sentry.SentryOptions
+import io.sentry.android.replay.ReplayState.CLOSED
+import io.sentry.android.replay.ReplayState.PAUSED
+import io.sentry.android.replay.ReplayState.RESUMED
+import io.sentry.android.replay.ReplayState.STARTED
+import io.sentry.android.replay.ReplayState.STOPPED
 import io.sentry.android.replay.capture.BufferCaptureStrategy
 import io.sentry.android.replay.capture.CaptureStrategy
 import io.sentry.android.replay.capture.CaptureStrategy.ReplaySegment
@@ -40,6 +45,7 @@ import io.sentry.protocol.SentryId
 import io.sentry.transport.ICurrentDateProvider
 import io.sentry.transport.RateLimiter
 import io.sentry.transport.RateLimiter.IRateLimitObserver
+import io.sentry.util.AutoClosableReentrantLock
 import io.sentry.util.FileUtils
 import io.sentry.util.HintUtils
 import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
@@ -100,15 +106,16 @@ public class ReplayIntegration(
         Executors.newSingleThreadScheduledExecutor(ReplayExecutorServiceThreadFactory())
     }
 
-    // TODO: probably not everything has to be thread-safe here
     internal val isEnabled = AtomicBoolean(false)
-    private val isRecording = AtomicBoolean(false)
+    internal val isManualPause = AtomicBoolean(false)
     private var captureStrategy: CaptureStrategy? = null
     public val replayCacheDir: File? get() = captureStrategy?.replayCacheDir
     private var replayBreadcrumbConverter: ReplayBreadcrumbConverter = NoOpReplayBreadcrumbConverter.getInstance()
     private var replayCaptureStrategyProvider: ((isFullSession: Boolean) -> CaptureStrategy)? = null
     private var mainLooperHandler: MainLooperHandler = MainLooperHandler()
     private var gestureRecorderProvider: (() -> GestureRecorder)? = null
+    private val lifecycleLock = AutoClosableReentrantLock()
+    private val lifecycle = ReplayLifecycle()
 
     override fun register(scopes: IScopes, options: SentryOptions) {
         this.options = options
@@ -151,51 +158,68 @@ public class ReplayIntegration(
         finalizePreviousReplay()
     }
 
-    override fun isRecording(): Boolean = isRecording.get()
+    override fun isRecording(): Boolean = lifecycle.currentState >= STARTED && lifecycle.currentState < STOPPED
 
     override fun start() {
-        // TODO: add lifecycle state instead and manage it in start/pause/resume/stop
-        if (!isEnabled.get()) {
-            return
-        }
+        lifecycleLock.acquire().use {
+            if (!isEnabled.get()) {
+                return
+            }
 
-        if (isRecording.getAndSet(true)) {
-            options.logger.log(
-                DEBUG,
-                "Session replay is already being recorded, not starting a new one"
-            )
-            return
-        }
+            if (!lifecycle.isAllowed(STARTED)) {
+                options.logger.log(
+                    DEBUG,
+                    "Session replay is already being recorded, not starting a new one"
+                )
+                return
+            }
 
-        val isFullSession = random.sample(options.sessionReplay.sessionSampleRate)
-        if (!isFullSession && !options.sessionReplay.isSessionReplayForErrorsEnabled) {
-            options.logger.log(INFO, "Session replay is not started, full session was not sampled and onErrorSampleRate is not specified")
-            return
-        }
+            val isFullSession = random.sample(options.sessionReplay.sessionSampleRate)
+            if (!isFullSession && !options.sessionReplay.isSessionReplayForErrorsEnabled) {
+                options.logger.log(INFO, "Session replay is not started, full session was not sampled and onErrorSampleRate is not specified")
+                return
+            }
 
-        val recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.sessionReplay)
-        captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) {
-            SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, replayCacheProvider)
-        } else {
-            BufferCaptureStrategy(options, scopes, dateProvider, random, replayExecutor, replayCacheProvider)
-        }
+            val recorderConfig = recorderConfigProvider?.invoke(false) ?: ScreenshotRecorderConfig.from(context, options.sessionReplay)
+            captureStrategy = replayCaptureStrategyProvider?.invoke(isFullSession) ?: if (isFullSession) {
+                SessionCaptureStrategy(options, scopes, dateProvider, replayExecutor, replayCacheProvider)
+            } else {
+                BufferCaptureStrategy(options, scopes, dateProvider, random, replayExecutor, replayCacheProvider)
+            }
 
-        captureStrategy?.start(recorderConfig)
-        recorder?.start(recorderConfig)
-        registerRootViewListeners()
+            captureStrategy?.start(recorderConfig)
+            recorder?.start(recorderConfig)
+            registerRootViewListeners()
+            lifecycle.currentState = STARTED
+        }
     }
 
     override fun resume() {
-        if (!isEnabled.get() || !isRecording.get()) {
-            return
-        }
+        isManualPause.set(false)
+        resumeInternal()
+    }
 
-        captureStrategy?.resume()
-        recorder?.resume()
+    private fun resumeInternal() {
+        lifecycleLock.acquire().use {
+            if (!isEnabled.get() || !lifecycle.isAllowed(RESUMED)) {
+                return
+            }
+
+            if (isManualPause.get() || options.connectionStatusProvider.connectionStatus == DISCONNECTED ||
+                scopes?.rateLimiter?.isActiveForCategory(All) == true ||
+                scopes?.rateLimiter?.isActiveForCategory(Replay) == true
+            ) {
+                return
+            }
+
+            captureStrategy?.resume()
+            recorder?.resume()
+            lifecycle.currentState = RESUMED
+        }
     }
 
     override fun captureReplay(isTerminating: Boolean?) {
-        if (!isEnabled.get() || !isRecording.get()) {
+        if (!isEnabled.get() || !isRecording()) {
             return
         }
 
@@ -220,25 +244,35 @@ public class ReplayIntegration(
     override fun getBreadcrumbConverter(): ReplayBreadcrumbConverter = replayBreadcrumbConverter
 
     override fun pause() {
-        if (!isEnabled.get() || !isRecording.get()) {
-            return
-        }
+        isManualPause.set(true)
+        pauseInternal()
+    }
 
-        recorder?.pause()
-        captureStrategy?.pause()
+    private fun pauseInternal() {
+        lifecycleLock.acquire().use {
+            if (!isEnabled.get() || !lifecycle.isAllowed(PAUSED)) {
+                return
+            }
+
+            recorder?.pause()
+            captureStrategy?.pause()
+            lifecycle.currentState = PAUSED
+        }
     }
 
     override fun stop() {
-        if (!isEnabled.get() || !isRecording.get()) {
-            return
-        }
+        lifecycleLock.acquire().use {
+            if (!isEnabled.get() || !lifecycle.isAllowed(STOPPED)) {
+                return
+            }
 
-        unregisterRootViewListeners()
-        recorder?.stop()
-        gestureRecorder?.stop()
-        captureStrategy?.stop()
-        isRecording.set(false)
-        captureStrategy = null
+            unregisterRootViewListeners()
+            recorder?.stop()
+            gestureRecorder?.stop()
+            captureStrategy?.stop()
+            captureStrategy = null
+            lifecycle.currentState = STOPPED
+        }
     }
 
     override fun onScreenshotRecorded(bitmap: Bitmap) {
@@ -258,27 +292,30 @@ public class ReplayIntegration(
     }
 
     override fun close() {
-        if (!isEnabled.get()) {
-            return
-        }
+        lifecycleLock.acquire().use {
+            if (!isEnabled.get() || !lifecycle.isAllowed(CLOSED)) {
+                return
+            }
 
-        options.connectionStatusProvider.removeConnectionStatusObserver(this)
-        scopes?.rateLimiter?.removeRateLimitObserver(this)
-        if (options.sessionReplay.isTrackOrientationChange) {
-            try {
-                context.unregisterComponentCallbacks(this)
-            } catch (ignored: Throwable) {
+            options.connectionStatusProvider.removeConnectionStatusObserver(this)
+            scopes?.rateLimiter?.removeRateLimitObserver(this)
+            if (options.sessionReplay.isTrackOrientationChange) {
+                try {
+                    context.unregisterComponentCallbacks(this)
+                } catch (ignored: Throwable) {
+                }
             }
+            stop()
+            recorder?.close()
+            recorder = null
+            rootViewsSpy.close()
+            replayExecutor.gracefullyShutdown(options)
+            lifecycle.currentState = CLOSED
         }
-        stop()
-        recorder?.close()
-        recorder = null
-        rootViewsSpy.close()
-        replayExecutor.gracefullyShutdown(options)
     }
 
     override fun onConfigurationChanged(newConfig: Configuration) {
-        if (!isEnabled.get() || !isRecording.get()) {
+        if (!isEnabled.get() || !isRecording()) {
             return
         }
 
@@ -289,6 +326,10 @@ public class ReplayIntegration(
         captureStrategy?.onConfigurationChanged(recorderConfig)
 
         recorder?.start(recorderConfig)
+        // we have to restart recorder with a new config and pause immediately if the replay is paused
+        if (lifecycle.currentState == PAUSED) {
+            recorder?.pause()
+        }
     }
 
     override fun onConnectionStatusChanged(status: ConnectionStatus) {
@@ -298,10 +339,10 @@ public class ReplayIntegration(
         }
 
         if (status == DISCONNECTED) {
-            pause()
+            pauseInternal()
         } else {
             // being positive for other states, even if it's NO_PERMISSION
-            resume()
+            resumeInternal()
         }
     }
 
@@ -312,15 +353,18 @@ public class ReplayIntegration(
         }
 
         if (rateLimiter.isActiveForCategory(All) || rateLimiter.isActiveForCategory(Replay)) {
-            pause()
+            pauseInternal()
         } else {
-            resume()
+            resumeInternal()
         }
     }
 
     override fun onLowMemory(): Unit = Unit
 
     override fun onTouchEvent(event: MotionEvent) {
+        if (!isEnabled.get() || !lifecycle.isTouchRecordingAllowed()) {
+            return
+        }
         captureStrategy?.onTouchEvent(event)
     }
 
@@ -336,7 +380,7 @@ public class ReplayIntegration(
                     scopes?.rateLimiter?.isActiveForCategory(Replay) == true
                 )
         ) {
-            pause()
+            pauseInternal()
         }
     }
 

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

@@ -0,0 +1,58 @@
+package io.sentry.android.replay
+
+internal enum class ReplayState {
+    /**
+     * Initial state of a Replay session. This is the state when ReplayIntegration is constructed
+     * but has not been started yet.
+     */
+    INITIAL,
+
+    /**
+     * Started state for a Replay session. This state is reached after the start() method is called
+     * and the recording is initialized successfully.
+     */
+    STARTED,
+
+    /**
+     * Resumed state for a Replay session. This state is reached after resume() is called on an
+     * already started recording.
+     */
+    RESUMED,
+
+    /**
+     * Paused state for a Replay session. This state is reached after pause() is called on a
+     * resumed recording.
+     */
+    PAUSED,
+
+    /**
+     * Stopped state for a Replay session. This state is reached after stop() is called.
+     * The recording can be started again from this state.
+     */
+    STOPPED,
+
+    /**
+     * Closed state for a Replay session. This is the terminal state reached after close() is called.
+     * No further state transitions are possible after this.
+     */
+    CLOSED;
+}
+
+/**
+ * Class to manage state transitions for ReplayIntegration
+ */
+internal class ReplayLifecycle {
+    @field:Volatile
+    internal var currentState = ReplayState.INITIAL
+
+    fun isAllowed(newState: ReplayState): Boolean = when (currentState) {
+        ReplayState.INITIAL -> newState == ReplayState.STARTED || newState == ReplayState.CLOSED
+        ReplayState.STARTED -> newState == ReplayState.PAUSED || newState == ReplayState.STOPPED || newState == ReplayState.CLOSED
+        ReplayState.RESUMED -> newState == ReplayState.PAUSED || newState == ReplayState.STOPPED || newState == ReplayState.CLOSED
+        ReplayState.PAUSED -> newState == ReplayState.RESUMED || newState == ReplayState.STOPPED || newState == ReplayState.CLOSED
+        ReplayState.STOPPED -> newState == ReplayState.STARTED || newState == ReplayState.CLOSED
+        ReplayState.CLOSED -> false
+    }
+
+    fun isTouchRecordingAllowed(): Boolean = currentState == ReplayState.STARTED || currentState == ReplayState.RESUMED
+}

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

@@ -5,6 +5,7 @@ import android.view.MotionEvent
 import io.sentry.Breadcrumb
 import io.sentry.DateUtils
 import io.sentry.IScopes
+import io.sentry.SentryLevel.ERROR
 import io.sentry.SentryOptions
 import io.sentry.SentryReplayEvent.ReplayType
 import io.sentry.SentryReplayEvent.ReplayType.BUFFER
@@ -183,7 +184,11 @@ internal abstract class BaseCaptureStrategy(
                         task()
                     }
                 } else {
-                    task()
+                    try {
+                        task()
+                    } catch (e: Throwable) {
+                        options.logger.log(ERROR, "Failed to execute task $TAG.runInBackground", e)
+                    }
                 }
             }