FragmentStatePagerAdapter memory leak (nested fragments with viewpager)

Question

I have a "memory leak" in my adapter (quotes will be explained later). I'm currently using nested fragments to host a viewpager.

My setup is as following:
1. Activity (Empty activity that hosts Fragment A)
2. Fragment A - fragment that hosts viewpager with Fragmentstatepageradapter. Each viewpager page hosts fragment B.
3. Fragment B - a fragment that contains an imageview.

Everything is working great except when a configuration change occurs. Monitoring the heap, it seems to grow by 100 kb everytime a rotation occurs. Manually GCing does not release the memory.

Things I've tried:
1. Replace Fragment B with a blank fragment - same issue occurs so it's not the imageview that's causing the issue.
2. Remove both fragment A and B and rotate the activity. No memory leak occurs so it's not the activity.
3. Used MAT before any orientation changes and after rotating about 50 times to get the heap up. MAT shows 1 major suspect which is my adapter class. It shows 7MB of retained heap (very small shallow heap) of observers like so:

array java.util.ArrayList @ 0x42079938 24 7,000,832 
.\mObservers android.database.DataSetObservable @ 0x42053508 16 7,000,848 
..\mObservable com.example.main.Adapter@ 0x4205a048 40 7,001,416 

Why I'm using a viewpager inside a fragment:
1. I want to keep the state of the adapter and other variables related to the viewpager alive by setting setretaininstance(true).
2. After a config change, I don't recreate the adapter but use the old one to attach to the viewpager.
3. If I don't reuse the old adapter but create a new adapter after a configuration change, the memory leak dissapears.
4. the memory leak also dissapears after i close the activity and return to the previous activity.

Any ideas? Would appreciate any help.

Thanks, JC

Breathtaking

Answer 1

I had a similar memory leak which is now resolved.

In my corresponding Fragment A, I was instantiating the FragmentStatePagerAdapter with this.getFragmentManager() instead of this.getChildFragmentManager() since nested fragments are in place.

Please let me know if this also fixes your issue.

Vitebsk

Thanks man! I would have never figured it out myself (even though it's logical). – Demicanton

Should be marked as the the correct awnser. Thanks! – Koto

can you please tell me where to put getChildFragmentManager() i'm doing the following : MainActivity: fragmentAdapter = new FragmentAdapter(getSupportFragmentManager()); and in Fragment Adapter: public FragmentAdapter(FragmentManager fm) { super(fm); } – Cohleen

Please try the following in your MainActivity: fragmentAdapter = new FragmentAdapter(getChildFragmentManager()); – Vitebsk

Outstanding fix, Thanks alot. – Quadrisect

dude you saved my life. id give you a +2 if i could. – Kristine

so i should always use childfragmentmanager in nested fragment. didnt know this. – Kristine

Thanks you! Saved the day! – Hawsepipe

Answer 2

I had a similar issue, I used ViewPager2, and was in need to use getChildFragmentManager() instead of getSupportFragmentManager() because I want the PageFragment to refer back to the parent fragment (which hosts the ViewPager) with requireParentFragment()

N.B. When I used getSupportFragmentManager() then I got java.lang.IllegalStateException Fragment PageFragment{f152edf} (af30cf2b-acf1-4930-9b83-03ac8144cfc6) f49} is not a child Fragment

The other reason I used getChildFragmentManager(), because I'm using Navigation components, so I don't need to manage the the ViewPager child fragments with the same fragment manager of the navigation components which is getSupportFragmentManager().

Here's the LeakCanary log

2020-09-15 05:39:33.461 9611-9689/.... D/LeakCanary: ┬───
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │ GC Root: Local variable in native code
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ android.os.HandlerThread instance
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: NO (PathClassLoader↓ is not leaking)
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Thread name: 'LeakCanary-Heap-Dump'
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ HandlerThread.contextClassLoader
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ dalvik.system.PathClassLoader instance
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never leaking)
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ PathClassLoader.runtimeInternalObjects
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ java.lang.Object[] array
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: NO (InternalLeakCanary↓ is not leaking)
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ Object[].[597]
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ leakcanary.internal.InternalLeakCanary class
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ static InternalLeakCanary.resumedActivity
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ .....ui.MainActivity instance
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: NO (Activity#mDestroyed is false)
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ MainActivity.mLifecycleRegistry
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │                   ~~~~~~~~~~~~~~~~~~
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ androidx.lifecycle.LifecycleRegistry instance
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    ↓ LifecycleRegistry.mObserverMap
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │                        ~~~~~~~~~~~~
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: ├─ androidx.arch.core.internal.FastSafeIterableMap instance
2020-09-15 05:39:33.462 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ FastSafeIterableMap.mHashMap
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │                          ~~~~~~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ java.util.HashMap instance
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ HashMap.table
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │              ~~~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ java.util.HashMap$Node[] array
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ HashMap$Node[].[5]
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │                     ~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ java.util.HashMap$Node instance
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ HashMap$Node.key
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │                   ~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ androidx.viewpager2.adapter.FragmentStateAdapter$FragmentMaxLifecycleEnforcer$3 instance
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Anonymous class implementing androidx.lifecycle.LifecycleEventObserver
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ FragmentStateAdapter$FragmentMaxLifecycleEnforcer$3.this$1
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │                                                          ~~~~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ androidx.viewpager2.adapter.FragmentStateAdapter$FragmentMaxLifecycleEnforcer instance
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    Leaking: UNKNOWN
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │    ↓ FragmentStateAdapter$FragmentMaxLifecycleEnforcer.mViewPager
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: │                                                        ~~~~~~~~~~
2020-09-15 05:39:33.463 9611-9689/.... D/LeakCanary: ├─ androidx.viewpager2.widget.ViewPager2 instance
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    Leaking: YES (View detached and has parent)
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    mContext instance of .....ui.MainActivity with mDestroyed = false
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View#mParent is set
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View#mAttachInfo is null (view detached)
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View.mID = R.id.viewpager
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View.mWindowAttachCount = 1
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    ↓ ViewPager2.mParent
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: ├─ androidx.coordinatorlayout.widget.CoordinatorLayout instance
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    Leaking: YES (ViewPager2↑ is leaking and View detached and has parent)
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    mContext instance of .....ui.MainActivity with mDestroyed = false
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View#mParent is set
2020-09-15 05:39:33.465 9611-9689/.... D/LeakCanary: │    View#mAttachInfo is null (view detached)
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: │    View.mWindowAttachCount = 1
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: │    ↓ CoordinatorLayout.mParent
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ╰→ androidx.drawerlayout.widget.DrawerLayout instance
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ​     Leaking: YES (ObjectWatcher was watching this because .....ui.fragments.ReadFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent leaks))
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ​     key = e3b96092-06d5-48ae-93bf-b38680cc0c35
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ​     watchDurationMillis = 6413
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ​     retainedDurationMillis = 1400
2020-09-15 05:39:33.466 9611-9689/.... D/LeakCanary: ​     mContext instance of .....ui.MainActivity with mDestroyed = false
2020-09-15 05:39:33.467 9611-9689/.... D/LeakCanary: ​     View#mParent is null
2020-09-15 05:39:33.467 9611-9689/.... D/LeakCanary: ​     View#mAttachInfo is null (view detached)
2020-09-15 05:39:33.467 9611-9689/.... D/LeakCanary: ​     View.mID = R.id.drawer_layout

So, to solve this, I cleared any child fragment that has been off the ViewPager; to do this I had to register all fragment Ids, and override containsItem() to clear them in there like below:

public class PageFragmentPagerAdapter extends FragmentStateAdapter {

    private FragmentManager mFragmentMgr;
    private List<Integer> currentPageIds = new ArrayList<>();

    public PageFragmentPagerAdapter(@NonNull FragmentManager fragmentManager, @NonNull Lifecycle lifecycle) {
        super(fragmentManager, lifecycle);
        mFragmentMgr = fragmentManager;
    }

    @NonNull
    @Override
    public Fragment createFragment(int position) {
        PageFragment pageFragment = PageFragment.newInstance(position);
        currentPageIds.add(position);
        return pageFragment;
    }


    @Override
    public long getItemId(int position) {
        return position;
    }

    @Override
    public int getItemCount() {
        return N_PAGES;
    }

    @Override
    public boolean containsItem(long itemId) {
        for (Integer id : currentPageIds)
            if (id == itemId) {
                currentPageIds.remove(Integer.valueOf(String.valueOf(itemId)));
                clearFragment(id);
                break;
            }
        return super.containsItem(itemId);
    }

    private void clearFragment(int fragmentId) {
        FragmentTransaction transaction = mFragmentMgr.beginTransaction();
        PageFragment fragment = (PageFragment) mFragmentMgr.findFragmentByTag("f" + fragmentId);
        if (fragment != null) {
            transaction.remove(fragment);
        }
        transaction.commitAllowingStateLoss();
    }

}

The second thing don't instantiate the ViewPager adapter with the lifecycle of the activity with requireActivity().getLifecycle(), instead use the lifecycle of its fragment getViewLifecycleOwner().getLifecycle() like below

PageFragmentPagerAdapter mPagerAdapter = new PageFragmentPagerAdapter(
     getChildFragmentManager(), getViewLifecycleOwner().getLifecycle());

Crossfade

Answer 3

When Android configuration changes, such as screen rotation, activities and fragments are recreated, so you need clean references to avoid memory leak.

For example:

public void onDestroyView() {
    super.onDestroyView();

    if(viewPager != null) {
        viewPager.setAdapter(null);
        viewPager.removeOnPageChangeListener(null);
    }

    if(tabLayout != null) {
        tabLayout.clearOnTabSelectedListeners();
        tabLayout.setupWithViewPager(null);
    }

    viewPager = null;
    tabLayout = null;
}

Denominational

Answer 4

I had same issue. I set null TabLayoutMediator value but Leak didn't disappear after this, I detached TabLayoutMediator in onDestroyView and it worked for me.

override fun onDestroyView() {
    super.onDestroyView()

    mediator?.detach()
    mediator = null
    _binding = null
}

Dorcia

Answer 5

To anybody wondering... the reason why this leaks a lot in ViewPager2 (androidx) is because: The Adapter requires a parameter called Lifecycle.

This parameter connects an Observer to the parent Fragment.

As is required by Adapters, they need to be "cleared" upon view destruction.

The issue is that there is no REAL way to do this.

This means that the only way is to:

recyclerView.setAdapter(null);

The problem with this is that the Adapter had previously connected an observer to the lifeCycle:

mLifecycle.addObserver(new LifecycleEventObserver() {

holding a reference to mFragments inside the Adapter, and not only that but each and every field captured by the LifecycleEventObserver.

There is one instance in particular thats the culrpit of all leaks and is the observer connected at line 548 in FragmentStateAdapter.class

mLifecycle.addObserver(new LifecycleEventObserver() {
    @Override
    public void onStateChanged(@NonNull LifecycleOwner source,
            @NonNull Lifecycle.Event event) {
        if (event == Lifecycle.Event.ON_DESTROY) {
            handler.removeCallbacks(runnable);
            source.getLifecycle().removeObserver(this);
        }
    }
});

The default way in which Adapters must be instantiated according to the docummetnation, and using the default constructor is by passing fragment.getLifecycle();

Leaks: Scenario A.

An Adapter field instance held inside parent Fragment while it is being pushed to the backstack.

solution A: While still using the default constructor, the Adapter field instance must be set to null during onViewDestroyed, both the Adapter field, AND the viewPager.setAdapter to NULL. This solution creates a problem during Scenario B...

solution B: use the NON-default constructor and pass fragment.getViewLifecycleOwner().getLifeCycle() instead of Fragment. Build a new adapter each time onViewCreated(). and the observer will automatically unregister itself upon onViewDestroyed(). The adapter must still be set to null BUT the vp.setAdapter NOT

Scenario B:

Parent Fragment is setRetainInstance(true) while solution A is being used.

cause: a new FragmentManager is delivered to the ViewPager but the previous Adapter instance, captured by the LifecycleEventObserver will attepmt to remove Frgaments with a FragmentManager version that does not exist anymore. this leak appears 10 seconds after a configuration change.

solution: I am currently working on it and will update If a solution is found...

Forget solution A (fragment constructor). For it to work, Fragment lifecycles need to be fixed, since the phases are in disorder.

FIRST) onCreate should be applied BEFORE onAttach, while onDestroy should come AFTER onDetach. And this sequence should be kept during ALL form of transactions, either it be backstack push/pull/pop or config change.

SECOND) 2 Event callback steps to the Lifecycle should be added, or rather... moved from the place they are currently: these events would be ON_START and ON_STOP, and they should be dispatched during onAttach and onDetach.

This events should be the ones that define weather or not the Observer should be detached during a configuration change and NOT onDestroy OR onDestroyView.

This would allow:

A) using the Fragment.lifecycle instead of the fragment.viewLifecycleOwner lifecycle (as was originally intended).

B) preventing an unnecessary disconnection of the Observer during backstack push, since observer garbage collection is not required during this action.

C) to correctly infer a disconnection triggered by configuration change and detaching the observer in this phase.

So, my recommendation is to use viewLifecycleOwner instead... it's easier...

Shadow