Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Fix leaks in BaseBindingFragment #28

Merged
merged 3 commits into from
Feb 10, 2022
Merged

Fix leaks in BaseBindingFragment #28

merged 3 commits into from
Feb 10, 2022

Conversation

Goooler
Copy link
Owner

@Goooler Goooler commented Feb 9, 2022

Activity → addFragment(AFragment())replaceFragment(BFragment(), isAddToBackStack = true)bFragment.onDestroyView() → leaked

Comment on lines +36 to +39
override fun onDestroyView() {
_binding = null
super.onDestroyView()
}
Copy link
Owner Author

@Goooler Goooler Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Release binding's reference when next fragment was attached to parent with replaceFragment(isAddToBackStack = true)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous fragment was called onDestroyView() but not onDestroy(), LeakCanary analyzes the view held by fragment should be released.

@Goooler
Copy link
Owner Author

Goooler commented Feb 9, 2022

┬───
│ GC Root: Thread object
│
├─ Bk instance
│    Leaking: NO (PathClassLoader↓ is not leaking)
│    Thread name: 'CleanupReference'
│    ↓ Thread.contextClassLoader
├─ dalvik.system.PathClassLoader instance
│    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never
│    leaking)
│    ↓ ClassLoader.runtimeInternalObjects
├─ java.lang.Object[] array
│    Leaking: NO (InternalLeakCanary↓ is not leaking)
│    ↓ Object[896]
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (WebActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ io.goooler.demoapp.web.WebActivity instance
│    Leaking: NO (BlankFragment↓ is not leaking and Activity#mDestroyed is
│    false)
│    mApplication instance of io.goooler.demoapp.DemoApplication
│    mBase instance of androidx.appcompat.view.ContextThemeWrapper
│    ↓ ComponentActivity.mActivityResultRegistry
├─ androidx.activity.ComponentActivity$2 instance
│    Leaking: NO (BlankFragment↓ is not leaking)
│    Anonymous subclass of androidx.activity.result.ActivityResultRegistry
│    this$0 instance of io.goooler.demoapp.web.WebActivity with mDestroyed =
│    false
│    ↓ ActivityResultRegistry.mKeyToCallback
├─ java.util.HashMap instance
│    Leaking: NO (BlankFragment↓ is not leaking)
│    ↓
│    HashMap["FragmentManager:ad37cf17-b2a0-4eeb-979a-4915d3565c96:StartIntentSe
│    nderForResult"]
├─ androidx.activity.result.ActivityResultRegistry$CallbackAndContract instance
│    Leaking: NO (BlankFragment↓ is not leaking)
│    ↓ ActivityResultRegistry$CallbackAndContract.mCallback
├─ androidx.fragment.app.FragmentManager$8 instance
│    Leaking: NO (BlankFragment↓ is not leaking)
│    Anonymous class implementing androidx.activity.result.
│    ActivityResultCallback
│    ↓ FragmentManager$8.this$0
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: NO (BlankFragment↓ is not leaking)
│    ↓ FragmentManager.mParent
├─ io.goooler.demoapp.web.BlankFragment instance
│    Leaking: NO (Fragment#mFragmentManager is not null)
│    ↓ BlankFragment.binding
│                    ~~~~~~~
├─ io.goooler.demoapp.web.databinding.BlankFragmentBindingImpl instance
│    Leaking: UNKNOWN
│    Retaining 13.3 kB in 135 objects
│    ↓ ViewDataBinding.mRoot
│                      ~~~~~
╰→ androidx.constraintlayout.widget.ConstraintLayout instance
     Leaking: YES (ObjectWatcher was watching this because io.goooler.demoapp.
     web.BlankFragment received Fragment#onDestroyView() callback (references
     to its views should be cleared to prevent leaks))
     Retaining 13.2 kB in 131 objects
     key = 1ffe3906-50c6-474a-98f0-e0aad80f61de
     watchDurationMillis = 10351
     retainedDurationMillis = 5351
     View not part of a window view hierarchy
     View.mAttachInfo is null (view detached)
     View.mWindowAttachCount = 1
     mContext instance of io.goooler.demoapp.web.WebActivity with mDestroyed =
     false

METADATA

Build.VERSION.SDK_INT: 30
Build.MANUFACTURER: OnePlus
LeakCanary version: 2.8.1
App process name: io.goooler.demoapp
Stats: LruCache[maxSize=3000,hits=43896,misses=112800,hitRate=28%]
RandomAccess[bytes=13775117,reads=112800,travel=56667537741,range=33309943,size=
39913741]
Analysis duration: 3553 ms

@Goooler Goooler added this to the 1.5.0 milestone Feb 9, 2022
@Goooler Goooler added the bug Something isn't working label Feb 9, 2022
Comment on lines 1 to 15
package io.goooler.demoapp.web

import android.os.Bundle
import android.view.View
import android.widget.TextView
import androidx.fragment.app.Fragment

class BlankFragment : Fragment(R.layout.web_blank_fragment) {
lateinit var tvContent: TextView

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
tvContent = view.findViewById(R.id.tv_content)
}
}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Goooler Goooler merged commit b0842f1 into kotlin Feb 10, 2022
@Goooler Goooler deleted the fix_binding_leak branch February 10, 2022 04:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant