Commit c663849f authored by Alex Blanck's avatar Alex Blanck Committed by Facebook GitHub Bot

Fix UndefinedBehaviorSanitizer error in `retryingPolicyCappedJitteredExponentialBackoff`

Summary:
While running some unit tests which happen to call into `folly::futures::retryingPolicyCappedJitteredExponentialBackoff`, I noticed that  `retryingJitteredExponentialBackoffDur` was triggering a "float-cast-overflow" error from UndefinedBehaviorSanitizer.

It turns out that this only happened if `backoff_min` was 0 and there were at least 1025 retries. The large retry count caused the exponential term in the calculation to become `Infinity`. In floating point, Infinity multiplied by 0 was `NaN`, causing the sanitizer error.

Although `backoff_min` should typically not be set to 0 because this effectively disables all backoff, I think there may be cases, such as unit tests, where setting `backoff_min` to 0 does make sense. This commit adds an additional check to handle this unusual case.

Reviewed By: yfeldblum

Differential Revision: D24122470

fbshipit-source-id: 48a8aede0d22e73cf001dab5eb184e841f3c23aa
parent d1015a0b
......@@ -178,6 +178,10 @@ Duration retryingJitteredExponentialBackoffDur(
Duration backoff_max,
double jitter_param,
URNG& rng) {
// short-circuit to avoid 0 * inf = nan when computing backoff_rep below
if (UNLIKELY(backoff_min == Duration(0))) {
return Duration(0);
}
auto dist = std::normal_distribution<double>(0.0, jitter_param);
auto jitter = std::exp(dist(rng));
auto backoff_rep = jitter * backoff_min.count() * std::pow(2, n - 1);
......
......@@ -207,6 +207,30 @@ TEST(RetryingTest, policy_capped_jittered_exponential_backoff_many_retries) {
EXPECT_EQ(backoff, max_backoff);
}
TEST(RetryingTest, policy_capped_jittered_exponential_backoff_min_zero) {
using namespace futures::detail;
mt19937_64 rng(0);
Duration min_backoff(0);
Duration max_backoff(2000);
EXPECT_EQ(
retryingJitteredExponentialBackoffDur(5, min_backoff, max_backoff, 0, rng)
.count(),
0);
EXPECT_EQ(
retryingJitteredExponentialBackoffDur(5, min_backoff, max_backoff, 1, rng)
.count(),
0);
EXPECT_EQ(
retryingJitteredExponentialBackoffDur(
1025, min_backoff, max_backoff, 0, rng)
.count(),
0);
}
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