Properly round Duration instances to milliseconds (#3921)

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

Author: kevincianfarini

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

@@ -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
@@ -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.
@@ -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.
@@ -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
+}

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

@@ -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())
+    }
+}