Obsolete
Status Update
No update yet.
Comments
yb...@google.com
please provide a sample app.
If the AppListFragment is restored on rotation, it should return the same ViewModel.
If it is not restored, it will give a new view model but there is nothing to hold onto the old ViewModel anymore so it should not be leaked.
If the AppListFragment is restored on rotation, it should return the same ViewModel.
If it is not restored, it will give a new view model but there is nothing to hold onto the old ViewModel anymore so it should not be leaked.
kl...@gmail.com
I have quickly cut off the app to basic setup with one Activity and fragment with models injected by dagger from Providers.
With that one VM is not saved for some reason, in full app it leaks. Take a look if you have time, tommorow maybe I will try to reproduce the issue on smaller sample.
With that one VM is not saved for some reason, in full app it leaks. Take a look if you have time, tommorow maybe I will try to reproduce the issue on smaller sample.
kl...@gmail.com
Alright, here is the app with leak.
Steps:
- launch
- rotate several times
- observe logs and see ViewModel references
- press back button and observe leak (Clearing VM called for all leaks)
As a workaround VMProvider in fragment should be moved from onAttach to onViewCreated, but that;s far too late, especially with conjunction with LiveData and DataBinding.
Btw. first sample showcases very common setup of a project, where VM are not even saved.
Steps:
- launch
- rotate several times
- observe logs and see ViewModel references
- press back button and observe leak (Clearing VM called for all leaks)
As a workaround VMProvider in fragment should be moved from onAttach to onViewCreated, but that;s far too late, especially with conjunction with LiveData and DataBinding.
Btw. first sample showcases very common setup of a project, where VM are not even saved.
se...@google.com
In onAttach method none of fragment state is restored, ViewModels aren't an exception, so they aren't restored as well. Only after Fragment.super.onCreate(...) they will be available.
But we probably should try to throw an error in this case.
But we probably should try to throw an error in this case.
kl...@gmail.com
Yeah, probably, many people will never spot that they have this issue under the hood. Thanks for support with that one
se...@google.com
It turned out, that we need to some changes in support library to detect this. In one of the next support lib releases (probably not next one, but one after it) we will make changes in support lib and then fix arch components.
kl...@gmail.com
Cool, thanks
yb...@google.com
il...@google.com
il...@google.com
We've actually gone the other way and made it safe to access ViewModels in onAttach() in
Description
Version used: 1.0.0-beta1
Devices/Android versions reproduced on: Nexus 5/Android 5
Given I'm retrieving models from factory via fragment:
@Provides
public static AppListViewModel provideModel(AppListViewModelFactory factory, AppListFragment fragment) {
return ViewModelProviders.of(fragment, factory).get(AppListViewModel.class);
}
- It will produce new ViewModel for each new Fragment of the same class on rotation.
- Despite Fragmnet is calling onDestroy on rotation, the ViewModel.onCleared is not called until activity is finished
That will result in holding as many VM of dead fragments as many times screen is rotated.
As a workaround model can be retrieved using Activity, but current API is unintuituve and looks like a unintended behavior