Skip to content

Commit 5e6ab6f

Browse files
Properly round Duration instances to milliseconds
Prior to this commit Durations used in for delays or timeouts lost their nanosecond granularity when being converted to a millisecond Long value. This effectively meant that delays could resume prior to when they were scheduled to do so. This commit solves this by rounding a Duration with nanosecond components up to the next largest millisecond. Closes Kotlin#3920
1 parent 2b5d93f commit 5e6ab6f

File tree

2 files changed

+78
-6
lines changed

2 files changed

+78
-6
lines changed

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

+9-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package kotlinx.coroutines
77
import kotlinx.coroutines.selects.*
88
import kotlin.coroutines.*
99
import kotlin.time.*
10+
import kotlin.time.Duration.Companion.nanoseconds
1011

1112
/**
1213
* This dispatcher _feature_ is implemented by [CoroutineDispatcher] implementations that natively support
@@ -106,7 +107,7 @@ internal interface DelayWithTimeoutDiagnostics : Delay {
106107
public suspend fun awaitCancellation(): Nothing = suspendCancellableCoroutine {}
107108

108109
/**
109-
* Delays coroutine for a given time without blocking a thread and resumes it after a specified time.
110+
* Delays coroutine for at least the given time without blocking a thread and resumes it after a specified time.
110111
* If the given [timeMillis] is non-positive, this function returns immediately.
111112
*
112113
* This suspending function is cancellable.
@@ -133,7 +134,7 @@ public suspend fun delay(timeMillis: Long) {
133134
}
134135

135136
/**
136-
* Delays coroutine for a given [duration] without blocking a thread and resumes it after the specified time.
137+
* Delays coroutine for at least the given [duration] without blocking a thread and resumes it after the specified time.
137138
* If the given [duration] is non-positive, this function returns immediately.
138139
*
139140
* This suspending function is cancellable.
@@ -154,8 +155,10 @@ public suspend fun delay(duration: Duration): Unit = delay(duration.toDelayMilli
154155
internal val CoroutineContext.delay: Delay get() = get(ContinuationInterceptor) as? Delay ?: DefaultDelay
155156

156157
/**
157-
* Convert this duration to its millisecond value.
158-
* Positive durations are coerced at least `1`.
158+
* Convert this duration to its millisecond value. Durations which have a nanosecond component less than
159+
* a single millisecond will be rounded up to the next largest millisecond.
159160
*/
160-
internal fun Duration.toDelayMillis(): Long =
161-
if (this > Duration.ZERO) inWholeMilliseconds.coerceAtLeast(1) else 0
161+
internal fun Duration.toDelayMillis(): Long = when (isPositive()) {
162+
true -> plus(999_999L.nanoseconds).inWholeMilliseconds
163+
false -> 0L
164+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright 2016-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
package kotlinx.coroutines
5+
6+
import kotlin.test.*
7+
import kotlin.time.*
8+
import kotlin.time.Duration.Companion.milliseconds
9+
import kotlin.time.Duration.Companion.nanoseconds
10+
import kotlin.time.Duration.Companion.seconds
11+
12+
class DurationToMillisTest {
13+
14+
@Test
15+
fun testNegativeDurationCoercedToZeroMillis() {
16+
assertEquals(0L, (-1).seconds.toDelayMillis())
17+
}
18+
19+
@Test
20+
fun testZeroDurationCoercedToZeroMillis() {
21+
assertEquals(0L, 0.seconds.toDelayMillis())
22+
}
23+
24+
@Test
25+
fun testOneNanosecondCoercedToOneMillisecond() {
26+
assertEquals(1L, 1.nanoseconds.toDelayMillis())
27+
}
28+
29+
@Test
30+
fun testOneSecondCoercedTo1000Milliseconds() {
31+
assertEquals(1_000L, 1.seconds.toDelayMillis())
32+
}
33+
34+
@Test
35+
fun testMixedComponentDurationRoundedUpToNextMillisecond() {
36+
assertEquals(999L, (998.milliseconds + 75909.nanoseconds).toDelayMillis())
37+
}
38+
39+
@Test
40+
fun testOneExtraNanosecondRoundedUpToNextMillisecond() {
41+
assertEquals(999L, (998.milliseconds + 1.nanoseconds).toDelayMillis())
42+
}
43+
44+
@Test
45+
fun testInfiniteDurationCoercedToLongMaxValue() {
46+
assertEquals(Long.MAX_VALUE, Duration.INFINITE.toDelayMillis())
47+
}
48+
49+
@Test
50+
fun testNegativeInfiniteDurationCoercedToZero() {
51+
assertEquals(0L, (-Duration.INFINITE).toDelayMillis())
52+
}
53+
54+
@Test
55+
fun testNanosecondOffByOneInfinityDoesNotOverflow() {
56+
assertEquals(Long.MAX_VALUE / 1_000_000, (Long.MAX_VALUE - 1L).nanoseconds.toDelayMillis())
57+
}
58+
59+
@Test
60+
fun testMillisecondOffByOneInfinityDoesNotIncrement() {
61+
assertEquals((Long.MAX_VALUE / 2) - 1, ((Long.MAX_VALUE / 2) - 1).milliseconds.toDelayMillis())
62+
}
63+
64+
@Test
65+
fun testOutOfBoundsNanosecondsButFiniteDoesNotIncrement() {
66+
val milliseconds = Long.MAX_VALUE / 10
67+
assertEquals(milliseconds, milliseconds.milliseconds.toDelayMillis())
68+
}
69+
}

0 commit comments

Comments
 (0)