backoff icon indicating copy to clipboard operation
backoff copied to clipboard

Infinite loop if `ExponentialBackoff.Stop` is zero/unset

Open bradleyjkemp opened this issue 1 year ago • 4 comments

If ExponentialBackoff.Stop is zero (e.g. if you forget to initialise it) then the backoff function can run into an infinite loop on this line: https://github.com/cenkalti/backoff/blob/a04a6fe64ffb0e3fd0816460529d300be5f252df/retry.go#L98

Here b.NextBackoff() returns the ExponentialBackoff.Stop value but it's being compared against the package constant Stop (defined as -1)

I'm not sure why you'd ever want to change this value (so perhaps it shouldn't have been an exported member?) but I think this needs changing to either:

  • Compare b.NextBackoff() against whatever ExponentialBackoff.Stop is set to
  • Do some initialising logic so that ExponentialBackoff.Stop is always set to a sensible value

bradleyjkemp avatar May 24 '23 08:05 bradleyjkemp

It's a shame that it's exported.

As far as I know usage of NewExponentialBackOff will ensure that ExponentialBackoff.Stop is always set to a sensible value as you want. So I'm not sure there is anything to be done here.

Dean-Coakley avatar Jun 29 '23 15:06 Dean-Coakley

Oh yeah entirely my mistake for initialising the struct myself rather than using the constructor. Would be nice to guard against this though, as the effects are pretty bad! (rather than running the function with a graceful backoff, run it repeatedly with no delay 😬)

bradleyjkemp avatar Jun 30 '23 10:06 bradleyjkemp

Well un-exporting the field would be a breaking change and won't be done in v4 anyway, and unfortunately as far as I know a v5 is not planned. 😕

Dean-Coakley avatar Jun 30 '23 15:06 Dean-Coakley

Encountered same issue

tot-ra avatar Jul 06 '23 09:07 tot-ra