diff --git a/CHANGELOG.md b/CHANGELOG.md index bc5e4a89d4..4538683409 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Feat: Allow setting proguard via Options and/or external resources (#1728) * Feat: Add breadcrumbs for the Apollo integration (#1726) * Fix: Don't set lastEventId for transactions (#1727) +* Fix: ActivityLifecycleIntegration#appStartSpan memory leak (#1732) ## 5.2.0-beta.3 diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index 44509d5451..e0a6715c6e 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -45,7 +45,7 @@ object Config { val okhttp = "com.squareup.okhttp3:okhttp" // only bump gson if https://github.com/google/gson/issues/1597 is fixed val gson = "com.google.code.gson:gson:2.8.5" - val leakCanary = "com.squareup.leakcanary:leakcanary-android:2.6" + val leakCanary = "com.squareup.leakcanary:leakcanary-android:2.7" private val lifecycleVersion = "2.2.0" val lifecycleProcess = "androidx.lifecycle:lifecycle-process:$lifecycleVersion" diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 2723342cdb..e870344db3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -291,10 +291,19 @@ public synchronized void onActivitySaveInstanceState( public synchronized void onActivityDestroyed(final @NonNull Activity activity) { addBreadcrumb(activity, "destroyed"); + // in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid + // memory leak + if (appStartSpan != null && !appStartSpan.isFinished()) { + appStartSpan.finish(SpanStatus.CANCELLED); + } + // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, // we make sure to finish it when the activity gets destroyed. stopTracing(activity, true); + // set it to null in case its been just finished as cancelled + appStartSpan = null; + // clear it up, so we don't start again for the same activity if the activity is in the activity // stack still. // if the activity is opened again and not in memory, transactions will be created normally. @@ -309,6 +318,12 @@ WeakHashMap getActivitiesWithOngoingTransactions() { return activitiesWithOngoingTransactions; } + @TestOnly + @Nullable + ISpan getAppStartSpan() { + return appStartSpan; + } + private void setColdStart(final @Nullable Bundle savedInstanceState) { if (!firstActivityCreated && performanceEnabled) { // if Activity has savedInstanceState then its a warm start diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index f612d5a20f..01751f1b8e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -26,6 +26,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -429,6 +430,38 @@ class ActivityLifecycleIntegrationTest { assertTrue(sut.activitiesWithOngoingTransactions.isEmpty()) } + @Test + fun `When Activity is destroyed, sets appStartSpan status to cancelled and finish it`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + setAppStartTime() + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) + + val span = fixture.transaction.children.first() + assertEquals(span.status, SpanStatus.CANCELLED) + assertTrue(span.isFinished) + } + + @Test + fun `When Activity is destroyed, sets appStartSpan to null`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + setAppStartTime() + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) + + assertNull(sut.appStartSpan) + } + @Test fun `When new Activity and transaction is created, finish previous ones`() { val sut = fixture.getSut()