Status Update
No update yet.
Comments
du...@google.com
reemission of the same liveData is racy
lo...@gmail.com
du...@google.com
ap...@google.com
@Test
fun raceTest() {
val subLiveData = MutableLiveData(1)
val subject = liveData(testScope.coroutineContext) {
emitSource(subLiveData)
emitSource(subLiveData) //crashes
}
subject.addObserver().apply {
testScope.advanceUntilIdle()
}
}
lo...@gmail.com
du...@google.com
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} )
}
lo...@gmail.com
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
du...@google.com
While I agree, part of the issue is that wrapping withContext(Disaptcher.Default) { } will also make it difficult to fast-forward / control timing when used with test scope / pauseDispatcher from kotlinx-coroutines-test. There isn't a good way for Paging to forward the fetchDispatcher to the transforms to provide the default, since the only Paging3 APIs that actually accept a bg / fetchDispatcher are in the paging-runtime adapter / differ APIs.
lo...@gmail.com
I think having a jank-free app by default is mort important than timing control in testing API, as users will ultimately only experience the former.
Now, I think there's a solution to satisfy both use cases:
Have a context: CoroutineContext parameter that defaults to Dispatchers.Default, which allows people to replace it with EmptyCoroutineContext, or whatever they want (including Dispatchers.Main if they need to sync something on the main thread).
du...@google.com
I think having a jank-free app by default is mort important than timing control in testing API, as users will ultimately only experience the former.
I agree, except in this case we would make it impossible for it to be testable, so the trade-off is not great here.
Now, I think there's a solution to satisfy both use cases: Have a context: CoroutineContext parameter that defaults to Dispatchers.Default, which allows people to replace it with EmptyCoroutineContext, or whatever they want (including Dispatchers.Main if they need to sync something on the main thread).
Adding a param seems not great either. Also what happens if developer wants to use .flowOn {} etc to change the executing context? I think generally Flow operators are not supposed to accept a CoroutineContext, but I could be wrong here. Certainly something to think about though.. I think the real solution here is to try to default .submitData to collect on fetchDispatcher, then report differ events through mainDispatcher, but I haven't tried yet so am not sure how feasible it is.
lo...@gmail.com
If EmptyCoroutineContext is used, flowOn will work as expected.
I agree it'd be best to have the collector collect in the right dispatcher. I'm curious if there's any challenges that poses.
du...@google.com
Right, I meant that it is surprising to need to pass in EmptyCoroutineContext to get .flowOn to work.
Description
Hello,
In the
PagingDataclass, there'sfilterSyncand other functions that accept a blocking lambda, but there's no information in the KDoc about which dispatcher/thread they are called on.I guess they are called on
Dispatchers.Default… is it the case?If so, I'd submit a PR on GitHub for this change on
filterSyncand friends.