Merge 13bfd36 into aed2cf7

Author: romtsn

.github/workflows/agp-matrix.yml

@@ -70,7 +70,7 @@ jobs:
           arch: x86
           channel: canary # Necessary for ATDs
           disk-size: 4096M
-          script: ./gradlew sentry-android-integration-tests:sentry-uitest-android:connectedReleaseAndroidTest -DtestBuildType=release --daemon
+          script: ./gradlew sentry-android-integration-tests:sentry-uitest-android:connectedReleaseAndroidTest -DtestBuildType=release -Denvironment=github --daemon
 
       - name: Upload test results
         if: always()

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - 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))
 
 ## 7.19.0
 

buildSrc/src/main/java/Config.kt

@@ -194,6 +194,7 @@ object Config {
         val mockitoKotlin = "org.mockito.kotlin:mockito-kotlin:4.1.0"
         val mockitoInline = "org.mockito:mockito-inline:4.8.0"
         val awaitility = "org.awaitility:awaitility-kotlin:4.1.1"
+        val awaitility3 = "org.awaitility:awaitility-kotlin:3.1.6" // need this due to a conflict of awaitility4+ and espresso on hamcrest
         val mockWebserver = "com.squareup.okhttp3:mockwebserver:${Libs.okHttpVersion}"
         val jsonUnit = "net.javacrumbs.json-unit:json-unit:2.32.0"
         val hsqldb = "org.hsqldb:hsqldb:2.6.1"

sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts

@@ -26,6 +26,7 @@ android {
         // This doesn't work on some devices with Android 11+. Clearing package data resets permissions.
         // Check the readme for more info.
         testInstrumentationRunnerArguments["clearPackageData"] = "true"
+        buildConfigField("String", "ENVIRONMENT", "\"${System.getProperty("environment", "")}\"")
     }
 
     testOptions {
@@ -125,6 +126,7 @@ dependencies {
     androidTestImplementation(Config.TestLibs.mockWebserver)
     androidTestImplementation(Config.TestLibs.androidxJunit)
     androidTestImplementation(Config.TestLibs.leakCanaryInstrumentation)
+    androidTestImplementation(Config.TestLibs.awaitility3)
     androidTestUtil(Config.TestLibs.androidxTestOrchestrator)
 }
 

sentry-android-integration-tests/sentry-uitest-android/proguard-rules.pro

@@ -40,3 +40,4 @@
 -dontwarn org.mockito.internal.**
 -dontwarn org.jetbrains.annotations.**
 -dontwarn io.sentry.android.replay.ReplayIntegration
+-keep class curtains.** { *; }

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/ReplayTest.kt

@@ -0,0 +1,84 @@
+package io.sentry.uitest.android
+
+import androidx.lifecycle.Lifecycle
+import androidx.test.core.app.launchActivity
+import io.sentry.SentryOptions
+import leakcanary.LeakAssertions
+import leakcanary.LeakCanary
+import org.awaitility.kotlin.await
+import org.hamcrest.CoreMatchers.`is`
+import org.junit.Assume.assumeThat
+import org.junit.Before
+import shark.AndroidReferenceMatchers
+import shark.IgnoredReferenceMatcher
+import shark.ReferencePattern
+import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.test.Test
+
+class ReplayTest : BaseUiTest() {
+
+    @Before
+    fun setup() {
+        // we can't run on GH actions emulator, because they don't allow capturing screenshots properly
+        @Suppress("KotlinConstantConditions")
+        assumeThat(
+            BuildConfig.ENVIRONMENT != "github",
+            `is`(true)
+        )
+    }
+
+    @Test
+    fun composeReplayDoesNotLeak() {
+        val sent = AtomicBoolean(false)
+
+        LeakCanary.config = LeakCanary.config.copy(
+            referenceMatchers = AndroidReferenceMatchers.appDefaults +
+                listOf(
+                    IgnoredReferenceMatcher(
+                        ReferencePattern.InstanceFieldPattern(
+                            "com.saucelabs.rdcinjector.testfairy.TestFairyEventQueue",
+                            "context"
+                        )
+                    ),
+                    // Seems like a false-positive returned by LeakCanary when curtains is used in
+                    // the host application (LeakCanary uses it itself internally). We use kind of
+                    // the same approach which possibly clashes with LeakCanary's internal state.
+                    // Only the case when replay is enabled.
+                    // TODO: check if it's actually a leak on our side, or a false-positive and report to LeakCanary's github issue tracker
+                    IgnoredReferenceMatcher(
+                        ReferencePattern.InstanceFieldPattern(
+                            "curtains.internal.RootViewsSpy",
+                            "delegatingViewList"
+                        )
+                    )
+                ) + ('a'..'z').map { char ->
+                IgnoredReferenceMatcher(
+                    ReferencePattern.StaticFieldPattern(
+                        "com.testfairy.modules.capture.TouchListener",
+                        "$char"
+                    )
+                )
+            }
+        )
+
+        val activityScenario = launchActivity<ComposeActivity>()
+        activityScenario.moveToState(Lifecycle.State.RESUMED)
+
+        initSentry {
+            it.experimental.sessionReplay.sessionSampleRate = 1.0
+
+            it.beforeSendReplay =
+                SentryOptions.BeforeSendReplayCallback { event, _ ->
+                    sent.set(true)
+                    event
+                }
+        }
+
+        // wait until first segment is being sent
+        await.untilTrue(sent)
+
+        activityScenario.moveToState(Lifecycle.State.DESTROYED)
+
+        LeakAssertions.assertNoLeaks()
+    }
+}

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

@@ -8,6 +8,7 @@ import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.ColorProducer
 import androidx.compose.ui.graphics.painter.Painter
 import androidx.compose.ui.layout.LayoutCoordinates
+import androidx.compose.ui.layout.findRootCoordinates
 import androidx.compose.ui.node.LayoutNode
 import androidx.compose.ui.text.TextLayoutResult
 import kotlin.math.roundToInt
@@ -165,8 +166,8 @@ private inline fun Float.fastCoerceAtMost(maximumValue: Float): Float {
  *
  * @return boundaries of this layout relative to the window's origin.
  */
-internal fun LayoutCoordinates.boundsInWindow(root: LayoutCoordinates?): Rect {
-    root ?: return Rect()
+internal fun LayoutCoordinates.boundsInWindow(rootCoordinates: LayoutCoordinates?): Rect {
+    val root = rootCoordinates ?: findRootCoordinates()
 
     val rootWidth = root.size.width.toFloat()
     val rootHeight = root.size.height.toFloat()

sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt

@@ -27,6 +27,7 @@ import io.sentry.android.replay.util.toOpaque
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.GenericViewHierarchyNode
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.ImageViewHierarchyNode
 import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode.TextViewHierarchyNode
+import java.lang.ref.WeakReference
 
 @TargetApi(26)
 internal object ComposeViewHierarchyNode {
@@ -62,7 +63,7 @@ internal object ComposeViewHierarchyNode {
         return options.experimental.sessionReplay.maskViewClasses.contains(className)
     }
 
-    private var _rootCoordinates: LayoutCoordinates? = null
+    private var _rootCoordinates: WeakReference<LayoutCoordinates>? = null
 
     private fun fromComposeNode(
         node: LayoutNode,
@@ -77,11 +78,11 @@ internal object ComposeViewHierarchyNode {
         }
 
         if (isComposeRoot) {
-            _rootCoordinates = node.coordinates.findRootCoordinates()
+            _rootCoordinates = WeakReference(node.coordinates.findRootCoordinates())
         }
 
         val semantics = node.collapsedSemantics
-        val visibleRect = node.coordinates.boundsInWindow(_rootCoordinates)
+        val visibleRect = node.coordinates.boundsInWindow(_rootCoordinates?.get())
         val isVisible = !node.outerCoordinator.isTransparent() &&
             (semantics == null || !semantics.contains(SemanticsProperties.InvisibleToUser)) &&
             visibleRect.height() > 0 && visibleRect.width() > 0