-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chaining rxSingle
calls that use Unconfined
dispatcher and blockingGet
results in deadlock
#3458
Comments
ExplanationThe example can be simplified a lot: rxSingle(Dispatchers.Unconfined) {
}.blockingGet()
println("the top half is OK")
runBlocking(Dispatchers.Unconfined) {
rxSingle(Dispatchers.Unconfined) {
}.blockingGet()
} So, doing For example, this test will immediately pass, but without this implementation detail of @Test
fun nestedLaunches() {
fun CoroutineScope.launchNested(remainingLevels: Int): Job? {
return if (remainingLevels > 0) {
launch {
launchNested(remainingLevels - 1)?.join()
}
} else {
null
}
}
runBlocking(Dispatchers.Unconfined) {
launchNested(100000)?.join()
}
} Now, here's how this causes the issue. The way On the other hand, when WorkaroundReplacing /**
* A dispatcher that executes the computations immediately in the thread that requests it.
*
* This dispatcher is similar to [Dispatchers.Unconfined] but does not attempt to avoid stack overflows.
*/
object DirectDispatcher : CoroutineDispatcher() {
override fun dispatch(context: CoroutineContext, block: Runnable) {
block.run()
}
} I don't know if RxJava actually does handle nesting in its call chains in this manner, but initially, it does seem so. |
@dkhalanskyjb Thanks for your response! I'm a little curious if you think using something like the |
Certainly, the risk is back. Replacing That said, I think this issue is inherent to RxJava: ReactiveX/RxJava#6958 does look like the kind of an issue that we're solving with the event loop, and https://stackoverflow.com/questions/42363452/rxjava-stackoverflowerror-exception-in-long-chain does, too. The only question is, does |
OK that makes sense. I will do some more testing and digging into what I'd expect for RxJava here and decide which of the following risks is the one I'm most willing to tolerate:
Thanks again for your help! |
Please do share the results if you discover anything interesting! |
Will do! |
OK so here's what I found from some additional testing: Summary
So overall I haven't really seen any practical downsides to using
DetailsFor my tests of potential RxJava onlyConsider the following recursively constructed call chain: val outerScheduler: Scheduler = ...
fun mockRequest(): Single<Unit> = ...
fun buildChain(
current: Int,
max: Int = 10_000
): Single<Unit> = mockRequest()
.flatMap {
if (current < max) {
buildChain(current = current + 1)
} else {
mockRequest()
}
}
buildChain(current = 0)
.subscribeOn(outerScheduler)
.blockingGet() The details of the behavior here depends in part on the implementation of When fun mockRequest(): Single<Unit> =
Single.fromCallable { } The chain above either results in an explicit These issues go away, though, if fun mockRequest(): Single<Unit> =
Single.fromCallable { }.subscribeOn(innerScheduler) In this case, the call chain succeeds no matter what is used for Coroutine wrapperNow we can compare this against the behavior of fun mockRequest(): Single<Unit> =
rxSingle(DirectDispatcher) {} all tests fail with a When considering the case where the wrapped call itself uses its own fun mockRequest(): Single<Unit> =
rxSingle(DirectDispatcher) {
withContext(innerDispatcher) {}
} we get a similar result to the second RxJava-only test: when using an So in both types of cases here,
Plus, when considering that the wrapped calls would have some |
Consider the follow setup in which two
rxSingle
calls are chained together and the second one usesblockingGet()
:(Set aside that
blockingGet()
is not a good practical choice here...this is just to highlight the problem.)The above triggers the timeout because the
blockingGet()
call is deadlocked:This happens even if the suspending code inside
rxSingle
operates with a different dispatcher:The code completes successfully and prints
"Success"
if eitherdispatcher1
ordispatcher2
is set toDispatchers.Default
/Dispatchers.IO
etc. so it appears that the use ofUnconfined
on both is key to the issue.My main question is: is this expected? If so, why does it require both requests to use
Unconfined
to see the issue? Is this because of the "event-loop" that gets involved here for nested coroutines:My understanding is that this was actually introduced in #860 to avoid deadlocks when using nested
runBlocking
calls. Is it possible the conversion from suspending functions toSingle
/Completable
/ etc. unintentionally allows this problem to occur again when usingSingle.blockingGet()
?For a little more context, the practical consideration here is that we have a networking layer that exposes suspend functions and then an RxJava wrapper around that network layer for callers who prefer to use RxJava. The calls are set up like the following:
The reasoning behind
Dispatchers.Unconfined
here is twofold:suspendingNetworkCall
already manages its own threading).These ideas were similarly discussed in #2925 (comment) . Setting up our RxJava wrapper layer in this manner leaves us susceptible to the deadlocks discussed above, however, if any callers use
blockingGet()
in a network request chain for any reason. Is this just expected behavior and we should avoid usingDispatchers.Unconfined
in this way? I see there has been some discussion about a hypothetical dispatcher that works likeUnconfined
and could be used in these kinds of cases : #2485 (comment)Kotlin version :
1.5.21
coroutines-core / coroutines-rx2 version :
1.5.1
RxJava version :
2.2.19
The text was updated successfully, but these errors were encountered: