Skip to content

Commit 43d831f

Browse files
committed
Fixed dispatching logic of withTimeout (removed extra dispatch)
1 parent fadf8c8 commit 43d831f

File tree

2 files changed

+40
-15
lines changed

2 files changed

+40
-15
lines changed

kotlinx-coroutines-core/src/main/kotlin/kotlinx/coroutines/experimental/Scheduled.kt

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616

1717
package kotlinx.coroutines.experimental
1818

19+
import kotlinx.coroutines.experimental.intrinsics.startUndispatchedCoroutine
1920
import java.util.concurrent.ScheduledExecutorService
2021
import java.util.concurrent.ScheduledThreadPoolExecutor
2122
import java.util.concurrent.TimeUnit
23+
import kotlin.coroutines.experimental.Continuation
2224
import kotlin.coroutines.experimental.ContinuationInterceptor
23-
import kotlin.coroutines.experimental.startCoroutine
25+
import kotlin.coroutines.experimental.intrinsics.suspendCoroutineOrReturn
2426

2527
private val KEEP_ALIVE = java.lang.Long.getLong("kotlinx.coroutines.ScheduledExecutor.keepAlive", 1000L)
2628

@@ -70,25 +72,25 @@ internal fun scheduledExecutorShutdownNowAndRelease() {
7072
public suspend fun <T> withTimeout(time: Long, unit: TimeUnit = TimeUnit.MILLISECONDS, block: suspend () -> T): T {
7173
require(time >= 0) { "Timeout time $time cannot be negative" }
7274
if (time <= 0L) throw CancellationException("Timed out immediately")
73-
return suspendCancellableCoroutine sc@ { cont: CancellableContinuation<T> ->
75+
return suspendCoroutineOrReturn sc@ { delegate: Continuation<T> ->
7476
// schedule cancellation of this continuation on time
75-
val runnable = CancelTimedOutContinuationRunnable(time, unit, cont)
77+
val cont = TimeoutContinuation(time, unit, delegate)
7678
val delay = cont.context[ContinuationInterceptor] as? Delay
7779
if (delay != null)
78-
cont.disposeOnCompletion(delay.invokeOnTimeout(time, unit, runnable)) else
79-
cont.cancelFutureOnCompletion(scheduledExecutor.schedule(runnable, time, unit))
80-
// restart block in a separate coroutine using cancellable context of this continuation,
81-
block.startCoroutine(cont)
80+
cont.disposeOnCompletion(delay.invokeOnTimeout(time, unit, cont)) else
81+
cont.cancelFutureOnCompletion(scheduledExecutor.schedule(cont, time, unit))
82+
// restart block using cancellable context of this continuation,
83+
// however start it as undispatched coroutine, because we are already in the proper context
84+
block.startUndispatchedCoroutine(cont)
85+
cont.getResult()
8286
}
8387
}
8488

85-
private class CancelTimedOutContinuationRunnable(
89+
private class TimeoutContinuation<T>(
8690
private val time: Long,
8791
private val unit: TimeUnit,
88-
private val cont: CancellableContinuation<*>
89-
): Runnable {
90-
override fun run() {
91-
cont.cancel(CancellationException("Timed out waiting for $time $unit"))
92-
}
93-
override fun toString(): String = "CancelTimedOutContinuationRunnable[$time,$unit,$cont]"
92+
delegate: Continuation<T>
93+
) : CancellableContinuationImpl<T>(delegate, active = true), Runnable {
94+
override fun defaultResumeMode(): Int = MODE_DIRECT
95+
override fun run() { cancel(CancellationException("Timed out waiting for $time $unit")) }
9496
}

kotlinx-coroutines-core/src/test/kotlin/kotlinx/coroutines/experimental/RunBlockingTest.kt renamed to kotlinx-coroutines-core/src/test/kotlin/kotlinx/coroutines/experimental/WithTimeoutTest.kt

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,30 @@ package kotlinx.coroutines.experimental
1818

1919
import org.junit.Test
2020

21-
class RunBlockingTest {
21+
class WithTimeoutTest : TestBase() {
22+
/**
23+
* Tests property dispatching of `withTimeout` blocks
24+
*/
25+
@Test
26+
fun testDispatch() = runBlocking {
27+
expect(1)
28+
launch(context) {
29+
expect(4)
30+
yield() // back to main
31+
expect(7)
32+
}
33+
expect(2)
34+
// test that it does not yield to the above job when started
35+
withTimeout(1000) {
36+
expect(3)
37+
yield() // yield only now
38+
expect(5)
39+
}
40+
expect(6)
41+
yield() // back to launch
42+
finish(8)
43+
}
44+
2245
/**
2346
* Tests that a 100% CPU-consuming loop will react on timeout if it has yields.
2447
*/

0 commit comments

Comments
 (0)