IssueTracker

Yigit, do you have time to fix it?

reemission of the same liveData is racy

yea i'll take it.

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.

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

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)

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

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

We would like to investigate as well. Do you have a sample project that reproduces the issues that you can upload as a part of this bug?

Here it goes:

- in order to avoid dealing with initial boilerplate, cloned https://github.com/googlecodelabs/android-navigation;
- uncomented TODO from the codelab steps;
- updated deps, plugins versions, fixed some compiling issue;
- removed code, left Home and DeepLinkFragment only;
- changed app:uri to

  "www.example.com/query?param1={param1}&param2={param2}";

- added arguments in deepLinkFrangment in mobile_navigation.xml:

        <argument
            android:name="param1"
            android:defaultValue="@null"
            app:nullable="true"/>

        <argument
            android:name="param2"
            android:defaultValue="@null"
            app:nullable="true"/>


- added action in homeFragment targeting deepLinkFragment

        <action
            android:id="@+id/deeplink_action"
            app:destination="@+id/deeplink_dest" />


- added a button and the following code to HomeFragment, using action generated code (notice that no argument is passed because they default to null):
        view.findViewById<Button>(R.id.navigate_direct_button)?.setOnClickListener {
            findNavController().navigate(HomeFragmentDirections.deeplinkAction(param1 = "HELLO"))
        }

- added a button and the following code to HomeFragment, using Uri (notice that param2 is absent):
        view.findViewById<Button>(R.id.navigate_deeplink_button)?.setOnClickListener {
            findNavController().navigate(
                Uri.parse("example://www.example.com/query?param1=HELLO"))
        }

(had to change app:uri from "www.example.com" to "example://www.example.com/..." because the first wasn't matching (another issue i guess))

- adjusted DeepLinkFragment to show param1 and param2, from the arguments bundle (could have used by navArgs with same result)

        textView.text = arguments?.getString("param1", "") + arguments?.getString("param2", "")

Running the app, when the "navigate with direct action" button is clicked, "HELLO" is shown in the next fragment, as expected. When "navigate with deeplink" is clicked, "HELLO@null" is shown, which is the issue.

I'm facing this issue as well, default null seems never be null.
The root cause seems in the androidx.navigation.NavDeepLink.java:

// Missing parameter so see if it has a default value or is Nullable
if (argument != null
    && (value == null || value.replaceAll("[{}]", "").equals(argName))) {
    if (argument.getDefaultValue() != null) {
        value = argument.getDefaultValue().toString();
    } else if (argument.isNullable()) {
        value = "@null";
    }
}

+1
arguments?.run {
            val args by navArgs<BFragmentArgs>()
            //Should be null instead "@null"?
            Log.d("ARGS", "Token ${args.token}")
        }

It would be helpful if defaultValue is really null, instead "@null" on deeplink cases like com.sample://fragmentB?token= (that we could receive by mistake from backend/external service)

<argument
            android:name="token"
            android:defaultValue="@null"
            app:argType="string"
            app:nullable="true" />
        <deepLink
            android:id="@+id/cta"
            app:uri="com.sample://fragmentB?token={token}" />

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

commit 6e93d1d7ee8ac63451e8822422fb3b1da90fa321
Author: Jeremy Woods <jbwoods@google.com>
Date:   Mon Nov 25 15:29:59 2019

    Make nullable deep link params default to null
   
    Currently a deep link param that cannot be matched but is marked as
    nullable will be stored in the argument bundle with a value of "@null".
    This causes the behavior for a missing param to be inconsistent between
    a normal navigation and navigation from a deep link.
   
    This change stores null an instance of null in the bundle for the value
    instead to ensure we mimic the behavior of non-deep link nullable
    params.
   
    Test: adjusted NavDeepLinkTest
    BUG: 141613546
    Change-Id: I14625699fc9365088e07df9456a05257974a375b

M       navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M       navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.java

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

This has been fixed internally and will be available in the Navigation 2.2.0-rc03 release.

  The following changes were cherrypicked through Coastguard (https://android-build.googleplex.com/coastguard/)
  Release Track: (https://android-build.googleplex.com/coastguard/dashboard/6205454677704704/#/history/all)
  Created: 2019-11-26 03:40:40.026747+00:00
  Changes: aosp/1174705