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
}
}
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.