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

When system time is incorrect and in the past coroutine can be suspended for a long period of time #805

Closed
iliasapsyrin opened this issue Feb 20, 2023 · 2 comments · Fixed by #810
Assignees
Labels
bug This issue is a bug.

Comments

@iliasapsyrin
Copy link

iliasapsyrin commented Feb 20, 2023

Describe the bug

If system time rolls back (ie it was incorrect and then set back to correct value) capacity will have negative value and delayMs can be potentially as big as time difference between lastTimestamp and current system time.

https://github.com/awslabs/smithy-kotlin/blob/7babb8883133b35d2f81c6b51cebc3c03773775f/runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/retries/delay/StandardRetryTokenBucket.kt#L67

Expected Behavior

Coroutine should not suspend longer than for options.maxCapacity.

Current Behavior

Coroutine get suspended longer than for options.maxCapacity.

Steps to Reproduce

  1. Set system time 1 day in the future
  2. Create a queue using SqsClient.createQueue(). It should return SignatureDoesNotMatch error
  3. Reset time back to correct value
  4. Make another call to SqsClient.createQueue(). It will suspend for 86335800 milliseconds

Possible Solution

Wrap calculation of refillMs with abs in StandardRetryTokenBucket:

    private fun refillCapacity() {
        val refillMs = abs(now() - lastTimestamp)
        val refillSize = floor(options.refillUnitsPerSecond.toDouble() / MS_PER_S * refillMs).toInt()
        capacity = min(options.maxCapacity, capacity + refillSize)
    }

Context

If time on device is wrong we can receive SignatureDoesNotMatch error from SQS and ask user to set time correctly. But when user changes time back there's no way to see that there's an issue as coroutine have got suspended and no exceptions being thrown.

Your Environment

Smithy Kotlin version - 0.17.0-beta
Operating system - Android 12

@iliasapsyrin iliasapsyrin changed the title When system time is incorrect del When system time is incorrect and in the past coroutine can be suspended for a long period of time Feb 20, 2023
@ianbotsf
Copy link
Contributor

Thank you for the bug report @iliasapsyrin. I've reproduced the issue locally and I'll have a fix in PR shortly.

@ianbotsf
Copy link
Contributor

The fix is merged to main and will be available in today's release (smithy-kotlin 0.16.0 consumed by aws-sdk-kotlin 0.21.0-beta).

Thank you very much @iliasapsyrin for the detailed report and the debugging you did to present the issue. It would've been a challenge to debug and reproduce this without your help!

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

Successfully merging a pull request may close this issue.

2 participants