Fixed
Status Update
Comments
il...@google.com <il...@google.com> #2
Yigit, do you have time to fix it?
reemission of the same liveData is racy
reemission of the same liveData is racy
ja...@gmail.com <ja...@gmail.com> #3
yea i'll take it.
ub...@gmail.com <ub...@gmail.com> #4
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.
ja...@gmail.com <ja...@gmail.com> #5
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()
}
}
@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}
ub...@gmail.com <ub...@gmail.com> #6
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)
ja...@gmail.com <ja...@gmail.com> #7
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} )
}
I actually have a WIP fix for it:
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} )
}
il...@google.com <il...@google.com>
al...@google.com <al...@google.com> #8
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
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
se...@google.com <se...@google.com> #9
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.
ap...@google.com <ap...@google.com> #10
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
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
il...@google.com <il...@google.com> #11
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.
@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.
zh...@gmail.com <zh...@gmail.com> #12
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
?
------
Is it to deliberately kill code such as:
and
?
il...@google.com <il...@google.com> #13
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
ta...@gmail.com <ta...@gmail.com> #14
I've seen in recent changelog that giving layout in constructors is also supported in AppCompatActivity. Is this something supported in older API levels?
il...@google.com <il...@google.com> #15
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."
"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."
Description
Version used: Version 1.0.0-alpha04
Since `R.layout` is non-final in library projects, we can't use the `@ContentView` annotation.