Skip to content

Commit

Permalink
Properly round Duration instances to milliseconds
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kevincianfarini committed Oct 27, 2023
1 parent 2b5d93f commit 5e6ab6f
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 6 deletions.
15 changes: 9 additions & 6 deletions kotlinx-coroutines-core/common/src/Delay.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package kotlinx.coroutines
import kotlinx.coroutines.selects.*
import kotlin.coroutines.*
import kotlin.time.*
import kotlin.time.Duration.Companion.nanoseconds

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

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

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

/**
* Convert this duration to its millisecond value.
* Positive durations are coerced at least `1`.
* Convert this duration to its millisecond value. Durations which have a nanosecond component less than
* a single millisecond will be rounded up to the next largest millisecond.
*/
internal fun Duration.toDelayMillis(): Long =
if (this > Duration.ZERO) inWholeMilliseconds.coerceAtLeast(1) else 0
internal fun Duration.toDelayMillis(): Long = when (isPositive()) {
true -> plus(999_999L.nanoseconds).inWholeMilliseconds
false -> 0L
}
69 changes: 69 additions & 0 deletions kotlinx-coroutines-core/common/test/DurationToMillisTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright 2016-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
*/
package kotlinx.coroutines

import kotlin.test.*
import kotlin.time.*
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.nanoseconds
import kotlin.time.Duration.Companion.seconds

class DurationToMillisTest {

@Test
fun testNegativeDurationCoercedToZeroMillis() {
assertEquals(0L, (-1).seconds.toDelayMillis())
}

@Test
fun testZeroDurationCoercedToZeroMillis() {
assertEquals(0L, 0.seconds.toDelayMillis())
}

@Test
fun testOneNanosecondCoercedToOneMillisecond() {
assertEquals(1L, 1.nanoseconds.toDelayMillis())
}

@Test
fun testOneSecondCoercedTo1000Milliseconds() {
assertEquals(1_000L, 1.seconds.toDelayMillis())
}

@Test
fun testMixedComponentDurationRoundedUpToNextMillisecond() {
assertEquals(999L, (998.milliseconds + 75909.nanoseconds).toDelayMillis())
}

@Test
fun testOneExtraNanosecondRoundedUpToNextMillisecond() {
assertEquals(999L, (998.milliseconds + 1.nanoseconds).toDelayMillis())
}

@Test
fun testInfiniteDurationCoercedToLongMaxValue() {
assertEquals(Long.MAX_VALUE, Duration.INFINITE.toDelayMillis())
}

@Test
fun testNegativeInfiniteDurationCoercedToZero() {
assertEquals(0L, (-Duration.INFINITE).toDelayMillis())
}

@Test
fun testNanosecondOffByOneInfinityDoesNotOverflow() {
assertEquals(Long.MAX_VALUE / 1_000_000, (Long.MAX_VALUE - 1L).nanoseconds.toDelayMillis())
}

@Test
fun testMillisecondOffByOneInfinityDoesNotIncrement() {
assertEquals((Long.MAX_VALUE / 2) - 1, ((Long.MAX_VALUE / 2) - 1).milliseconds.toDelayMillis())
}

@Test
fun testOutOfBoundsNanosecondsButFiniteDoesNotIncrement() {
val milliseconds = Long.MAX_VALUE / 10
assertEquals(milliseconds, milliseconds.milliseconds.toDelayMillis())
}
}

0 comments on commit 5e6ab6f

Please sign in to comment.