razir / ProgressButton

Android Progress Button
856 stars 77 forks source link

Memory leak #19

Closed GuilhE closed 4 years ago

GuilhE commented 4 years ago

Output from LeakCanary:

┬─── │ GC Root: Input or output parameters in native code │ ├─ okio.AsyncTimeout class │ Leaking: NO (PathClassLoader↓ is not leaking and a class is never leaking) │ ↓ static AsyncTimeout.$class$classLoader ├─ dalvik.system.PathClassLoader instance │ Leaking: NO (ProgressButtonHolderKt↓ is not leaking and A ClassLoader is never leaking) │ ↓ PathClassLoader.runtimeInternalObjects ├─ java.lang.Object[] array │ Leaking: NO (ProgressButtonHolderKt↓ is not leaking) │ ↓ Object[].[454] ├─ com.github.razir.progressbutton.ProgressButtonHolderKt class │ Leaking: NO (a class is never leaking) │ ↓ static ProgressButtonHolderKt.activeViews │ ~~~ ├─ java.util.WeakHashMap instance │ Leaking: UNKNOWN │ ↓ WeakHashMap.table │ ~ ├─ java.util.WeakHashMap$Entry[] array │ Leaking: UNKNOWN │ ↓ WeakHashMap$Entry[].[1] │ ~~~ ├─ java.util.WeakHashMap$Entry instance │ Leaking: UNKNOWN │ ↓ WeakHashMap$Entry.value │ ~ ├─ com.github.razir.progressbutton.DrawableViewData instance │ Leaking: UNKNOWN │ ↓ DrawableViewData.callback │ ~~~~ ├─ com.github.razir.progressbutton.DrawableButtonExtensionsKt$setupDrawableCallback$callback$1 instance │ Leaking: UNKNOWN │ Anonymous class implementing android.graphics.drawable.Drawable$Callback │ ↓ DrawableButtonExtensionsKt$setupDrawableCallback$callback$1.$textView │ ~~~~~ ├─ com.google.android.material.button.MaterialButton instance │ Leaking: YES (View.mContext references a destroyed activity) │ mContext instance of ...MyActivity with mDestroyed = true │ View#mParent is set │ View#mAttachInfo is null (view detached) │ View.mID = R.id.submitMaterialButton │ View.mWindowAttachCount = 1 │ ↓ MaterialButton.mContext ╰→ ...MyActivity instance ​ Leaking: YES (ObjectWatcher was watching this because ...MyActivity received Activity#onDestroy() callback and Activity#mDestroyed is true) ​ key = 5a2c63ce-96e2-49a1-b85e-8ac67750641c ​ watchDurationMillis = 5175 ​ retainedDurationMillis = 172

METADATA

Build.VERSION.SDK_INT: 29 Build.MANUFACTURER: Google LeakCanary version: 2.2 App process name: ... Analysis duration: 12134 ms

Any clue?

IlyaPavlovskii commented 4 years ago

Same problem. @GuilhE Is annotation processing enabled? I think the better solution is to migrate to upgraded https://developer.android.com/reference/android/arch/lifecycle/DefaultLifecycleObserver

GuilhE commented 4 years ago

I've tested and this function is being called:

private class ProgressButtonHolder(private val textView: WeakReference<TextView>) :
    LifecycleObserver {

    @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
    fun onDestroy() {
        textView.get()?.let {
            it.cancelAnimations()
            it.cleanUpDrawable()
            it.removeTextAnimationAttachViewListener()
            it.removeDrawableAttachViewListener()
            attachedViews.remove(it)
        }
    }
}
GuilhE commented 4 years ago

The only way I'm getting rid of this leaks is by making sure I'm calling:

bindProgressButton(binding.submitMaterialButton)

and

override fun onDestroy() {
    binding.submitMaterialButton.cleanUpDrawable()
    super.onDestroy()
}

@razir any thoughts on this?

GuilhE commented 4 years ago

I've found the problem and it was mine, really hard to catch involving removeObservers(), etc. But I think your suggestion is good and I'll create a PR regarding Google doc:

If you use Java 8 language, always prefer it over annotations.