Allow negative timeouts in delay, withTimeout and onTimeout on JVM

Fixes #310

Author: qwwdfsad

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/Delay.kt

@@ -35,7 +35,6 @@ public interface Delay {
      * immediately resumes with [CancellationException].
      */
     suspend fun delay(time: Long, unit: TimeUnit = TimeUnit.MILLISECONDS) {
-        require(time >= 0) { "Delay time $time cannot be negative" }
         if (time <= 0) return // don't delay
         return suspendCancellableCoroutine { scheduleResumeAfterDelay(time, unit, it) }
     }
@@ -99,7 +98,6 @@ public suspend fun delay(time: Int) =
  * @param unit time unit.
  */
 public suspend fun delay(time: Long, unit: TimeUnit = TimeUnit.MILLISECONDS) {
-    require(time >= 0) { "Delay time $time cannot be negative" }
     if (time <= 0) return // don't delay
     return suspendCancellableCoroutine sc@ { cont: CancellableContinuation<Unit> ->
         cont.context.delay.scheduleResumeAfterDelay(time, unit, cont)

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

@@ -61,7 +61,6 @@ public suspend fun <T> withTimeout(time: Int, block: suspend CoroutineScope.() -
  * @param unit timeout unit (milliseconds by default)
  */
 public suspend fun <T> withTimeout(time: Long, unit: TimeUnit = TimeUnit.MILLISECONDS, block: suspend CoroutineScope.() -> T): T {
-    require(time >= 0) { "Timeout time $time cannot be negative" }
     if (time <= 0L) throw CancellationException("Timed out immediately")
     return suspendCoroutineOrReturn { cont: Continuation<T> ->
         setupTimeout(TimeoutCoroutine(time, unit, cont), block)
@@ -151,7 +150,6 @@ public suspend fun <T> withTimeoutOrNull(time: Int, block: suspend CoroutineScop
  * @param unit timeout unit (milliseconds by default)
  */
 public suspend fun <T> withTimeoutOrNull(time: Long, unit: TimeUnit = TimeUnit.MILLISECONDS, block: suspend CoroutineScope.() -> T): T? {
-    require(time >= 0) { "Timeout time $time cannot be negative" }
     if (time <= 0L) return null
     return suspendCoroutineOrReturn { cont: Continuation<T?> ->
         setupTimeout(TimeoutOrNullCoroutine(time, unit, cont), block)

common/kotlinx-coroutines-core-common/src/main/kotlin/kotlinx/coroutines/experimental/selects/Select.kt

@@ -52,6 +52,7 @@ public interface SelectBuilder<in R> {
 
     /**
      * Clause that selects the given [block] after a specified timeout passes.
+     * If timeout is negative or zero, [block] is selected immediately.
      *
      * @param time timeout time
      * @param unit timeout unit (milliseconds by default)
@@ -416,8 +417,7 @@ internal class SelectBuilderImpl<in R>(
     }
 
     override fun onTimeout(time: Long, unit: TimeUnit, block: suspend () -> R) {
-        require(time >= 0) { "Timeout time $time cannot be negative" }
-        if (time == 0L) {
+        if (time <= 0L) {
             if (trySelect(null))
                 block.startCoroutineUndispatched(completion)
             return

common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/WithTimeoutOrNullTest.kt

@@ -192,4 +192,18 @@ class WithTimeoutOrNullTest : TestBase() {
     }
 
     private class TestException : Exception()
+
+    @Test
+    fun testNegativeTimeout() = runTest {
+        expect(1)
+        var result = withTimeoutOrNull(-1) {
+            expectUnreached()
+        }
+        assertNull(result)
+        result = withTimeoutOrNull(0) {
+            expectUnreached()
+        }
+        assertNull(result)
+        finish(2)
+    }
 }

common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/WithTimeoutTest.kt

@@ -179,5 +179,19 @@ class WithTimeoutTest : TestBase() {
     }
 
     private class TestException : Exception()
+
+    @Test
+    fun testNegativeTimeout() = runTest {
+        expect(1)
+        try {
+            withTimeout(-1) {
+                expectUnreached()
+                "OK"
+            }
+        } catch (e: CancellationException) {
+            assertEquals("Timed out immediately", e.message)
+            finish(2)
+        }
+    }
 }
 

common/kotlinx-coroutines-core-common/src/test/kotlin/kotlinx/coroutines/experimental/selects/SelectTimeoutTest.kt

@@ -20,6 +20,7 @@ import kotlinx.coroutines.experimental.*
 import kotlin.test.*
 
 class SelectTimeoutTest : TestBase() {
+
     @Test
     fun testBasic() = runTest {
         expect(1)
@@ -40,4 +41,62 @@ class SelectTimeoutTest : TestBase() {
         assertEquals("OK", result)
         finish(3)
     }
-}
+
+    @Test
+    fun testZeroTimeout() = runTest {
+        expect(1)
+        val result = select<String> {
+            onTimeout(1000) {
+                expectUnreached()
+                "FAIL"
+            }
+            onTimeout(0) {
+                expect(2)
+                "OK"
+            }
+        }
+        assertEquals("OK", result)
+        finish(3)
+    }
+
+    @Test
+    fun testNegativeTimeout() = runTest {
+        expect(1)
+        val result = select<String> {
+            onTimeout(1000) {
+                expectUnreached()
+                "FAIL"
+            }
+            onTimeout(-10) {
+                expect(2)
+                "OK"
+            }
+        }
+        assertEquals("OK", result)
+        finish(3)
+    }
+
+    @Test
+    fun testUnbiasedNegativeTimeout() = runTest {
+        val counters = intArrayOf(0, 0, 0)
+        val iterations =10_000
+        for (i in 0..iterations) {
+            val result = selectUnbiased<Int> {
+                onTimeout(-10) {
+                    0
+                }
+                onTimeout(0) {
+                    1
+                }
+                onTimeout(10) {
+                    expectUnreached()
+                    2
+                }
+            }
+            ++counters[result]
+        }
+        assertEquals(0, counters[2])
+        assertTrue { counters[0] >  iterations / 4 }
+        assertTrue { counters[1] >  iterations / 4 }
+    }
+}

core/kotlinx-coroutines-core/src/test/kotlin/kotlinx/coroutines/experimental/DelayTest.kt

@@ -21,10 +21,7 @@ import org.hamcrest.core.IsEqual
 import org.junit.Test
 import java.util.concurrent.Executor
 import java.util.concurrent.Executors
-import kotlin.coroutines.experimental.AbstractCoroutineContextElement
-import kotlin.coroutines.experimental.Continuation
-import kotlin.coroutines.experimental.ContinuationInterceptor
-import kotlin.coroutines.experimental.CoroutineContext
+import kotlin.coroutines.experimental.*
 
 class DelayTest : TestBase() {
     /**
@@ -47,7 +44,6 @@ class DelayTest : TestBase() {
         pool.shutdown()
     }
 
-
     @Test
     fun testDelayWithoutDispatcher() = runBlocking(CoroutineName("testNoDispatcher.main")) {
         // launch w/o a specified dispatcher
@@ -58,6 +54,21 @@ class DelayTest : TestBase() {
         assertThat(c.await(), IsEqual(42))
     }
 
+    @Test
+    fun testNegativeDelay() = runBlocking {
+        expect(1)
+        val job = async(coroutineContext) {
+            expect(3)
+            delay(0)
+            expect(4)
+        }
+
+        delay(-1)
+        expect(2)
+        job.await()
+        finish(5)
+    }
+
     class CustomInterceptor(val pool: Executor) : AbstractCoroutineContextElement(ContinuationInterceptor), ContinuationInterceptor {
         override fun <T> interceptContinuation(continuation: Continuation<T>): Continuation<T> =
             Wrapper(pool, continuation)

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

@@ -0,0 +1,59 @@
+
+package kotlinx.coroutines.experimental
+
+import kotlin.test.*
+import java.io.IOException
+
+class WithTimeoutTest : TestBase() {
+    @Test
+    fun testExceptionOnTimeout() = runTest {
+        expect(1)
+        try {
+            withTimeout(100) {
+                expect(2)
+                delay(1000)
+                expectUnreached()
+                "OK"
+            }
+        } catch (e: CancellationException) {
+            assertEquals("Timed out waiting for 100 MILLISECONDS", e.message)
+            finish(3)
+        }
+    }
+
+    @Test
+    fun testSuppressExceptionWithResult() = runTest(
+        expected = { it is CancellationException }
+    ) {
+        expect(1)
+        val result = withTimeout(100) {
+            expect(2)
+            try {
+                delay(1000)
+            } catch (e: CancellationException) {
+                finish(3)
+            }
+            "OK"
+        }
+        expectUnreached()
+    }
+
+    @Test
+    fun testSuppressExceptionWithAnotherException() = runTest(
+        expected = { it is IOException }
+    ) {
+        expect(1)
+        withTimeout(100) {
+            expect(2)
+            try {
+                delay(1000)
+            } catch (e: CancellationException) {
+                finish(3)
+                throw IOException(e)
+            }
+            expectUnreached()
+            "OK"
+        }
+        expectUnreached()
+    }
+}