diff --git a/.github/workflows/agp-matrix.yml b/.github/workflows/agp-matrix.yml index 5c7672d3294..a0a7e2405c9 100644 --- a/.github/workflows/agp-matrix.yml +++ b/.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() diff --git a/CHANGELOG.md b/CHANGELOG.md index 7190b1d6ef0..2d1a18877fd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 2715dd57671..bf5dba03be5 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/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" diff --git a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts b/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts index 7755166a4e2..02609ba7875 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/build.gradle.kts +++ b/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) } diff --git a/sentry-android-integration-tests/sentry-uitest-android/proguard-rules.pro b/sentry-android-integration-tests/sentry-uitest-android/proguard-rules.pro index 02f5e80ba30..5de2dac4bdb 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/proguard-rules.pro +++ b/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.** { *; } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/ReplayTest.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/ReplayTest.kt new file mode 100644 index 00000000000..ffea8f2e048 --- /dev/null +++ b/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() + 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() + } +} diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt index 56083717221..38c2d583b4e 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/util/Nodes.kt +++ b/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() diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt index 5640fbc96f2..652e8cd0406 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/viewhierarchy/ComposeViewHierarchyNode.kt +++ b/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? = 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