Skip to content

Commit

Permalink
Properly round Duration instances to milliseconds (#3921)
Browse files Browse the repository at this point in the history
Prior to this commit Durations used for delays or timeouts
lost their nanosecond granularity during the conversion to a
millisecond Long value. This effectively meant that delays could
resume at most a millisecond in advance.

This commit solves this by rounding a Duration with nanosecond
components up to the next largest millisecond.

Fixes #3920
  • Loading branch information
kevincianfarini committed Nov 15, 2023
1 parent 3dd48ac commit ff95ab7
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 ff95ab7

Please sign in to comment.