android / views-widgets-samples

Multiple samples showing the best practices in views-widgets on Android.
Apache License 2.0
5.03k stars 3.01k forks source link

Viewpager2 FragmentStateAdapter leak #122

Open PierreFourreau opened 4 years ago

PierreFourreau commented 4 years ago

Hi,

I implement a basic viewpager2

My main fragment (which contain a viewpager of 2 fragments (fragmentA and fragment B))

...
 override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
 my_view_pager.adapter = MyAdapter(activity, args.refBrand)
        TabLayoutMediator(my_tab_layout, view_pager) { tab, position ->
            tab.text = if(position == 0) "1" else "2"
        }.attach()
}
...

Adapter :

class MyAdapter : FragmentStateAdapter(fragmentActivity) {

    override fun getItemCount(): Int = 2

    override fun createFragment(position: Int): Fragment = when (position) {
        0 -> FirstFragment.newInstance(filter = true)
        1 -> Second.newInstance(filter = false)
        else -> throw IllegalArgumentException...
    }
}

The FragmentStateAdapter is leaking.

Like my others screens without viewpagers I tried

override fun onDestroyView() {
        super.onDestroyView()
        my_view_pager.adapter = null
    }

But it crash when I go to another fragment destination from fragmentA (or B) and come back :

E/AndroidRuntime: FATAL EXCEPTION: main Process: app, PID: 30471 java.lang.IllegalStateException: Fragment no longer exists for key f#0: unique id 7ee10572-a2f6-4e46-8227-daaad57fd649 at androidx.fragment.app.FragmentManager.getFragment(FragmentManager.java:772) at androidx.viewpager2.adapter.FragmentStateAdapter.restoreState(FragmentStateAdapter.java:549) at androidx.viewpager2.widget.ViewPager2.restorePendingState(ViewPager2.java:350) at androidx.viewpager2.widget.ViewPager2.dispatchRestoreInstanceState(ViewPager2.java:375) at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:3864) at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:3864) at android.view.View.restoreHierarchyState(View.java:19793) at androidx.fragment.app.Fragment.restoreViewState(Fragment.java:573) at androidx.fragment.app.FragmentStateManager.restoreViewState(FragmentStateManager.java:356) at androidx.fragment.app.FragmentManager.moveToState(FragmentManager.java:1189) at androidx.fragment.app.FragmentManager.addAddedFragments(FragmentManager.java:2224) at androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1997) at androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1953) at androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1849) at androidx.fragment.app.FragmentManager$4.run(FragmentManager.java:413) at android.os.Handler.handleCallback(Handler.java:883) at android.os.Handler.dispatchMessage(Handler.java:100) at android.os.Looper.loop(Looper.java:214) at android.app.ActivityThread.main(ActivityThread.java:7356) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

Can you help? Thanks

hushino commented 4 years ago

Same problem here

jermgilb commented 4 years ago

When using a Navigation Graph which loads a contain fragment which contains FragmentStateAdapter

Its also reported here: https://issuetracker.google.com/issues/154751401

AleksandarAleksiev commented 3 years ago

I had the same problem but it looks like I have found a workaround. In my case, I was adding data to the adapter before attaching it to the ViewPager2 adapter. If I first set the ViewPager2 adapter and then add data then looks like the FragmentStateAdapter is no longer leaking and the app is not crashing

akelix commented 3 years ago

The same problem. My solution: viewPager2.addOnAttachStateChangeListener(object : View.OnAttachStateChangeListener { override fun onViewDetachedFromWindow(v: View) { viewPager2.removeOnAttachStateChangeListener(this) tabLayoutMediator.detach() viewPager2.setup.unregisterOnPageChangeCallback(onPageChangeCallback) viewPager2..adapter = null } })

FragmentStateAdapter is still leaking because it uses lifecycle from mainnavigationfragment. But it uses only 800bytes.

Rajarml commented 2 years ago

Using viewpager.adapter = null in onDestroyView() is causing this issue, removing this line avoid the crash. But nullify the ViewPager's adapter is an attempt to fix the memory leak which should be fixed with a yet unknown efficient manner 😞

vlasentiy commented 2 years ago

You can try to set this layout property for androidx.viewpager2.widget.ViewPager2 android:saveEnabled="false"

alkocher commented 2 years ago

Just create adapter with Fragment: FragmentStateAdapter(fragment)

Don't use FragmentStateAdapter(fragmentActivity)

OonKhengHuang commented 2 years ago

You can try to set this layout property for androidx.viewpager2.widget.ViewPager2 android:saveEnabled="false"

U are my lifesaver! my viewpager was already inside a fragment, and I try the childFragmentManager but the app will crash when I press again, but adding this"savedEnabled=false", it works perfectly!

hoc081098 commented 2 years ago

According to https://issuetracker.google.com/issues/151212195#comment3

FragmentStateAdapter(fragment.childFragmentManager, fragment.viewLifecycleOwner.lifecycle)
VinuPolly-Bonnet commented 1 year ago

Has someone found a fix for this issue yet? I have tried quite a few things and only android:saveEnabled="false seems to work, but I need the ViewPager2 to remember the page it is on

alkocher commented 1 year ago

Has someone found a fix for this issue yet? I have tried quite a few things and only android:saveEnabled="false seems to work, but I need the ViewPager2 to remember the page it is on

Try to use FragmentStateAdapter(fragment, lifecycle)

VinuPolly-Bonnet commented 1 year ago

@alkocher thanks, but isn't it same as using FragmentStateAdapter(fragment), because I will be anyways using the host fragments childFragmentManager and hostFragment's lifeCycle. Maybe I am missing something here?

alkocher commented 1 year ago

@alkocher thanks, but isn't it same as using FragmentStateAdapter(fragment), because I will be anyways using the host fragments childFragmentManager and hostFragment's lifeCycle. Maybe I am missing something here?

I use this constructor with fragment.viewLifecycleOwner.lifecycle and set adapter to null when onDestroyView.

VinuPolly-Bonnet commented 1 year ago

@alkocher thanks, will try this out 🙌

DiamondIT7 commented 1 year ago

You can try to set this layout property for androidx.viewpager2.widget.ViewPager2 android:saveEnabled="false"

brooooo you saved my time and nerves today!

MINTHANHTIKE25 commented 1 year ago

You can try to set this layout property for androidx.viewpager2.widget.ViewPager2 android:saveEnabled="false"

Thank you for this . Finally, I can fix it.

AlyonaRulezzz commented 7 months ago

Just create adapter with Fragment: FragmentStateAdapter(fragment)

Don't use FragmentStateAdapter(fragmentActivity)

It didn't help in my case(

HeXioahei commented 1 month ago

You can try to set this layout property for androidx.viewpager2.widget.ViewPager2 android:saveEnabled="false"

Thanks!