Android paging flowable leaking viewmodel and fragment

Question

I'm using the paging 3 android library with the RxJava source. I have two fragments, the first displays a list of images in a grid, when an image is clicked the second fragment is shown and it displays the image in fullscreen and has a ViewPager to swipe between images. Because those use the same data I figured I can use a shared view model, in both fragments I have

@Inject lateinit var viewModelFactory: ViewModelProvider.Factory
private val viewModel by activityViewModels<FilesViewModel> { viewModelFactory }

And the viewmodel creates the rx flowable that both fragments observe when their view is visible

class FilesViewModel @Inject constructor(
    settings: SettingsRepository,
    private val filesRepository: FilesRepository
): ViewModel() {

    ...

    var cachedFileList = filesRepository.getPagedFiles("path").cachedIn(viewModelScope)

    ...

}

After navigating back to the list the fragment is retained, after doing that five times LeakCanary shows the leak

┬───
│ GC Root: System class
│
├─ leakcanary.internal.InternalLeakCanary class
│    Leaking: NO (MainActivity↓ is not leaking and a class is never leaking)
│    ↓ static InternalLeakCanary.resumedActivity
├─ com.guillermonegrete.gallery.folders.MainActivity instance
│    Leaking: NO (Activity#mDestroyed is false)
│    mApplication instance of com.guillermonegrete.gallery.MyApplication
│    mBase instance of android.app.ContextImpl
│    ↓ ComponentActivity.mViewModelStore
│                        ~~~~~~~~~~~~~~~
├─ androidx.lifecycle.ViewModelStore instance
│    Leaking: UNKNOWN
│    Retaining 816 B in 11 objects
│    ↓ ViewModelStore.mMap
│                     ~~~~
├─ java.util.HashMap instance
│    Leaking: UNKNOWN
│    Retaining 804 B in 10 objects
│    ↓ HashMap.table
│              ~~~~~
├─ java.util.HashMap$HashMapEntry[] array
│    Leaking: UNKNOWN
│    Retaining 764 B in 9 objects
│    ↓ HashMap$HashMapEntry[].[0]
│                             ~~~
├─ java.util.HashMap$HashMapEntry instance
│    Leaking: UNKNOWN
│    Retaining 520 B in 6 objects
│    ↓ HashMap$HashMapEntry.value
│                           ~~~~~
├─ com.guillermonegrete.gallery.files.FilesViewModel instance
│    Leaking: UNKNOWN
│    Retaining 4,0 kB in 145 objects
│    ↓ FilesViewModel.cachedFileList
│                     ~~~~~~~~~~~~~~
├─ io.reactivex.internal.operators.flowable.FlowableFromPublisher instance
│    Leaking: UNKNOWN
│    Retaining 28 B in 2 objects
│    ↓ FlowableFromPublisher.publisher
│                            ~~~~~~~~~
├─ kotlinx.coroutines.reactive.FlowAsPublisher instance
│    Leaking: UNKNOWN
│    Retaining 16 B in 1 objects
│    ↓ FlowAsPublisher.flow
│                      ~~~~
├─ kotlinx.coroutines.flow.SafeFlow instance
│    Leaking: UNKNOWN
│    Retaining 48 B in 2 objects
│    ↓ SafeFlow.block
│               ~~~~~
├─ androidx.paging.multicast.Multicaster$flow$1 instance
│    Leaking: UNKNOWN
│    Retaining 36 B in 1 objects
│    Anonymous subclass of kotlin.coroutines.jvm.internal.SuspendLambda
│    ↓ Multicaster$flow$1.this$0
│                         ~~~~~~
├─ androidx.paging.multicast.Multicaster instance
│    Leaking: UNKNOWN
│    Retaining 108 B in 4 objects
│    ↓ Multicaster.channelManager$delegate
│                  ~~~~~~~~~~~~~~~~~~~~~~~
├─ kotlin.SynchronizedLazyImpl instance
│    Leaking: UNKNOWN
│    Retaining 50 B in 2 objects
│    ↓ SynchronizedLazyImpl._value
│                           ~~~~~~
├─ androidx.paging.multicast.ChannelManager instance
│    Leaking: UNKNOWN
│    Retaining 30 B in 1 objects
│    ↓ ChannelManager.actor
│                     ~~~~~
├─ androidx.paging.multicast.ChannelManager$Actor instance
│    Leaking: UNKNOWN
│    Retaining 19,5 kB in 573 objects
│    ↓ ChannelManager$Actor.channels
│                           ~~~~~~~~
├─ java.util.ArrayList instance
│    Leaking: UNKNOWN
│    Retaining 19,4 kB in 567 objects
│    ↓ ArrayList.elementData
│                ~~~~~~~~~~~
├─ java.lang.Object[] array
│    Leaking: UNKNOWN
│    Retaining 19,4 kB in 566 objects
│    ↓ Object[].[1]
│               ~~~
├─ androidx.paging.multicast.ChannelManager$ChannelEntry instance
│    Leaking: UNKNOWN
│    Retaining 3,6 kB in 110 objects
│    ↓ ChannelManager$ChannelEntry.channel
│                                  ~~~~~~~
├─ kotlinx.coroutines.channels.LinkedListChannel instance
│    Leaking: UNKNOWN
│    Retaining 3,6 kB in 109 objects
│    ↓ AbstractSendChannel.queue
│                          ~~~~~
├─ kotlinx.coroutines.internal.LockFreeLinkedListHead instance
│    Leaking: UNKNOWN
│    Retaining 3,6 kB in 108 objects
│    ↓ LockFreeLinkedListNode._next
│                             ~~~~~
├─ kotlinx.coroutines.channels.Closed instance
│    Leaking: UNKNOWN
│    Retaining 3,6 kB in 106 objects
│    ↓ Closed.closeCause
│             ~~~~~~~~~~
├─ kotlinx.coroutines.JobCancellationException instance
│    Leaking: UNKNOWN
│    Retaining 3,5 kB in 105 objects
│    ↓ JobCancellationException.job
│                               ~~~
├─ kotlinx.coroutines.reactive.FlowSubscription instance
│    Leaking: UNKNOWN
│    Retaining 3,4 kB in 102 objects
│    ↓ FlowSubscription.subscriber
│                       ~~~~~~~~~~
├─ io.reactivex.internal.operators.flowable.
│  FlowableSubscribeOn$SubscribeOnSubscriber instance
│    Leaking: UNKNOWN
│    Retaining 3,3 kB in 99 objects
│    ↓ FlowableSubscribeOn$SubscribeOnSubscriber.downstream
│                                                ~~~~~~~~~~
├─ io.reactivex.internal.operators.flowable.
│  FlowableObserveOn$ObserveOnSubscriber instance
│    Leaking: UNKNOWN
│    Retaining 3,2 kB in 93 objects
│    ↓ FlowableObserveOn$ObserveOnSubscriber.downstream
│                                            ~~~~~~~~~~
├─ io.reactivex.internal.subscribers.LambdaSubscriber instance
│    Leaking: UNKNOWN
│    Retaining 2,6 kB in 86 objects
│    ↓ LambdaSubscriber.onNext
│                       ~~~~~~
├─ com.guillermonegrete.gallery.files.details.
│  FileDetailsFragment$setUpViewModel$1 instance
│    Leaking: UNKNOWN
│    Retaining 2,5 kB in 85 objects
│    Anonymous class implementing io.reactivex.functions.Consumer
│    ↓ FileDetailsFragment$setUpViewModel$1.this$0
│                                           ~~~~~~
╰→ com.guillermonegrete.gallery.files.details.FileDetailsFragment instance
     Leaking: YES (ObjectWatcher was watching this because com.
     guillermonegrete.gallery.files.details.FileDetailsFragment received
     Fragment#onDestroy() callback and Fragment#mFragmentManager is null)
     Retaining 2,5 kB in 84 objects
     key = 0b0dad5d-1c55-4938-94d8-0f923fc29508
     watchDurationMillis = 23025
     retainedDurationMillis = 18023

You can see that the 2nd fragment is leaking and it is related to cachedFileList Now if I remove the cachedIn(viewModelScope) then the leaks are gone however the app now makes API calls every time I navigate between fragments, which the whole point of sharing the view model is to save api calls.

Is there any way to avoid the multiple api calls and the leaks? I know I can use a database but I want to avoid that overhead if possible.

EDIT:

How the flow is consumed, basically the same for both fragments

class FilesListFragment: Fragment(R.layout.fragment_files_list) {
   ...
   override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        bindViewModel(folder)
   }
   
   private fun bindViewModel(folder: String){
       disposable.add(viewModel.loadPagedFiles(folder)
            .subscribeOn(Schedulers.io()) // Omitted some mapping
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe(
                { adapter.submitData(lifecycle, it) },
                { error -> println("Error loading files: ${error.message}") }
            )
        )
   }

    override fun onDestroyView() {
        binding.filesList.adapter = null
        _binding = null
        disposable.clear()
        adapter.removeLoadStateListener(loadListener)
        super.onDestroyView()
    }
   ...
}

Here is how it is created

class DefaultFilesRepository @Inject constructor(private var fileAPI: FilesServerAPI): FilesRepository {

    override fun getPagedFiles(folder: String): Flowable<PagingData<File>> {
        return Pager(PagingConfig(pageSize = 20)) {
            FilesPageSource(fileAPI, baseUrl, folder)
        }.flowable
    }

}

@IR42 the activity is not leaking, it's the 2nd fragment, the one that is shown after clicking an item in the 1st fragment — Jorge Guillermo Negrete, Oct 20 '21 at 04:35
Can you share how you are consuming the PagingData stream in each of your fragments and also how the stream is created? Paging is completely dependent on the collector's scope, so it automatically cancels / closes itself when collection finishes. cachedIn on the other hand, turns the stream hot in the scope you pass in, so it will not get cleaned up until viewModelScope does, but I have a feeling this is not your issue. Please also try with 3.1.0-beta01 if you haven't already as there have been some small improvements to cachedIn recently. — dlam, Oct 21 '21 at 08:04
@dlam I used a workaround, I combined the two fragments into one and no more leaks, I just hide/show the views. For the original code, I use a CompositeDisposable set between onViewCreated/onDestroyView for both fragments. Yesterday I tried the latest version indicated by AS and the leaks still persisted. Here is the [code](https://github.com/memostark/LocalGallery/tree/master/app/src/main/java/com/guillermonegrete/gallery/files) still using the two fragments version, it's different from the code in the question but both versions leaked. I'll add the relevant code to the question. — Jorge Guillermo Negrete, Oct 22 '21 at 01:21
So there are two things kind of suspicious about your sample you should fix, although I am not sure that it will fix what LeakCanary is complaining about. 1. You should .`switchMap` a single stream of queries into `Flowable` and then call `.cachedIn` on the overall `Flowable`, otherwise you will keep a copy of the list cached unnecessarily every time your query changes until ViewModel is cleaned up. 2. Before paging 3.1.0-alpha04, the launched job from `.submitData` was not eagerly canceled so it depended on the observer's scope to close. Please try bumping to 3.1.0-beta01 as well. — dlam, Oct 23 '21 at 07:48
@dlam Using `switchMap` with a `PublishSubject` fixed the leaks. For some reason when observing the flowable adding `subscribeOn(Schedulers.io())` caused issues, I removed it, I figured the library already uses its own background dispatcher. — Jorge Guillermo Negrete, Oct 31 '21 at 19:45
What issues does `.subscribeOn(Schedulers.io())` cause? There aren't any restrictions to call `submitData` or observe the stream on main thread, so it would be ideal to move off of it if possible. Please try bumping your version to 3.1.0-beta01 when using `.subscribeOn(Schedulers.io())` to see if the eager cancellation fixes in the latest version resolve your issues. — dlam, Nov 01 '21 at 19:24
@dlam I added a new answer to the question that explains those issues and how I fixed them, unfortunately, I can't use the latest paging version because of the minimum 31 sdk restriction. — Jorge Guillermo Negrete, Nov 02 '21 at 01:31
The compile / target sdk should generally point to the latest, the min sdk for paging is still 14. — dlam, Nov 02 '21 at 01:44
@dlam Tried `3.1.0-rc01`, the `ClosedSendChannelException` is gone so I put the `subscribeOn` back on the 2nd fragment, but the 1st fragment still has the problem that the `Subject` is not called the first time, but I think this problem is related to `Subject` and not the paging library, so thanks for your help. — Jorge Guillermo Negrete, Nov 08 '21 at 04:37

score 1 · Accepted Answer · answered Nov 02 '21 at 01:29

As recommended by @dlam, I used a switchMap.

class FilesViewModel @Inject constructor(
    settings: SettingsRepository,
    private val filesRepository: FilesRepository
): ViewModel() {

    ...

    private val folderName: Subject<String> = PublishSubject.create()

    var cachedFileList: Flowable<PagingData<File>> = folderName.distinctUntilChanged().switchMap {
        filesRepository.getPagedFiles(it).toObservable()
    }.toFlowable(BackpressureStrategy.LATEST).cachedIn(viewModelScope)

    fun setFolderName(name: String){
        folderName.onNext(name)
    }
    ...
}

And observing the data from the fragments

disposable.add(viewModel.loadPagedFiles(folder)
            // .subscribeOn(Schedulers.io()) 
            .observeOn(AndroidSchedulers.mainThread()) // Omitted some mapping
            .subscribe(
                { adapter.submitData(lifecycle, it) },
                { error -> println("Error loading files: ${error.message}") }
            )
        )
viewModel.setFolderName(folder)

I removed .subscribeOn(Schedulers.io()) from the 1st fragment, for some reason it caused the switchMap to never be called at the start. A related question.

Also removed the one in the 2nd fragment. When navigating back to the 1st fragment this exception was thrown:

kotlinx.coroutines.channels.ClosedSendChannelException: Channel was closed

After removing the subscribeOn the exception went away.

Android paging flowable leaking viewmodel and fragment

1 Answers1