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

There is aosp/931676 inflight, that doesn't have this issue, but need to check if tools team can create integration with it, it looks good.

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

commit 2f634eec2143acd6a4ea19a375a6e3877cdcc2ed
Author: Ian Lake <ilake@google.com>
Date:   Tue Mar 19 10:14:07 2019

    Convert @ContentView to constructor annotation
   
    As layout IDs are non-final in library modules
    (and in the future, app modules), change
    @ContentView to only be a marker annotation for
    a constructor which developers can call with
    a specific layout ID.
   
    Fixes: 128352521
    Fixes: 127531658
    Test: updated tests
    Change-Id: I15e2edee8cbd68180991f89fbc3b04e12c961ede

M       activity/api/1.0.0-alpha06.txt
M       activity/api/current.txt
M       activity/src/androidTest/java/androidx/activity/ContentViewTest.kt
M       activity/src/main/java/androidx/activity/ComponentActivity.java
M       annotations/api/1.1.0-beta01.txt
M       annotations/api/current.txt
M       annotations/src/main/java/androidx/annotation/ContentView.java
M       appcompat/api/1.1.0-alpha04.txt
M       appcompat/api/current.txt
M       appcompat/src/main/java/androidx/appcompat/app/AppCompatActivity.java
M       fragment/api/1.1.0-alpha06.txt
M       fragment/api/current.txt
M       fragment/src/androidTest/java/androidx/fragment/app/FragmentFactoryTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/FragmentLifecycleTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/FragmentTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/FragmentTransactionTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/FragmentViewTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/NestedInflatedFragmentTest.kt
M       fragment/src/androidTest/java/androidx/fragment/app/StrictFragment.kt
M       fragment/src/androidTest/java/androidx/fragment/app/StrictViewFragment.kt
M       fragment/src/androidTest/java/androidx/fragment/app/test/FragmentTestActivity.kt
M       fragment/src/androidTest/java/androidx/fragment/app/test/LoaderActivity.kt
M       fragment/src/main/java/androidx/fragment/app/Fragment.java
M       fragment/src/main/java/androidx/fragment/app/FragmentActivity.java
M       testutils/src/main/java/androidx/testutils/RecreatedActivity.kt

https://android-review.googlesource.com/931676
https://goto.google.com/android-sha1/2f634eec2143acd6a4ea19a375a6e3877cdcc2ed

We've changed how @ContentView functions. The previous form of:

@ContentView(R.layout.main)
class MainFragment : Fragment() {
}

becomes

class MainFragment : Fragment(R.layout.main) {
}

While the default FragmentFactory (and AppComponentFactory for Activities on API 28+) still require a no-argument constructor, Fragment and ComponentActivity/FragmentActivity/AppCompatActivity allow your no-argument constructor or custom constructor to call up to a second constructor which takes a @LayoutRes int of the layout you want to inflate. That second constructor is annotated with @ContentView and will be used for future Tools integrations.

Layout IDs will become non-usable in annotations, in application modules?

------

Is it to deliberately kill code such as:

https://github.com/square/mortar/blob/d7bb8e5b5dd773376d940319a49bce9bf7c1058c/mortar-sample/src/main/java/com/example/mortar/screen/ChatListScreen.java#L33

and

https://github.com/lyft/scoop/blob/8d8e499bf3e6399676d314a18094a99a57b99a46/scoop-basics/src/main/java/com/example/scoop/basics/ui/layoutsample/screen/LayoutScreen.java#L10

?

For discussion on non-final IDs in app modules and why the Tools team is considering it, see the section on resource namespacing part of the "What's New With the Android Gradle Plugin" talk from AndroidDevSummit: https://www.youtube.com/watch?v=GlwvVJNWlWg&t=1318

I've seen in recent changelog that giving layout in constructors is also supported in AppCompatActivity. Is this something supported in older API levels?

Re #14 as per the Javadoc: https://developer.android.com/reference/androidx/appcompat/app/AppCompatActivity?hl=en#AppCompatActivity(int)

"This should generally be called from your constructor that takes no parameters, as is required for API 27 and lower or when using the default AppComponentFactory."