Skip to content

Commit

Permalink
Surface ObjectAnimator leaks
Browse files Browse the repository at this point in the history
ObjectAnimator has an mTarget field that is a weak ref to the target. It's a weak ref as a misguided attempt to prevent leaks. Unfortunately, when an object animator runs forever, the mTarget weak ref will be checked every frame and the target reference held as a java local for a brief moment, which makes it really hard for the GC to collect the targeted object.

This changes fixes that by surfacing an additional low priority strong reference from ObjectAnimator to its target.

Additionally, this change adds object inspectors for animators that show useful state (such as listeners and running state).

```
┬───
│ GC Root: Thread object
│
├─ java.lang.Thread instance
│    Leaking: NO (the main thread always runs)
│    Thread name: 'main'
│    ↓ Thread.threadLocals
│             ~~~~~~~~~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap instance
│    Leaking: UNKNOWN
│    Retaining 1.4 kB in 43 objects
│    ↓ ThreadLocal$ThreadLocalMap.table
│                                 ~~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap$Entry[] array
│    Leaking: UNKNOWN
│    Retaining 1.3 kB in 42 objects
│    ↓ ThreadLocal$ThreadLocalMap$Entry[10]
│                                      ~~~~
├─ java.lang.ThreadLocal$ThreadLocalMap$Entry instance
│    Leaking: UNKNOWN
│    Retaining 28 B in 1 objects
│    ↓ ThreadLocal$ThreadLocalMap$Entry.value
│                                       ~~~~~
├─ android.animation.AnimationHandler instance
│    Leaking: UNKNOWN
│    Retaining 338.6 kB in 3421 objects
│    ↓ AnimationHandler.mAnimationCallbacks
│                       ~~~~~~~~~~~~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 338.5 kB in 3417 objects
│    ↓ ArrayList[0]
│               ~~~
├─ android.animation.ObjectAnimator instance
│    Leaking: UNKNOWN
│    Retaining 338.4 kB in 3415 objects
│    mListeners = null
│    mPropertyName = null
│    mProperty.mName = alpha
│    mProperty.mType = java.lang.Float
│    mInitialized = true
│    mStarted = true
│    mRunning = true
│    mAnimationEndRequested = false
│    mDuration = 100
│    mStartDelay = 0
│    mRepeatCount = INFINITE (-1)
│    mRepeatMode = REVERSE (2)
│    ↓ ObjectAnimator.mTarget
│                     ~~~~~~~
╰→ android.widget.Button instance
     Leaking: YES (View.mContext references a destroyed activity)
```
  • Loading branch information
pyricau committed Jan 3, 2022
1 parent b896dae commit 9e605fb
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package com.example.leakcanary

import android.animation.ObjectAnimator
import android.animation.ValueAnimator
import android.app.Activity
import android.app.AlertDialog
import android.content.BroadcastReceiver
Expand Down Expand Up @@ -93,6 +95,17 @@ class MainActivity : Activity() {
}
LeakyReschedulingRunnable(leaky).run()
}
findViewById<Button>(R.id.infinite_animator).setOnClickListener { view ->
ObjectAnimator.ofFloat(view, View.ALPHA, 0.1f, 0.2f).apply {
duration = 100
repeatMode = ValueAnimator.REVERSE
repeatCount = ValueAnimator.INFINITE
start()
}
}
findViewById<Button>(R.id.finish_activity).setOnClickListener { view ->
finish()
}
}

class NoOpBroadcastReceiver : BroadcastReceiver() {
Expand Down
14 changes: 14 additions & 0 deletions leakcanary-android-sample/src/main/res/layout/main_activity.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,19 @@
android:layout_height="wrap_content"
android:text="Create Message leak"
/>
<Button
android:id="@+id/infinite_animator"
android:layout_width="wrap_content"
android:layout_gravity="center"
android:layout_height="wrap_content"
android:text="Create animator leak"
/>
<Button
android:id="@+id/finish_activity"
android:layout_width="wrap_content"
android:layout_gravity="center"
android:layout_height="wrap_content"
android:text="Finish"
/>

</LinearLayout>
2 changes: 2 additions & 0 deletions shark-android/api/shark-android.api
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public final class shark/AndroidMetadataExtractor : shark/MetadataExtractor {
public abstract class shark/AndroidObjectInspectors : java/lang/Enum, shark/ObjectInspector {
public static final field ACTIVITY Lshark/AndroidObjectInspectors;
public static final field ANDROIDX_FRAGMENT Lshark/AndroidObjectInspectors;
public static final field ANIMATOR Lshark/AndroidObjectInspectors;
public static final field APPLICATION Lshark/AndroidObjectInspectors;
public static final field APPLICATION_PACKAGE_MANAGER Lshark/AndroidObjectInspectors;
public static final field COMPOSITION_IMPL Lshark/AndroidObjectInspectors;
Expand All @@ -35,6 +36,7 @@ public abstract class shark/AndroidObjectInspectors : java/lang/Enum, shark/Obje
public static final field MESSAGE_QUEUE Lshark/AndroidObjectInspectors;
public static final field MORTAR_PRESENTER Lshark/AndroidObjectInspectors;
public static final field MORTAR_SCOPE Lshark/AndroidObjectInspectors;
public static final field OBJECT_ANIMATOR Lshark/AndroidObjectInspectors;
public static final field RECOMPOSER Lshark/AndroidObjectInspectors;
public static final field SERVICE Lshark/AndroidObjectInspectors;
public static final field SUPPORT_FRAGMENT Lshark/AndroidObjectInspectors;
Expand Down
59 changes: 57 additions & 2 deletions shark-android/src/main/java/shark/AndroidObjectInspectors.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import shark.FilteringLeakingObjectFinder.LeakingObjectFilter
import shark.HeapObject.HeapInstance
import java.util.EnumSet
import kotlin.math.absoluteValue
import shark.internal.InternalSharkCollectionsHelper

/**
* A set of default [ObjectInspector]s that knows about common AOSP and library
Expand Down Expand Up @@ -656,7 +657,7 @@ enum class AndroidObjectInspectors : ObjectInspector {
val mTitleField = mWindowAttributes["android.view.WindowManager\$LayoutParams", "mTitle"]!!
labels += if (mTitleField.value.isNonNullReference) {
val mTitle =
mTitleField.valueAsInstance!!.readAsJavaString()!!
mTitleField.valueAsInstance!!.readAsJavaString()!!
"mWindowAttributes.mTitle = \"$mTitle\""
} else {
"mWindowAttributes.mTitle is null"
Expand Down Expand Up @@ -781,7 +782,61 @@ enum class AndroidObjectInspectors : ObjectInspector {
}
}
}
}
},

ANIMATOR {
override fun inspect(reporter: ObjectReporter) {
reporter.whenInstanceOf("android.animation.Animator") { instance ->
val mListeners = instance["android.animation.Animator", "mListeners"]!!.valueAsInstance
if (mListeners != null) {
val listenerValues = InternalSharkCollectionsHelper.arrayListValues(mListeners).toList()
if (listenerValues.isNotEmpty()) {
listenerValues.forEach { value ->
labels += "mListeners$value"
}
} else {
labels += "mListeners is empty"
}
} else {
labels += "mListeners = null"
}
}
}
},

OBJECT_ANIMATOR {
override fun inspect(reporter: ObjectReporter) {
reporter.whenInstanceOf("android.animation.ObjectAnimator") { instance ->
labels += "mPropertyName = " + (instance["android.animation.ObjectAnimator", "mPropertyName"]!!.valueAsInstance?.readAsJavaString()
?: "null")
val mProperty = instance["android.animation.ObjectAnimator", "mProperty"]!!.valueAsInstance
if (mProperty == null) {
labels += "mProperty = null"
} else {
labels += "mProperty.mName = " + (mProperty["android.util.Property", "mName"]!!.valueAsInstance?.readAsJavaString()
?: "null")
labels += "mProperty.mType = " + (mProperty["android.util.Property", "mType"]!!.valueAsClass?.name
?: "null")
}
labels += "mInitialized = " + instance["android.animation.ValueAnimator", "mInitialized"]!!.value.asBoolean!!
labels += "mStarted = " + instance["android.animation.ValueAnimator", "mStarted"]!!.value.asBoolean!!
labels += "mRunning = " + instance["android.animation.ValueAnimator", "mRunning"]!!.value.asBoolean!!
labels += "mAnimationEndRequested = " + instance["android.animation.ValueAnimator", "mAnimationEndRequested"]!!.value.asBoolean!!
labels += "mDuration = " + instance["android.animation.ValueAnimator", "mDuration"]!!.value.asLong!!
labels += "mStartDelay = " + instance["android.animation.ValueAnimator", "mStartDelay"]!!.value.asLong!!
val repeatCount = instance["android.animation.ValueAnimator", "mRepeatCount"]!!.value.asInt!!
labels += "mRepeatCount = " + if (repeatCount == -1) "INFINITE (-1)" else repeatCount

val repeatModeConstant = when (val repeatMode =
instance["android.animation.ValueAnimator", "mRepeatMode"]!!.value.asInt!!) {
1 -> "RESTART (1)"
2 -> "REVERSE (2)"
else -> "Unknown ($repeatMode)"
}
labels += "mRepeatMode = $repeatModeConstant"
}
}
},
;

internal open val leakingObjectFilter: ((heapObject: HeapObject) -> Boolean)? = null
Expand Down
5 changes: 5 additions & 0 deletions shark/api/shark.api
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,11 @@ public final class shark/ReferencePattern$StaticFieldPattern : shark/ReferencePa
public final class shark/ReferencePattern$StaticFieldPattern$Companion {
}

public final class shark/internal/InternalSharkCollectionsHelper {
public static final field INSTANCE Lshark/internal/InternalSharkCollectionsHelper;
public final fun arrayListValues (Lshark/HeapObject$HeapInstance;)Lkotlin/sequences/Sequence;
}

public final class shark/internal/ObjectDominators {
public fun <init> ()V
public final fun renderDominatorTree (Lshark/HeapGraph;Ljava/util/List;ILjava/lang/String;Z)Ljava/lang/String;
Expand Down
49 changes: 49 additions & 0 deletions shark/src/main/java/shark/internal/AndroidReferenceReaders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package shark.internal

import shark.HeapGraph
import shark.HeapObject.HeapInstance
import shark.ValueHolder.ReferenceHolder
import shark.internal.ReferenceLocationType.ARRAY_ENTRY
import shark.internal.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader
import shark.internal.ChainingInstanceReferenceReader.VirtualInstanceReferenceReader.OptionalFactory
import shark.internal.Reference.LazyDetails
import shark.internal.ReferenceLocationType.INSTANCE_FIELD

internal enum class AndroidReferenceReaders : OptionalFactory {

Expand Down Expand Up @@ -44,5 +46,52 @@ internal enum class AndroidReferenceReaders : OptionalFactory {
}
}
}
},

ANIMATOR_WEAK_REF_SUCKS {
override fun create(graph: HeapGraph): VirtualInstanceReferenceReader? {
val objectAnimatorClass =
graph.findClassByName("android.animation.ObjectAnimator") ?: return null

val weakRefClassId =
graph.findClassByName("java.lang.ref.WeakReference")?.objectId ?: return null

val objectAnimatorClassId = objectAnimatorClass.objectId

return object : VirtualInstanceReferenceReader {
override fun matches(instance: HeapInstance) =
instance.instanceClassId == objectAnimatorClassId

override fun read(source: HeapInstance): Sequence<Reference> {
val mTarget = source["android.animation.ObjectAnimator", "mTarget"]?.valueAsInstance
?: return emptySequence()

if (mTarget.instanceClassId != weakRefClassId) {
return emptySequence()
}

val actualRef =
mTarget["java.lang.ref.Reference", "referent"]!!.value.holder as ReferenceHolder

return if (actualRef.isNull) {
emptySequence()
} else {
sequenceOf(Reference(
valueObjectId = actualRef.value,
isLowPriority = true,
lazyDetailsResolver = {
LazyDetails(
name = "mTarget",
locationClassObjectId = objectAnimatorClassId,
locationType = INSTANCE_FIELD,
matchedLibraryLeak = null,
isVirtual = true
)
}
))
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package shark.internal

import shark.HeapObject
import shark.HeapObject.HeapClass
import shark.HeapObject.HeapInstance
import shark.HeapObject.HeapObjectArray
import shark.HeapObject.HeapPrimitiveArray

/**
* INTERNAL
*
* This class is public to be accessible from other LeakCanary modules but shouldn't be
* called directly, the API may break at any point.
*/
object InternalSharkCollectionsHelper {

fun arrayListValues(heapInstance: HeapInstance): Sequence<String> {
val graph = heapInstance.graph
val arrayListReader = OpenJdkInstanceRefReaders.ARRAY_LIST.create(graph)
?: ApacheHarmonyInstanceRefReaders.ARRAY_LIST.create(graph)
?: return emptySequence()

if (!arrayListReader.matches(heapInstance)) {
return emptySequence()
}

return arrayListReader.read(heapInstance).map { reference ->
val arrayListValue = graph.findObjectById(reference.valueObjectId)
val details = reference.lazyDetailsResolver.resolve()
"[${details.name}] = ${className(arrayListValue)}"
}
}

private fun className(graphObject: HeapObject): String {
return when (graphObject) {
is HeapClass -> {
graphObject.name
}
is HeapInstance -> {
graphObject.instanceClassName
}
is HeapObjectArray -> {
graphObject.arrayClassName
}
is HeapPrimitiveArray -> {
graphObject.arrayClassName
}
}
}
}

0 comments on commit 9e605fb

Please sign in to comment.