[Library Leak] Navigation component 2.7.1

[yoobi]

Hello,

I think this leak is a false positive from navigation component. I'm using

androidx.navigation:navigation-ui-ktx:2.7.1
androidx.navigation:navigation-fragment-ktx:2.7.1

This leak appears when navigation back and forth between 2 fragment using a toolbar linked to Navigation Component via NavigationUI.setupWithNavController(toolbar, navController, configuration).

When checking out the code, AbstractAppBarOnDestinationChangedListener is keeping a strong reference over the context which is causing the leak

internal class ToolbarOnDestinationChangedListener(
    toolbar: Toolbar,
    configuration: AppBarConfiguration
) : AbstractAppBarOnDestinationChangedListener(toolbar.context, configuration) {
    private val toolbarWeakReference: WeakReference<Toolbar> = WeakReference(toolbar)
    ....
}

LeakTrace information

====================================
HEAP ANALYSIS RESULT
====================================
1 APPLICATION LEAKS

References underlined with "~~~" are likely causes.
Learn more at https://squ.re/leaks.

100039 bytes retained by leaking objects
Signature: cdab6e606e7c9c30d4c25cd9fea0f69809a85e25
┬───
│ GC Root: System class
│
├─ android.view.inputmethod.InputMethodManager class
│    Leaking: NO (InputMethodManager↓ is not leaking and a class is never leaking)
│    ↓ static InputMethodManager.sInstance
├─ android.view.inputmethod.InputMethodManager instance
│    Leaking: NO (VideoView↓ is not leaking and InputMethodManager is a singleton)
│    ↓ InputMethodManager.mCurRootView
├─ android.view.ViewRootImpl instance
│    Leaking: NO (VideoView↓ is not leaking)
│    mContext instance of com.android.internal.policy.DecorContext, wrapping activity com.example.
│    MainActivity with mDestroyed = false
│    ViewRootImpl#mView is not null
│    mWindowAttributes.mTitle = "com.example.beta.debug/com.example.MainActivity"
│    mWindowAttributes.type = 1
│    ↓ ViewRootImpl.mImeFocusController
├─ android.view.ImeFocusController instance
│    Leaking: NO (VideoView↓ is not leaking)
│    ↓ ImeFocusController.mNextServedView
├─ android.widget.VideoView instance
│    Leaking: NO (HomeFragment↓ is not leaking and View attached)
│    View is part of a window view hierarchy
│    View.mAttachInfo is not null (view attached)
│    View.mID = R.id.item_ad_video
│    View.mWindowAttachCount = 1
│    mContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping
│    activity com.example.MainActivity with mDestroyed = false
│    ↓ View.mContext
├─ dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper instance
│    Leaking: NO (HomeFragment↓ is not leaking)
│    mBase instance of com.example.MainActivity with mDestroyed = false
│    ViewComponentManager$FragmentContextWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ViewComponentManager$FragmentContextWrapper.fragment
├─ com.example.feature.home.ui.HomeFragment instance
│    Leaking: NO (Fragment#mFragmentManager is not null)
│    componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
│    wrapping activity com.example.MainActivity with mDestroyed = false
│    Fragment.mTag=2eaf9efb-84ea-4c71-84fd-75fed621c445
│    ↓ HomeFragment.homeNav
│                   ~~~~~~~
├─ com.example.di.impl.navigation.HomeNavigationImpl instance
│    Leaking: UNKNOWN
│    Retaining 12 B in 1 objects
│    ↓ HomeNavigationImpl.navController
│                         ~~~~~~~~~~~~~
├─ androidx.navigation.NavHostController instance
│    Leaking: UNKNOWN
│    Retaining 105,8 kB in 2461 objects
│    activity instance of com.example.MainActivity with mDestroyed = false
│    context instance of com.example.MainActivity with mDestroyed = false
│    ↓ NavController.onDestinationChangedListeners
│                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 102,2 kB in 2348 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ androidx.navigation.ui.ToolbarOnDestinationChangedListener instance
│    Leaking: UNKNOWN
│    Retaining 102,1 kB in 2343 objects
│    context instance of android.view.ContextThemeWrapper, wrapping activity com.example.MainActivity with
│    mDestroyed = false
│    ↓ AbstractAppBarOnDestinationChangedListener.context
│                                                 ~~~~~~~
├─ android.view.ContextThemeWrapper instance
│    Leaking: UNKNOWN
│    Retaining 101,8 kB in 2337 objects
│    mBase instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping
│    activity com.example.MainActivity with mDestroyed = false
│    ContextThemeWrapper wraps an Activity with Activity.mDestroyed false
│    ↓ ContextThemeWrapper.mInflater
│                          ~~~~~~~~~
├─ com.android.internal.policy.PhoneLayoutInflater instance
│    Leaking: UNKNOWN
│    Retaining 101,7 kB in 2328 objects
│    mContext instance of android.view.ContextThemeWrapper, wrapping activity com.example.MainActivity
│    with mDestroyed = false
│    mPrivateFactory instance of com.example.MainActivity with mDestroyed = false
│    ↓ LayoutInflater.mFactory
│                     ~~~~~~~~
├─ android.view.LayoutInflater$FactoryMerger instance
│    Leaking: UNKNOWN
│    Retaining 101,6 kB in 2326 objects
│    ↓ LayoutInflater$FactoryMerger.mF1
│                                   ~~~
├─ androidx.fragment.app.FragmentLayoutInflaterFactory instance
│    Leaking: UNKNOWN
│    Retaining 101,6 kB in 2325 objects
│    ↓ FragmentLayoutInflaterFactory.mFragmentManager
│                                    ~~~~~~~~~~~~~~~~
├─ androidx.fragment.app.FragmentManagerImpl instance
│    Leaking: UNKNOWN
│    Retaining 101,6 kB in 2324 objects
│    ↓ FragmentManager.mOnAttachListeners
│                      ~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 100,1 kB in 2272 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ androidx.fragment.app.FragmentManager$7 instance
│    Leaking: UNKNOWN
│    Retaining 100,1 kB in 2269 objects
│    Anonymous class implementing androidx.fragment.app.FragmentOnAttachListener
│    ↓ FragmentManager$7.val$parent
│                        ~~~~~~~~~~
╰→ com.example.feature.user.details.ui.UserDetails instance
​     Leaking: YES (ObjectWatcher was watching this because com.example.feature.user.details.ui.
​     UserDetails received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
​     Retaining 100,0 kB in 2268 objects
​     key = b780aa68-dfa3-4060-b39d-4e0ef44aca9c
​     watchDurationMillis = 7671
​     retainedDurationMillis = 2671
​     componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
​     wrapping activity com.example.MainActivity with mDestroyed = false
====================================
0 LIBRARY LEAKS

A Library Leak is a leak caused by a known bug in 3rd party code that you do not have control over.
See https://square.github.io/leakcanary/fundamentals-how-leakcanary-works/#4-categorizing-leaks
====================================
0 UNREACHABLE OBJECTS

An unreachable object is still in memory but LeakCanary could not find a strong reference path
from GC roots.
====================================
METADATA

Please include this in bug reports and Stack Overflow questions.

Build.VERSION.SDK_INT: 32
Build.MANUFACTURER: Google
LeakCanary version: 2.12
App process name: com.example.beta.debug
Class count: 26447
Instance count: 211931
Primitive array count: 140674
Object array count: 30156
Thread count: 54
Heap total bytes: 28761351
Bitmap count: 8
Bitmap total bytes: 17848328
Large bitmap count: 0
Large bitmap total bytes: 0
Db 1: open /data/user/0/com.example.beta.debug/databases/com.google.android.datatransport.events
Db 2: closed /data/user/0/com.example.beta.debug/databases/google_app_measurement_local.db
Count of retained yet cleared: 1 KeyedWeakReference instances
Stats: LruCache[maxSize=3000,hits=115612,misses=211827,hitRate=35%]
RandomAccess[bytes=10575589,reads=211827,travel=91794764022,range=33578308,size=42323797]
Analysis duration: 6068 ms
Heap dump file path: /storage/emulated/0/Download/leakcanary-com.example.beta.
debug/2023-09-06_08-50-39_058.hprof
Heap dump timestamp: 1693983048437
Heap dump duration: Unknown
====================================

[pyricau]

I think this leak is a false positive

It's not a false positive, this is a real leak: UserDetails here is a parent fragment that was destroyed, UserDetails is leaking.

It's hard to figure out the details just from this leak trace, a heap dump or a working repro example project would help.

I highly suspect that FragmentManagerImpl is destroyed as well

Separate note: we should read & surface FragmentManagerImpl.mDestroyed in AndroidObjectInspectors.

I have to guess a little, but it looks like this is a real leak in your application code, it's one of two things:

• Either the NavHostController was created for the UserDetails fragment or one of its children. If that's the case, then com.example.feature.home.ui.HomeFragment should not keep a reference to homeNav after UserDetails is destroyed
• Or the code that is setting up the androidx.navigation.ui.ToolbarOnDestinationChangedListener as a destination listener is forgetting to remove it when UserDetails is destroyed.

[yoobi]

I'll look into my samples project if I still got the one I used for this issue

[yoobi]

Hello ! Found the project: https://github.com/yoobi/CleanArch you can navigate from the "Home" to "Search" and the leak will appear

[yoobi]

Hello, any news on this ?

I have to guess a little, but it looks like this is a real leak in your application code, it's one of two things:

• Either the NavHostController was created for the UserDetails fragment or one of its children. If that's the case, then com.example.feature.home.ui.HomeFragment should not keep a reference to homeNav after UserDetails is destroyed
• Or the code that is setting up the androidx.navigation.ui.ToolbarOnDestinationChangedListener as a destination listener is forgetting to remove it when UserDetails is destroyed.

The NavHostController is created in the Activity so it's fine for it to have detailed of the children (I guess?)
The code setting androidx.navigation.ui.ToolbarOnDestinationChangedListener is directly coming from navigation library which is why I though it was a library leak

[pyricau]

If you think this is an issue with jetpack navigation, you should create a small project that reproduces the issue then file a bug to jetpack.

[yoobi]

I've found this issue from hilt: google/dagger#2070 and saying they can't solve it as the API to solve it aren't public.