Android: ViewModel with paging 3 flow is leaking

2.8k views Asked by At

My problem is, that my shopViewModel which holds an instance of a paging-flow is somehow leaking. I've tried to solve this problem by converting the flow into a livedata, but that changed nothing.

ViewModel

class ShopViewModel @ViewModelInject constructor(
    private val shopPagingSource: ShopPagingSource,
) : ViewModel() {
    val SHOP_PAGE_CONFIG: PagingConfig = PagingConfig(pageSize = 20, enablePlaceholders = false)

    // As LiveData
    val shopFlow = Pager(SHOP_PAGE_CONFIG) { shopPagingSource }.flow.cachedIn(viewModelScope).asLiveData()

   // Before
    val shopFlow = Pager(SHOP_PAGE_CONFIG) { shopPagingSource }.flow.cachedIn(viewModelScope)
}

Fragment

@AndroidEntryPoint
class ShopFragment(private val shopListAdapter: ShopAdapter) : Fragment(R.layout.fragment_shop), ShopAdapter.OnItemClickListener {
    private val shopViewModel: ShopViewModel by viewModels()
    private val shopBinding: FragmentShopBinding by viewBinding()

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        shopBinding.adapter = shopListAdapter.withLoadStateFooter(ShopLoadAdapter(shopListAdapter::retry))
        shopListAdapter.clickHandler(this)
        collectShopList()
    }
    
    override fun forwardClick(product: @NotNull Product) {
        val action = ShopFragmentDirections.actionShopFragmentToShopItemFragment(product)
        findNavController().navigate(action)
    }
   
    private fun collectShopListWithLiveData() = lifecycleScope.launch {
        shopViewModel.shopFlow.observe(viewLifecycleOwner) {
            lifecycleScope.launch {
                shopListAdapter.submitData(it)
            }
        }
    }

    // Before converting to livedata
    private fun collectShopListWithFlow() = lifecycleScope.launch {
        shopViewModel.shopFlow.collectLatest {
             shopListAdapter.submitData(it)
        }
    }


    // To avoid memory leak from injected adapter
    override fun onDestroyView() {
        requireView().findViewById<RecyclerView>(R.id.rv_shop).adapter = null
        super.onDestroyView()
    }
}

Adapter

class ShopAdapter @Inject constructor() : PagingDataAdapter<Product, ShopAdapter.ShopViewHolder>(Companion) {

    private lateinit var clickListener: OnItemClickListener

    companion object: DiffUtil.ItemCallback<Product>() {
        override fun areItemsTheSame(oldItem: Product, newItem: Product): Boolean = oldItem.articelNumber == newItem.articelNumber
        override fun areContentsTheSame(oldItem: Product, newItem: Product): Boolean = oldItem == newItem
    }

    inner class ShopViewHolder(val binding: ShopListItemBinding) : RecyclerView.ViewHolder(binding.root)

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ShopAdapter.ShopViewHolder {
        val layoutInflater = LayoutInflater.from(parent.context)
        val binding = ShopListItemBinding.inflate(layoutInflater, parent, false)
        return ShopViewHolder(binding).also {
            binding.mcvProductItem.setOnClickListener { clickListener.forwardClick(binding.product!!) }
        }
    }

    override fun onBindViewHolder(holder: ShopAdapter.ShopViewHolder, position: Int) {
        holder.binding.product = getItem(position) ?: return
        holder.binding.executePendingBindings()
    }

    fun clickHandler(clickEventHandler: OnItemClickListener) {
        clickListener = clickEventHandler
    }

    interface OnItemClickListener {
        fun forwardClick(product: @NotNull Product)
    }
}

MainFragmentFactory

class MainFragmentFactory @Inject constructor(
    // .. other dependencies
    private val shopAdapter: ShopAdapter,
) : FragmentFactory() {
    override fun instantiate(classLoader: ClassLoader, className: String): Fragment = when(className) {
        // ... other fragments
        ShopFragment::class.java.name -> ShopFragment(shopAdapter)
        else -> super.instantiate(classLoader, className)
}

PagingSource

class ShopPagingSource @Inject constructor(
    private val shopRepository: ShopFirebaseRepository,
) : PagingSource<QuerySnapshot, Product>() {

    override suspend fun load(params: LoadParams<QuerySnapshot>): LoadResult<QuerySnapshot, Product> = try {
            withTimeout(SHOP_MAX_LOADING_TIME) {
                val currentPage = params.key ?: shopRepository.getCurrentPage()

                val lastDocumentSnapShot = currentPage.documents[currentPage.size() - 1]

                val nextPage = shopRepository.getNextPage(lastDocumentSnapShot)

                LoadResult.Page(
                    data = currentPage.toObjects(),
                    prevKey = null,
                    nextKey = nextPage
                )
            }
        } catch (e: TimeoutCancellationException) {
            Timber.d("Mediator failed, No Internet Connection")
            LoadResult.Error(e)
        } catch (e: ArrayIndexOutOfBoundsException) {
            Timber.d("Mediator failed, ArrayIndexOutOfBounds")
            LoadResult.Error(e)
        } catch (e: Exception) {
            Timber.d("Mediator failed, Unknown Error: ${e.message.toString()}")
            LoadResult.Error(e)
        }
    }

LeakCanary

D/LeakCanary: ====================================
    HEAP ANALYSIS RESULT
    ====================================
    1 APPLICATION LEAKS
    
    References underlined with "~~~" are likely causes.
    Learn more at https://squ.re/leaks.
    
    2618 bytes retained by leaking objects
    Signature: 944313b4ecbdb77c99682dc8c1646e12e4f37d8
    ┬───
    │ GC Root: Local variable in native code
    │
    ├─ dalvik.system.PathClassLoader instance
    │    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never leaking)
    │    ↓ PathClassLoader.runtimeInternalObjects
    ├─ java.lang.Object[] array
    │    Leaking: NO (InternalLeakCanary↓ is not leaking)
    │    ↓ Object[].[2142]
    ├─ leakcanary.internal.InternalLeakCanary class
    │    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
    │    ↓ static InternalLeakCanary.resumedActivity
    ├─ com.example.app.framework.ui.view.MainActivity instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking and Activity#mDestroyed is false)
    │    ↓ MainActivity.navController$delegate
    ├─ kotlin.SynchronizedLazyImpl instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking)
    │    ↓ SynchronizedLazyImpl._value
    ├─ androidx.navigation.NavHostController instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking)
    │    ↓ NavHostController.mLifecycleOwner
    ├─ com.example.app.framework.ui.view.utils.MainNavHostFragment instance
    │    Leaking: NO (Fragment#mFragmentManager is not null)
    │    ↓ MainNavHostFragment.mainFragmentFactory
    │                          ~~~~~~~~~~~~~~~~~~~
    ├─ com.example.app.framework.ui.view.utils.MainFragmentFactory instance
    │    Leaking: UNKNOWN
    │    ↓ MainFragmentFactory.shopAdapter
    │                          ~~~~~~~~~~~
    ├─ com.example.app.framework.ui.adapter.recyclerview.ShopAdapter instance
    │    Leaking: UNKNOWN
    │    ↓ ShopAdapter.differ
    │                  ~~~~~~
    ├─ androidx.paging.AsyncPagingDataDiffer instance
    │    Leaking: UNKNOWN
    │    ↓ AsyncPagingDataDiffer.differBase
    │                            ~~~~~~~~~~
    ├─ androidx.paging.AsyncPagingDataDiffer$differBase$1 instance
    │    Leaking: UNKNOWN
    │    Anonymous subclass of androidx.paging.PagingDataDiffer
    │    ↓ AsyncPagingDataDiffer$differBase$1.receiver
    │                                         ~~~~~~~~
    ├─ androidx.paging.PageFetcher$PagerUiReceiver instance
    │    Leaking: UNKNOWN
    │    ↓ PageFetcher$PagerUiReceiver.this$0
    │                                  ~~~~~~
    ├─ androidx.paging.PageFetcher instance
    │    Leaking: UNKNOWN
    │    ↓ PageFetcher.pagingSourceFactory
    │                  ~~~~~~~~~~~~~~~~~~~
    ├─ com.example.app.framework.ui.viewmodel.ShopViewModel$shopFlow$1 instance
    │    Leaking: UNKNOWN
    │    Anonymous subclass of kotlin.jvm.internal.Lambda
    │    ↓ ShopViewModel$shopFlow$1.this$0
    │                               ~~~~~~
    ╰→ com.example.app.framework.ui.viewmodel.ShopViewModel instance
    ​     Leaking: YES (ObjectWatcher was watching this because com.example.app.framework.ui.viewmodel.ShopViewModel received ViewModel#onCleared() callback)
    ​     key = 0e65fcab-e6dd-475a-83d4-87b2050d797b
    ​     watchDurationMillis = 7771
    ​     retainedDurationMillis = 2769
    ====================================

EDIT

When scoping the ShopAdapter with @FragmentScoped, I get the following leak:

    ┬───
    │ GC Root: Local variable in native code
    │
    ├─ dalvik.system.PathClassLoader instance
    │    Leaking: NO (InternalLeakCanary↓ is not leaking and A ClassLoader is never leaking)
    │    ↓ PathClassLoader.runtimeInternalObjects
    ├─ java.lang.Object[] array
    │    Leaking: NO (InternalLeakCanary↓ is not leaking)
    │    ↓ Object[].[409]
    ├─ leakcanary.internal.InternalLeakCanary class
    │    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
    │    ↓ static InternalLeakCanary.resumedActivity
    ├─ com.example.app.framework.ui.view.MainActivity instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking and Activity#mDestroyed is false)
    │    mApplication instance of com.example.app.App
    │    mBase instance of androidx.appcompat.view.ContextThemeWrapper, not wrapping known Android context
    │    ↓ MainActivity.navController$delegate
    ├─ kotlin.SynchronizedLazyImpl instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking)
    │    ↓ SynchronizedLazyImpl._value
    ├─ androidx.navigation.NavHostController instance
    │    Leaking: NO (MainNavHostFragment↓ is not leaking)
    │    mActivity instance of com.example.app.framework.ui.view.MainActivity with mDestroyed = false
    │    mContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper, wrapping
    │    activity com.example.app.framework.ui.view.MainActivity with mDestroyed = false
    │    ↓ NavHostController.mLifecycleOwner
    ├─ com.example.app.framework.ui.view.utils.MainNavHostFragment instance
    │    Leaking: NO (Fragment#mFragmentManager is not null)
    │    componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
    │    wrapping activity com.example.app.framework.ui.view.MainActivity with mDestroyed = false
    │    ↓ MainNavHostFragment.mainFragmentFactory
    │                          ~~~~~~~~~~~~~~~~~~~
D/LeakCanary: ├─ com.example.app.framework.ui.view.utils.MainFragmentFactory instance
    │    Leaking: UNKNOWN
    │    Retaining 212 bytes in 7 objects
    │    ↓ MainFragmentFactory.shopAdapter
    │                          ~~~~~~~~~~~
    ├─ com.example.app.framework.ui.adapter.recyclerview.ShopAdapter instance
    │    Leaking: UNKNOWN
    │    Retaining 14461 bytes in 546 objects
    │    ↓ ShopAdapter.clickListener
    │                  ~~~~~~~~~~~~~
    ╰→ com.example.app.framework.ui.view.fragments.shop.ShopFragment instance
    ​     Leaking: YES (ObjectWatcher was watching this because com.example.app.framework.ui.view.fragments.shop.
    ​     ShopFragment received Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
    ​     Retaining 2121 bytes in 79 objects
    ​     key = 71ec5094-8509-47a5-9e0a-070fe642ca8a
    ​     watchDurationMillis = 18366
    ​     retainedDurationMillis = 13365
    ​     componentContext instance of dagger.hilt.android.internal.managers.ViewComponentManager$FragmentContextWrapper,
    ​     wrapping activity com.example.app.framework.ui.view.MainActivity with mDestroyed = false
2

There are 2 answers

0
Andrew On BEST ANSWER

Okay I've managed to solve this leak. The leak was caused, because I've injected my ShopAdapter via Constructor Injection into my Fragment. When injecting something into the fragment via constructor injection, you have to pass the dependency to the MainFragmentFactory. But because of this, the MainFragmentFactory will always hold a reference to the adapter, even when the fragment is destroyed and the fragment is not needed any more (therefore, requireView().findViewById<RecyclerView>(R.id.rv_shop).adapter = null wont't even make a change here).

To solve this problem, DON'T inject the Adapter via constructor injection and rather inject it via field injection.

1
Warcello On

I know that I'm late for the party but I can see that you are using lifecycleScope.launch a lot and inside you are calling the adapter to submitData. This means this adapter will not be able to be properly garbage collected. This is probably the root of memory leak. Try to use viewLifecycleOwner.lifecycleScope.launch instead.

This is a known mistake: https://proandroiddev.com/5-common-mistakes-when-using-architecture-components-403e9899f4cb