Illegal state on fragment operation triggered from LiveData onChaned [77944637]

Fixed

Bug

[AOSP] assigned

Status Update

No update yet.

Description

ni...@gmail.com

created issue #1

Apr 12, 2018 10:41PM

Component used: livedata
Version used:1.1.1
Devices/Android versions reproduced on: Android 4.x and 5.x

This is not something I can reproduce, but I'm seeing many errors in Crashlytics.

LiveData is supposed to notify changes only when it's safe to do fragment transactions, so this should not be happening, if I understand it correctly.

This is one of the exceptions I'm seeing:

java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
android.support.v4.app.FragmentManagerImpl.checkStateLoss (FragmentManager.java:2055)
android.support.v4.app.FragmentManagerImpl.enqueueAction (FragmentManager.java:2078)
android.support.v4.app.BackStackRecord.commitInternal (BackStackRecord.java:678)
android.support.v4.app.BackStackRecord.commit (BackStackRecord.java:632)
android.support.v4.app.DialogFragment.show (DialogFragment.java:160)
ar.com.lichtmaier.antenas.Pagame.mostrar (Pagame.java:33)
ar.com.lichtmaier.antenas.AntenaActivity.esPro (AntenaActivity.java:708)
ar.com.lichtmaier.antenas.AntenaActivity.bridge$lambda$0$AntenaActivity (AntenaActivity.java)
ar.com.lichtmaier.antenas.AntenaActivity$$Lambda$0.onChanged (Unknown Source)
android.arch.lifecycle.LiveData.considerNotify (LiveData.java:109)
android.arch.lifecycle.LiveData.dispatchingValue (LiveData.java:121)
android.arch.lifecycle.LiveData.access$400 (LiveData.java:59)
android.arch.lifecycle.LiveData$ObserverWrapper.activeStateChanged (LiveData.java:416)
android.arch.lifecycle.LiveData$LifecycleBoundObserver.onStateChanged (LiveData.java:368)
android.arch.lifecycle.LifecycleRegistry$ObserverWithState.dispatchEvent (LifecycleRegistry.java:354)
android.arch.lifecycle.LifecycleRegistry.forwardPass (LifecycleRegistry.java:292)
android.arch.lifecycle.LifecycleRegistry.sync (LifecycleRegistry.java:332)
android.arch.lifecycle.LifecycleRegistry.moveToState (LifecycleRegistry.java:137)
android.arch.lifecycle.LifecycleRegistry.handleLifecycleEvent (LifecycleRegistry.java:123)
android.arch.lifecycle.ReportFragment.dispatch (ReportFragment.java:121)
android.arch.lifecycle.ReportFragment.onResume (ReportFragment.java:88)
android.app.Fragment.performResume (Fragment.java:2096)
android.app.FragmentManagerImpl.moveToState (FragmentManager.java:928)
android.app.FragmentManagerImpl.moveToState (FragmentManager.java:1067)
android.app.FragmentManagerImpl.moveToState (FragmentManager.java:1049)
android.app.FragmentManagerImpl.dispatchResume (FragmentManager.java:1879)
android.app.Activity.performResume (Activity.java:6128)
android.app.ActivityThread.performResumeActivity (ActivityThread.java:2995)
android.app.ActivityThread.handleResumeActivity (ActivityThread.java:3037)
android.app.ActivityThread$H.handleMessage (ActivityThread.java:1350)
android.os.Handler.dispatchMessage (Handler.java:102)
android.os.Looper.loop (Looper.java:135)
android.app.ActivityThread.main (ActivityThread.java:5290)
java.lang.reflect.Method.invoke (Method.java)
java.lang.reflect.Method.invoke (Method.java:372)
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:911)
com.android.internal.os.ZygoteInit.main (ZygoteInit.java:706)

Comments

yb...@google.com <yb...@google.com> #2Apr 17, 2018 01:52AM

Yigit, do you have time to fix it?

reemission of the same liveData is racy

yb...@google.com <yb...@google.com> #3Apr 17, 2018 01:54AM

Assigned to se...@google.com.

yea i'll take it.

ni...@gmail.com <ni...@gmail.com> #4Apr 17, 2018 02:39AM

Thanks for the detailed analysis. This may not be an issue anymore since we've started using Main.immediate there but I' not sure; I'll try to create a test case.

se...@google.com <se...@google.com> #5May 2, 2018 06:12PM

just emitting same live data reproduces the issue.

@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}

ni...@gmail.com <ni...@gmail.com> #6Aug 18, 2018 09:13PM

With 2.2.0-alpha04 (that use Main.immediate), the issue seems to be still there (I tested it by calling emitSource() twice, like your test case)

se...@google.com <se...@google.com> #7Aug 20, 2018 06:32PM

Status: Won't Fix (Intended Behavior)

yea sorry immediate does not fix it.
I actually have a WIP fix for it:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1112186

if your case is the one i found (emitting same LiveData multiple times, as shown in #5) you can work around it by adding a dummy transformation.

val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData.map {it })
emitSource(subLiveData.map {it} )
}

ni...@gmail.com <ni...@gmail.com> #8Aug 21, 2018 12:51AM

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

commit af12e75e6b4110f48e44ca121466943909de8f06
Author: Yigit Boyar <yboyar@google.com>
Date: Tue Sep 03 12:58:11 2019

Fix coroutine livedata race condition

This CL fixes a bug in liveData builder where emitting same
LiveData source twice would make it crash because the second
emission registry could possibly happen before first one is
removed as source.

We fix it by using a suspending dispose function. It does feel
a bit hacky but we cannot make DisposableHandle.dispose async
and we do not want to block there. This does not mean that there
is a problem if developer disposes it manually since our emit
functions take care of making sure it disposes (and there is
no other way to add source to the underlying MediatorLiveData)

Bug: 140249349
Test: BuildLiveDataTest#raceTest_*

Change-Id: I0b464c242a583da4669af195cf2504e2adc4de40

M lifecycle/lifecycle-livedata-ktx/api/2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/current.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/public_plus_experimental_current.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_2.2.0-alpha05.txt
M lifecycle/lifecycle-livedata-ktx/api/restricted_current.txt
M lifecycle/lifecycle-livedata-ktx/src/main/java/androidx/lifecycle/CoroutineLiveData.kt
M lifecycle/lifecycle-livedata-ktx/src/test/java/androidx/lifecycle/BuildLiveDataTest.kt

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

https://goto.google.com/android-sha1/af12e75e6b4110f48e44ca121466943909de8f06

se...@google.com <se...@google.com> #9Aug 21, 2018 12:53AM

Status: Assigned (reopened)

Ah sorry I misread your stacktrace.

se...@google.com <se...@google.com> #10Aug 21, 2018 01:11AM

+ Ian,

Ian, do you have any ideas why we may fail to run fragment transaction in onResume?

ni...@gmail.com <ni...@gmail.com> #11Aug 21, 2018 03:29AM

Yeah, some platform fragment things appear there in the stack, but that seems to be something the activity calls whether you use platform fragments or not. Thank you for reopening this.

se...@google.com <se...@google.com> #12Aug 22, 2018 02:14AM

Ok, I was able to reproduce it. The issue is FragmentActivity.onResume doesn't resume its fragments and allow transactions immediately.

ap...@google.com <ap...@google.com> #13Aug 24, 2018 10:48PM

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

commit 9823ca6d8057b27adb44c8ffd3f211191bb41963
Author: Sergey Vasilinets <sergeyv@google.com>
Date: Thu Aug 23 18:41:17 2018

Call noteStateNotSaved() in OnResume

bug: 77944637
Test: SafeTransactionInOnResumeTest
Change-Id: I13211e0fece639382c8e3e961a8c702519d1c051

M fragment/build.gradle
M fragment/src/androidTest/AndroidManifest.xml
A fragment/src/androidTest/java/androidx/fragment/app/SafeTransactionInOnResumeTest.kt
M fragment/src/androidTest/res/values/styles.xml
M fragment/src/main/java/androidx/fragment/app/FragmentActivity.java

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

https://goto.google.com/android-sha1/9823ca6d8057b27adb44c8ffd3f211191bb41963

se...@google.com <se...@google.com> #14Aug 27, 2018 09:08PM

Marked as fixed.

So it is fixed, however, I'm not sure in what release it will go in 1.0.0-rc03 or 1.1.0

ni...@gmail.com <ni...@gmail.com> #15Aug 28, 2018 02:51AM

Thanks a lot!

[Deleted User] <[Deleted User]> #16Sep 10, 2018 01:12PM

Am I correct that the bug will be fixed only for AndroidX? Does this mean that we need to migrate our codebase to AndroidX in order to rely on LiveData not lead to IllegalStateException: Can not perform this action after onSaveInstanceState?

ni...@gmail.com <ni...@gmail.com> #17Sep 12, 2018 07:17PM

This is the number one issue crashing my app. Can't the fix be released in a rc? Will I need to wait to 1.1.0? =/

zh...@gmail.com <zh...@gmail.com> #18Nov 6, 2018 08:56AM

Use `commitAllowingStateLoss()`, it is the only safe way to commit a fragment transaction.

il...@google.com <il...@google.com> #19Nov 6, 2018 01:57PM

This fix was included in the Fragment 1.1.0-alpha01 release:

https://developer.android.com/jetpack/androidx/androidx-rn#2018-nov-fragment