Skip to content

Commit 6743801

Browse files
committed
Avoid potential StackOverflow on wrapped UnconfinedDispatcher+yield
See ImmediateYieldTest.testWrappedUnconfinedDispatcherYieldStackOverflow This commit changes the way Unconfined dispatcher is detected. Instead of equality check we try to dispatch to the dispatcher with a specially updated coroutine context that has YieldContext element that is processed by the Unconfied dispatcher in a special way.
1 parent 91b70eb commit 6743801

File tree

7 files changed

+54
-11
lines changed

7 files changed

+54
-11
lines changed

kotlinx-coroutines-core/common/src/AbstractCoroutine.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public abstract class AbstractCoroutine<in T>(
133133
*
134134
* * [DEFAULT] uses [startCoroutineCancellable].
135135
* * [ATOMIC] uses [startCoroutine].
136-
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
136+
* * [UNCONFINED] uses [startCoroutineUndispatched].
137137
* * [LAZY] does nothing.
138138
*/
139139
public fun start(start: CoroutineStart, block: suspend () -> T) {
@@ -150,7 +150,7 @@ public abstract class AbstractCoroutine<in T>(
150150
*
151151
* * [DEFAULT] uses [startCoroutineCancellable].
152152
* * [ATOMIC] uses [startCoroutine].
153-
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
153+
* * [UNCONFINED] uses [startCoroutineUndispatched].
154154
* * [LAZY] does nothing.
155155
*/
156156
public fun <R> start(start: CoroutineStart, receiver: R, block: suspend R.() -> T) {

kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt

+4
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ public abstract class CoroutineDispatcher :
7676
*
7777
* This method should generally be exception-safe. An exception thrown from this method
7878
* may leave the coroutines that use this dispatcher in the inconsistent and hard to debug state.
79+
*
80+
* **Note**: This method must not immediately call [block]. Doing so would result in [StackOverflowError]
81+
* when [yield] is repeatedly called from a loop. However, an implementation can delegate this function
82+
* to `dispatch` method of [Dispatchers.Unconfined], which is integrated with [yield] to avoid this problem.
7983
*/
8084
public abstract fun dispatch(context: CoroutineContext, block: Runnable)
8185

kotlinx-coroutines-core/common/src/CoroutineStart.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public enum class CoroutineStart {
2727
* invoked the coroutine builder continues execution.
2828
*
2929
* Note that [Dispatchers.Unconfined] always returns `false` from its [CoroutineDispatcher.isDispatchNeeded]
30-
* function, so starting a coroutine with [Dispatchers.Unconfined] by [DEFAULT] is the same as using [UNDISPATCHED].
30+
* function, so starting a coroutine with [Dispatchers.Unconfined] by [DEFAULT] is the same as using [UNCONFINED].
3131
*
3232
* If coroutine [Job] is cancelled before it even had a chance to start executing, then it will not start its
3333
* execution at all, but will complete with an exception.
@@ -79,7 +79,7 @@ public enum class CoroutineStart {
7979
*
8080
* * [DEFAULT] uses [startCoroutineCancellable].
8181
* * [ATOMIC] uses [startCoroutine].
82-
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
82+
* * [UNCONFINED] uses [startCoroutineUndispatched].
8383
* * [LAZY] does nothing.
8484
*
8585
* @suppress **This an internal API and should not be used from general code.**
@@ -98,7 +98,7 @@ public enum class CoroutineStart {
9898
*
9999
* * [DEFAULT] uses [startCoroutineCancellable].
100100
* * [ATOMIC] uses [startCoroutine].
101-
* * [UNDISPATCHED] uses [startCoroutineUndispatched].
101+
* * [UNCONFINED] uses [startCoroutineUndispatched].
102102
* * [LAZY] does nothing.
103103
*
104104
* @suppress **This an internal API and should not be used from general code.**

kotlinx-coroutines-core/common/src/Unconfined.kt

+24-2
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,35 @@
55
package kotlinx.coroutines
66

77
import kotlin.coroutines.*
8+
import kotlin.jvm.*
89

910
/**
1011
* A coroutine dispatcher that is not confined to any specific thread.
1112
*/
1213
internal object Unconfined : CoroutineDispatcher() {
1314
override fun isDispatchNeeded(context: CoroutineContext): Boolean = false
14-
// Just in case somebody wraps Unconfined dispatcher casing the "dispatch" to be called from "yield"
15-
override fun dispatch(context: CoroutineContext, block: Runnable) = block.run()
15+
16+
override fun dispatch(context: CoroutineContext, block: Runnable) {
17+
// Just in case somebody wraps Unconfined dispatcher casing the "dispatch" to be called from "yield"
18+
// See also code of "yield" function
19+
val yieldContext = context[YieldContext]
20+
if (yieldContext != null) {
21+
// report to "yield" that it is an unconfined dispatcher and don't call "block.run()"
22+
yieldContext.dispatcherWasUnconfined = true
23+
return
24+
}
25+
block.run()
26+
}
27+
1628
override fun toString(): String = "Unconfined"
1729
}
30+
31+
/**
32+
* Used to detect calls to [Unconfined.dispatch] from [yield] function.
33+
*/
34+
internal class YieldContext : AbstractCoroutineContextElement(Key) {
35+
companion object Key : CoroutineContext.Key<YieldContext>
36+
37+
@JvmField
38+
var dispatcherWasUnconfined = false
39+
}

kotlinx-coroutines-core/common/src/Yield.kt

+9-2
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,16 @@ public suspend fun yield(): Unit = suspendCoroutineUninterceptedOrReturn sc@ { u
2727
val context = uCont.context
2828
context.checkCompletion()
2929
val cont = uCont.intercepted() as? DispatchedContinuation<Unit> ?: return@sc Unit
30+
// This code detects the Unconfined dispatcher even if it was wrapped into another dispatcher
31+
val yieldContext = YieldContext()
32+
cont.dispatchYield(context + yieldContext, Unit)
3033
// Special case for the unconfined dispatcher that can yield only in existing unconfined loop
31-
if (cont.dispatcher === Unconfined) return@sc if (cont.yieldUndispatched()) COROUTINE_SUSPENDED else Unit
32-
cont.dispatchYield(Unit)
34+
if (yieldContext.dispatcherWasUnconfined) {
35+
// Means that the Unconfined dispatcher got the call, but did not do anything.
36+
// See also code of "Unconfined.dispatch" function.
37+
return@sc if (cont.yieldUndispatched()) COROUTINE_SUSPENDED else Unit
38+
}
39+
// It was some other dispatcher that successfully dispatched the coroutine
3340
COROUTINE_SUSPENDED
3441
}
3542

kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt

+1-2
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,7 @@ internal class DispatchedContinuation<in T>(
211211
}
212212

213213
// used by "yield" implementation
214-
internal fun dispatchYield(value: T) {
215-
val context = continuation.context
214+
internal fun dispatchYield(context: CoroutineContext, value: T) {
216215
_state = value
217216
resumeMode = MODE_CANCELLABLE
218217
dispatcher.dispatchYield(context, this)

kotlinx-coroutines-core/common/test/ImmediateYieldTest.kt

+11
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,15 @@ class ImmediateYieldTest : TestBase() {
4343
}
4444
finish(4) // after launch
4545
}
46+
47+
@Test
48+
fun testWrappedUnconfinedDispatcherYieldStackOverflow() = runTest {
49+
expect(1)
50+
withContext(wrapperDispatcher(Dispatchers.Unconfined)) {
51+
repeat(100_000) {
52+
yield()
53+
}
54+
}
55+
finish(2)
56+
}
4657
}

0 commit comments

Comments
 (0)