Navigation crashes when navigating from the initial fragment of the included graph when popUpTo is used and the destination is another included (and nested graph). [243778589]

Fixed

Bug

Status Update

No update yet.

Description

kr...@gmail.com

created issue #1

Aug 25, 2022 02:57PM

Component used: Navigation
Version used: every version since 2.4.0 (latest tested version is 2.5.1)
Latest version working correctly: 2.3.5
Devices/Android versions reproduced on: Emulator API 30, Oppo CPH2251 API 30, Samsung Galaxy S21 API 31

Repro steps:
1. Define the following 3 graphs (sample project is attached to this bug report):

<?xml version="1.0" encoding="utf-8"?>
<navigation xmlns:android="

http://schemas.android.com/apk/res/android"
xmlns:app="

http://schemas.android.com/apk/res-auto"
xmlns:tools="

http://schemas.android.com/tools"
android:id="@+id/level_0_graph"
app:startDestination="@id/level_0_fragment">

<fragment
android:id="@+id/level_0_fragment"
android:name="com.example.nestedpopupto.Level0Fragment"
tools:layout="@layout/fragment_level_0">

<action
android:id="@+id/to_level_1"
app:popUpTo="@id/level_0_graph"
app:destination="@id/level_1" />
</fragment>

<navigation
android:id="@+id/level_1"
app:startDestination="@id/level_1_graph">

<include app:graph="@navigation/level_1_graph" />
</navigation>

</navigation>

<?xml version="1.0" encoding="utf-8"?>
<navigation xmlns:android="

http://schemas.android.com/apk/res/android"
xmlns:app="

http://schemas.android.com/apk/res-auto"
xmlns:tools="

http://schemas.android.com/tools"
android:id="@+id/level_1_graph"
app:startDestination="@id/level_1_fragment">

<fragment
android:id="@+id/level_1_fragment"
android:name="com.example.nestedpopupto.Level1Fragment"
tools:layout="@layout/fragment_level_1">

<action
android:id="@+id/to_level_2"
app:popUpTo="@id/level_1_graph"
app:destination="@id/level_2" />
</fragment>

<navigation
android:id="@+id/level_2"
app:startDestination="@id/level_2_graph">

<include app:graph="@navigation/level_2_graph" />
</navigation>

</navigation>

<navigation xmlns:android="

http://schemas.android.com/apk/res/android"
xmlns:app="

http://schemas.android.com/apk/res-auto"
xmlns:tools="

http://schemas.android.com/tools"
android:id="@+id/level_2_graph"
app:startDestination="@id/level_2_fragment">

<fragment
android:id="@+id/level_2_fragment"
android:name="com.example.nestedpopupto.Level2Fragment"
tools:layout="@layout/fragment_level_2">
</fragment>

</navigation>

2. Use "to_level_1" to navigate to nested graph.
3. Use "to_level_2" action.

Result:
The action "to_level_2" will fail with the following stacktrace:

java.lang.IllegalArgumentException: No destination with ID 2131230965 is on the NavController's back stack. The current destination is Destination(com.example.nestedpopupto:id/level_2_fragment) class=com.example.nestedpopupto.Level2Fragment
at androidx.navigation.NavController.getBackStackEntry(NavController.kt:2209)
at androidx.navigation.NavController.addEntryToBackStack(NavController.kt:1918)
at androidx.navigation.NavController.addEntryToBackStack$default(NavController.kt:1813)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1721)
at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1719)
at androidx.navigation.NavController$NavControllerNavigatorState.push(NavController.kt:287)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:246)
at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:162)
at androidx.navigation.NavGraphNavigator.navigate(NavGraphNavigator.kt:83)
at androidx.navigation.NavGraphNavigator.navigate(NavGraphNavigator.kt:49)
at androidx.navigation.NavGraphNavigator.navigate(NavGraphNavigator.kt:83)
at androidx.navigation.NavGraphNavigator.navigate(NavGraphNavigator.kt:49)
at androidx.navigation.NavController.navigateInternal(NavController.kt:260)
at androidx.navigation.NavController.navigate(NavController.kt:1719)
at androidx.navigation.NavController.navigate(NavController.kt:1545)
at androidx.navigation.NavController.navigate(NavController.kt:1472)
at androidx.navigation.NavController.navigate(NavController.kt:1930)
at com.example.nestedpopupto.Level1Fragment.onViewCreated$lambda-0(Level1Fragment.kt:13)

The crash will NOT occur if any of the following is applied:
- the "to_level_2" action does not use "popUpTo"
- the "level_2_graph" is not wrapped with the <navigation>.

As one can see the "to_level_1" action is identical to the "to_level_2" action and is defined in an identical graph and it works correctly. It seems that the bug occurs only when navigation is performed in the graph that is included in an other graph.

In the end I would like to remind that everything works correctly with version 2.3.5 of the androidx.navigation.

nested-popupto.7z

22 KB

Download

Comments

jb...@google.com <jb...@google.com> #2Aug 25, 2022 07:38PM

Status: Won't Fix (Intended Behavior)

Yigit, do you have time to fix it?

reemission of the same liveData is racy

kr...@gmail.com <kr...@gmail.com> #3Aug 26, 2022 08:14AM

yea i'll take it.

kr...@gmail.com <kr...@gmail.com> #4Aug 26, 2022 08:19AM

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.

kr...@gmail.com <kr...@gmail.com> #5Aug 26, 2022 08:31AM

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()
}
}

kr...@gmail.com <kr...@gmail.com> #6Aug 26, 2022 09:06AM

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)

jb...@google.com <jb...@google.com> #7Aug 26, 2022 05:47PM

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

kr...@gmail.com <kr...@gmail.com> #8Aug 29, 2022 11:05AM

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

jb...@google.com <jb...@google.com> Aug 30, 2022 02:30AM

Status: Assigned (reopened)

ap...@google.com <ap...@google.com> #9Aug 30, 2022 03:53PM

Project: platform/frameworks/support
Branch: androidx-main

commit fec95ed9a91ef4058d5265e2f7ba1240db4f651d
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Aug 29 19:09:30 2022

Fix double nested graph popUpTo behavior

Navigation should always maintain the path to current destination as
part of that back stack. Under normal circumstances following the
recommended behavior this is true.

It is possible to create a scenario where this is not the case.

Given the following graph:

A
|
B
/ \
C D
|
E
|
F

Where C is the start destination of graph B, if you navigate to D while
popping C off the back stack (which is not recommended), Navigation
fails to build the proper hierarchy for D resulting in B being popped
incorrectly and causing an IllegalStateException.

We should ensure that Navigation properly handles nested destinations.

RelNote: "Fixed `IllegalStateException` caused by navigating to a double
nested graph that shares a parent with a new popped start destination."
Test: added testLifecycleDoubleNestedGraph
Bug: 243778589

Change-Id: I9f7cb2445e6021c6ab4cb81d62612411249c1257

M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavBackStackEntryLifecycleTest.kt

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

jb...@google.com <jb...@google.com> #10Aug 30, 2022 04:55PM

Marked as fixed.

I was able to find that is this particular case Navigation is indeed losing the proper back stack.

Again, this is not something we recommend doing when using Navigation as the startDestination is meant to be the entry and exit point of a particular graph. If this is something that the app needs, it might be a sign that the architecture needs to be re-evaluated.

This has been fixed and will be a part of the Navigation 2.5.2 and 2.6.0-alpha01 releases.

kr...@gmail.com <kr...@gmail.com> #11Sep 1, 2022 07:47AM

Thank you! Have a nice day :)

na...@google.com <na...@google.com> #12Sep 12, 2022 05:52PM

This bug was linked in a change in the following release(s):

androidx.navigation:navigation-runtime:2.6.0-alpha01
androidx.navigation:navigation-runtime:2.5.2

Issue 243778589

Description

Issue summary

Comments

jb...@google.com <jb...@google.com> #2Aug 25, 2022 07:38PM

kr...@gmail.com <kr...@gmail.com> #3Aug 26, 2022 08:14AM

kr...@gmail.com <kr...@gmail.com> #4Aug 26, 2022 08:19AM

kr...@gmail.com <kr...@gmail.com> #5Aug 26, 2022 08:31AM

kr...@gmail.com <kr...@gmail.com> #6Aug 26, 2022 09:06AM

jb...@google.com <jb...@google.com> #7Aug 26, 2022 05:47PM

kr...@gmail.com <kr...@gmail.com> #8Aug 29, 2022 11:05AM

jb...@google.com <jb...@google.com> Aug 30, 2022 02:30AM

ap...@google.com <ap...@google.com> #9Aug 30, 2022 03:53PM

jb...@google.com <jb...@google.com> #10Aug 30, 2022 04:55PM

kr...@gmail.com <kr...@gmail.com> #11Sep 1, 2022 07:47AM

na...@google.com <na...@google.com> #12Sep 12, 2022 05:52PM

Add comment

Issue metadata