ViewModel leak in nested fragments added to a backstack [129593351]

Fixed

Bug

Status Update

No update yet.

Description

dh...@gmail.com

created issue #1

Mar 31, 2019 09:22AM

Component used: 'androidx.lifecycle:lifecycle-extensions:2.0.0'
Devices/Android versions reproduced on: robolectric, emulator, real devices

I have an activity, and a fragment in it (MasterFragment).
Than I add nested fragment (NestedFragmentWithViewModel) to a MasterFragment.
NestedFragmentWithViewModel obtains viewmodel by `ViewModelProviders.of(this)`
Than I replace it (with adding to a backstack) with another fragment (NestedFragment).
After that, activity gets recreated, which is causing:
- MasterFragment to recrete
- NestedFragment (as it is on top of backstack) to recreate
After that, activity gets destroyed, which is causing:
- MasterFragment to destroy and clear its ViewModelStore
- NestedFragment to destroy and clear its ViewModelStore

But no one cares neither about NestedFragmentWithViewModel in the backstack, nor about clearing its ViewModelStore.
It is not cleared when fragment is added to a backstack (cause backstack can be popped to show fragment again).
And it is not cleared when MasterFragment destroys, cause it calls onDestroy on all its 'created' nested fragments, but fragments in a backstack were not created after activity get recreated, so should not be destroyed.

Fragment's ViewModelStore is cleared in fragment's onDestroy() in case activity.isFinishing(). That never happens so, whole ViewModelStore gets leaked.

It is not a totally FragmentManager issue, cause its respects lifecycle of fragments - 'what is dead may never die'.
So it seems fragment's logic to clear its ViewModelStore should be fixed.

Project with described scenario is attached.

ViewModelLeak.zip

714 KB

Download

Comments

se...@google.com <se...@google.com> Mar 31, 2019 10:59AM

Reassigned to il...@google.com.

dh...@gmail.com <dh...@gmail.com> #2Apr 1, 2019 06:17PM

datastore-core and datastore-preferences have dependencies on android for SharedPreferencesMigrations and convenience constructors. We should extract these dependencies so users can use this in kotlin-only parts of their code.

il...@google.com <il...@google.com> #3Apr 9, 2019 06:29PM

Reassigned to jb...@google.com.

Would this enable Kotlin Multiplatform usage? Or just Kotlin/JVM in non-Android settings?

ap...@google.com <ap...@google.com> #4Apr 18, 2019 05:58PM

Marked as fixed.

This work is intended to support kotlin-only modules in android projects using Kotlin/JVM. We haven't tested this in non-android settings and haven't built it using Kotlin Multiplatform.

dh...@gmail.com <dh...@gmail.com> #5Apr 19, 2019 04:37AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c9bbc6f26740be8b2ca486032238442e1138c914
Author: rohitsat13 <rohitsat@google.com>
Date: Tue Sep 29 09:36:28 2020

Split out datastore and datastore-core modules

Test: Ran all datastore tests
Bug: 168512698
Relnote: Created a pure kotlin dependency for datastore to allow for faster compilation
Change-Id: I42d75d0f66dc0cac19bd0f452a84b58806a65a46

M buildSrc/src/main/kotlin/androidx/build/PublishDocsRules.kt
A datastore/datastore-core/api/1.0.0-alpha01.txt
M datastore/datastore-core/api/current.txt
A datastore/datastore-core/api/public_plus_experimental_1.0.0-alpha01.txt
M datastore/datastore-core/api/public_plus_experimental_current.txt
A datastore/datastore-core/api/restricted_1.0.0-alpha01.txt
M datastore/datastore-core/api/restricted_current.txt
M datastore/datastore-core/build.gradle
D datastore/datastore-core/lint-baseline.xml
M datastore/datastore-core/src/main/java/androidx/datastore/core/CorruptionHandler.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/DataMigration.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/DataMigrationInitializer.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/DataStore.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/DataStoreFactory.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/InitializerApi.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/Serializer.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/SingleProcessDataStore.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/handlers/NoOpCorruptionHandler.kt
M datastore/datastore-core/src/main/java/androidx/datastore/core/handlers/ReplaceFileCorruptionHandler.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/DataMigrationInitializerTest.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/DataStoreFactoryTest.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/SingleProcessDataStoreTest.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/TestingSerializer.kt
M datastore/datastore-core/src/test/java/androidx/datastore/core/handlers/ReplaceFileCorruptionHandlerTest.kt
M datastore/datastore-preferences/api/api_lint.ignore
M datastore/datastore-preferences/api/current.txt
M datastore/datastore-preferences/api/public_plus_experimental_current.txt
M datastore/datastore-preferences/api/restricted_current.txt
M datastore/datastore-preferences/build.gradle
M datastore/datastore-preferences/datastore-preferences-proto/src/main/java/androidx/datastore/preferences/PreferencesMapCompat.kt
M datastore/datastore-preferences/src/androidTest/java/androidx/datastore/preferences/PreferenceDataStoreFactoryTest.kt
M datastore/datastore-preferences/src/androidTest/java/androidx/datastore/preferences/SharedPreferencesToPreferencesTest.kt
M datastore/datastore-preferences/src/main/java/androidx/datastore/preferences/PreferenceDataStoreFactory.kt
M datastore/datastore-preferences/src/main/java/androidx/datastore/preferences/Preferences.kt
M datastore/datastore-preferences/src/main/java/androidx/datastore/preferences/PreferencesSerializer.kt
M datastore/datastore-preferences/src/test/java/androidx/datastore/preferences/PreferencesSerializerTest.kt
M datastore/datastore-proto/src/main/java/androidx/datastore/protos/ProtoSerializer.kt
M datastore/datastore-proto/src/test/java/androidx/datastore/protos/ProtoSerializerTest.kt
M datastore/datastore-sampleapp/build.gradle
M datastore/datastore-sampleapp/src/main/java/com/example/datastoresampleapp/PreferencesDataStoreActivity.kt
M datastore/datastore-sampleapp/src/main/java/com/example/datastoresampleapp/ProtoDataStoreActivity.kt
M datastore/datastore-sampleapp/src/main/java/com/example/datastoresampleapp/SettingsFragment.kt
M datastore/datastore/api/api_lint.ignore
A datastore/datastore/api/current.txt
A datastore/datastore/api/public_plus_experimental_current.txt
A datastore/datastore/api/res-current.txt
A datastore/datastore/api/restricted_current.txt
A datastore/datastore/build.gradle
M datastore/datastore/src/androidTest/AndroidManifest.xml
A datastore/datastore/src/androidTest/java/androidx/datastore/DataStoreFactoryTest.kt
M datastore/datastore/src/androidTest/java/androidx/datastore/TestingSerializer.kt
M datastore/datastore/src/androidTest/java/androidx/datastore/migrations/SharedPreferencesMigrationTest.kt
M datastore/datastore/src/main/AndroidManifest.xml
A datastore/datastore/src/main/java/androidx/datastore/DataStoreFactory.kt
M datastore/datastore/src/main/java/androidx/datastore/migrations/SharedPreferencesMigration.kt
M settings.gradle

https://android-review.googlesource.com/1441928

il...@google.com <il...@google.com> #6Apr 19, 2019 03:31PM

This should be available in the next release.

Issue 129593351