Skip to content

Commit

Permalink
rate: prevent overflows when calculating durationFromTokens
Browse files Browse the repository at this point in the history
Currently, there is a conversion from float64 to int64 when returning the duration needed to accumulate the required number of tokens.
When limiters are set with low limits, i.e. 1e-10, the duration needed is greater than math.MaxInt64.
As per the language specifications, in these scenarios the outcome is implementation determined.
This results in overflows on `amd64`, resulting in no wait, effectively jamming the limiter open.

Here we add a check for this scenario, returning InfDuration if the desired duration is greater than math.MaxInt64.

Fixes golang/go#71154

Change-Id: I775aab80fcc8563a59aa399844a64ef70b9eb76a
Reviewed-on: https://go-review.googlesource.com/c/time/+/641336
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
  • Loading branch information
tomcoupland authored and gopherbot committed Jan 8, 2025
1 parent 1ce61fe commit 2c6c5a2
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 2 deletions.
11 changes: 9 additions & 2 deletions rate/rate.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,15 @@ func (limit Limit) durationFromTokens(tokens float64) time.Duration {
if limit <= 0 {
return InfDuration
}
seconds := tokens / float64(limit)
return time.Duration(float64(time.Second) * seconds)

duration := (tokens / float64(limit)) * float64(time.Second)

// Cap the duration to the maximum representable int64 value, to avoid overflow.
if duration > float64(math.MaxInt64) {
return InfDuration
}

return time.Duration(duration)
}

// tokensFromDuration is a unit conversion function from a time duration to the number of tokens
Expand Down
17 changes: 17 additions & 0 deletions rate/rate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,3 +638,20 @@ func TestSetAfterZeroLimit(t *testing.T) {
// We set the limit to 10/s so expect to get another token in 100ms
runWait(t, tt, lim, wait{"wait-after-set-nonzero-after-zero", context.Background(), 1, 1, true})
}

// TestTinyLimit tests that a limiter does not allow more than burst, when the rate is tiny.
// Prior to resolution of issue 71154, this test
// would fail on amd64 due to overflow in durationFromTokens.
func TestTinyLimit(t *testing.T) {
lim := NewLimiter(1e-10, 1)

// The limiter starts with 1 burst token, so the first request should succeed
if !lim.Allow() {
t.Errorf("Limit(1e-10, 1) want true when first used")
}

// The limiter should not have replenished the token bucket yet, so the second request should fail
if lim.Allow() {
t.Errorf("Limit(1e-10, 1) want false when already used")
}
}

0 comments on commit 2c6c5a2

Please sign in to comment.