Backoff: Respect maximum delay in face of float overflow

net/base/backoff_entry.cc

Index: net/base/backoff_entry.cc
diff --git a/net/base/backoff_entry.cc b/net/base/backoff_entry.cc
index b1826b7af469b4d72cfccd6ad849173ceb1f24fe..43037f6d2f89144d6d9b2aa3a150d8dcff1c01f4 100644
--- a/net/base/backoff_entry.cc
+++ b/net/base/backoff_entry.cc
@@ -134,6 +134,10 @@ base::TimeTicks BackoffEntry::CalculateReleaseTime() const {
   //     effective_failure_count - 1) * Uniform(1 - jitter_factor, 1]
   double delay = policy_->initial_delay_ms;
   delay *= pow(policy_->multiply_factor, effective_failure_count - 1);
+  if (isinf(delay)) {
+    delay = std::max(policy_->maximum_backoff_ms,

[cbentzel, 2014/06/14 15:16:07]

maximum_backoff_ms is not always set - so it seems like a bad idea to go to
initial_delay_ms here.

Instead, I think this could be written like

-----

delay *= pow(policy...);
const int64 kMaxInt64 = std::numeric_limits<int64>::max();
int64 delay_int = 0;
if (isinf(delay)) {
  delay_int = kMaxInt64;
} else {
  delay -= base::RandDouble() * policy_->jitter_factory * delay;
  delay_int = (delay > kMaxInt64) ? kMaxInt64 : static_cast<int64>(delay + 0.5);
}
if (policy_->maximum_backoff_ms >= 0)
  delay_int = std::min(delay_int, policy_->maximum_backoff_ms);

------

In the overflow case this means that we will either be the maximum_backoff_ms or
kMaxInt64. Other case stays the same.

-------

However - I am worried that we may get integral overflow issues at some point if
exponential_backoff_release_time_ is kMaxInt64. Inside this class it does not
look like that will be a problem, but we do expose it (via GetReleaseTime) out
to callers and they may run into this issue.

[cbentzel, 2014/06/14 15:30:47]

Actually, we'd reach integral overflow with this approach below when doing
ImplGetTimeNow() + delay_int

[Ryan Sleevi, 2014/06/16 17:20:47]

^ base/basictypes.h has kint64max / kint64min
^ Pretty sure it could still overflow here

[Nicolas Zea, 2014/06/16 20:27:25]

Switched to doing all overflow checking via CheckedNumeric and in microseconds
(to ensure the TimeTicks conversion doesn't overflow).

+                     static_cast<int64>(policy_->initial_delay_ms));
+  }
   delay -= base::RandDouble() * policy_->jitter_factor * delay;
 
   const int64 kMaxInt64 = std::numeric_limits<int64>::max();