Commit 93474ecf authored by Alfredo Altamirano's avatar Alfredo Altamirano Committed by Facebook Github Bot

Fix integer overflow in futures retrying library

Summary:
retryingPolicyCappedJitteredExponentialBackoff had an integer overflow.
This occured usually when we had more than 64 retries, since we compute pow(2, n) and then we convert to a 64 bit integer.
I added a check that it won't overflow, and if it does, then it returns the max_backoff value.

Reviewed By: yfeldblum

Differential Revision: D7284952

fbshipit-source-id: 7424f502b8530f8e39ba0513dd70bfc9fdc5c8e5
parent c19e93ed
......@@ -142,10 +142,13 @@ Duration retryingJitteredExponentialBackoffDur(
Duration backoff_max,
double jitter_param,
URNG& rng) {
using d = Duration;
auto dist = std::normal_distribution<double>(0.0, jitter_param);
auto jitter = std::exp(dist(rng));
auto backoff = d(d::rep(jitter * backoff_min.count() * std::pow(2, n - 1)));
auto backoff_rep = jitter * backoff_min.count() * std::pow(2, n - 1);
if (UNLIKELY(backoff_rep >= std::numeric_limits<Duration::rep>::max())) {
return backoff_max;
}
auto backoff = Duration(Duration::rep(backoff_rep));
return std::max(backoff_min, std::min(backoff_max, backoff));
}
......
......@@ -143,6 +143,27 @@ TEST(RetryingTest, policy_capped_jittered_exponential_backoff) {
});
}
TEST(RetryingTest, policy_capped_jittered_exponential_backoff_many_retries) {
using namespace futures::detail;
mt19937_64 rng(0);
Duration min_backoff(1);
Duration max_backoff(10000000);
Duration backoff = retryingJitteredExponentialBackoffDur(
80, min_backoff, max_backoff, 0, rng);
EXPECT_EQ(backoff, max_backoff);
max_backoff = Duration(std::numeric_limits<int64_t>::max());
backoff = retryingJitteredExponentialBackoffDur(
63, min_backoff, max_backoff, 0, rng);
EXPECT_LT(backoff, max_backoff);
max_backoff = Duration(std::numeric_limits<int64_t>::max());
backoff = retryingJitteredExponentialBackoffDur(
64, min_backoff, max_backoff, 0, rng);
EXPECT_EQ(backoff, max_backoff);
}
TEST(RetryingTest, policy_sleep_defaults) {
multiAttemptExpectDurationWithin(5, milliseconds(200), milliseconds(400), []{
// To ensure that this compiles with default params.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment