Memory Leak : AggregatingVitalMonitor listeners are never unregistered

[joshskeen]

Describe the bug

in RumViewScope.init, https://github.com/DataDog/dd-sdk-android/blob/cff8b54d458f7cbf6f935deb7ff9b18aec23d9e3/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt#L145

cpuVitalMonitor.register(cpuVitalListener)
memoryVitalMonitor.register(memoryVitalListener)
frameRateVitalMonitor.register(frameRateVitalListener)

is called, but the corresponding AggregatingVitalMonitor.unregister calls never happen. The session-scoped AggregatingVitalMonitors hold a map of listeners, and the https://github.com/DataDog/dd-sdk-android/blob/cff8b54d458f7cbf6f935deb7ff9b18aec23d9e3/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/AggregatingVitalMonitor.kt#L16 map continues to grow, since listeners are never unregistered when the view is stopped, causing a memory leak.

Reproduction steps

• break on VitalMonitor.register and VitalMonitor.unregister while starting / stopping views. observe the map of AggregatingVitalMonitor.listeners continues to grow.

Logcat logs

No response

Expected behavior

Vital monitor listeners are unregistered from the listeners map when a view is stopped.

Affected SDK versions

2.8.0

Latest working SDK version

NA

Did you confirm if the latest SDK version fixes the bug?

Yes

Kotlin / Java version

No response

Gradle / AGP version

No response

Other dependencies versions

No response

Device Information

No response

Other relevant information

No response

[xgouchet]

Thanks a lot @joshskeen for raising this issue, we'll deal with it as soon as possible

[pyricau]

A little additional context: we originally found this by leveraging a new LeakCanary feature (WIP) that helps detect objects that are repeatedly growing. The tool surfaced the below trace and then @joshskeen & team investigated and found the exact issue.

┬───
│ GcRoot(StickyClass) (9500 objects)
│
├─STATIC_FIELD FontsContract.sContext -> instance of com.squareup.development.PosDevApp (1 objects)
│
├─INSTANCE_FIELD Application.mActivityLifecycleCallbacks -> instance of java.util.ArrayList (1 objects)
│
├─ARRAY_ENTRY ArrayList.[x] -> instance of com.datadog.android.rum.internal.vitals.JankStatsActivityLifecycleListener (1 objects)
│
├─INSTANCE_FIELD JankStatsActivityLifecycleListener.vitalObserver -> instance of com.datadog.android.rum.internal.vitals.AggregatingVitalMonitor (1 objects)
│
╰→INSTANCE_FIELD AggregatingVitalMonitor.listeners -> instance of java.util.LinkedHashMap (1 objects)
    Children:
    118 objects (8 new): ARRAY_ENTRY LinkedHashMap.[x] -> instance of com.datadog.android.rum.internal.domain.scope.RumViewScope$frameRateVitalListener$1
    118 objects (8 new): ARRAY_ENTRY LinkedHashMap.[x] -> instance of com.datadog.android.rum.internal.vitals.VitalInfo

[xgouchet]

Thanks a lot for the hint @pyricau , that's really useful indeed.