Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in withTimeout(Duration.ZERO) #1349

Closed
fvasco opened this issue Jul 17, 2019 · 1 comment
Closed

Error in withTimeout(Duration.ZERO) #1349

fvasco opened this issue Jul 17, 2019 · 1 comment
Labels

Comments

@fvasco
Copy link
Contributor

fvasco commented Jul 17, 2019

I found a regression bug #870 in manual merge #1006

import kotlinx.coroutines.time.withTimeout
import kotlinx.coroutines.withTimeout
import java.time.Duration

suspend fun main() {
    withTimeout(Duration.ZERO) {} // Duration works
    withTimeout(0) {} // Long does not work
}

Proposed changes are:

fun Duration.coerceToMillis(): Long {
    if (this <= Duration.ZERO) return 0
    if (this <= ChronoUnit.MILLIS.duration) return 1

    // Maximum scalar values of Duration.ofMillis(Long.MAX_VALUE)
    val maxSeconds = 9223372036854775
    val maxNanos = 807000000
    return if (seconds < maxSeconds || seconds == maxSeconds && nano < maxNanos) toMillis()
    else Long.MAX_VALUE
}

or original PR (logharitmic search):

fun Duration.toMillisDelay(): Long =
        if (this <= ChronoUnit.MILLIS.duration) {
            if (this <= Duration.ZERO) 0
            else 1
        } else {
            // Maximum scalar values of Duration.ofMillis(Long.MAX_VALUE)
            val maxSeconds = 9223372036854775
            val maxNanos = 807000000
            if (seconds < maxSeconds || seconds == maxSeconds && nano < maxNanos) toMillis()
            else Long.MAX_VALUE
        }
@fvasco
Copy link
Contributor Author

fvasco commented Jul 17, 2019

Moreover this bug introduces an unrequested delay in case of delay(Duration.ZERO), it become delay(1L).
In this case it avoids delay(Long) fast path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants