Fix: ActivityLifecycleIntegration#appStartSpan memory leak

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
 

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"

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<Activity, ITransaction> 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

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<Activity>()
+        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<Activity>()
+        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()