Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential memory leak #36

Closed
dnozdrin opened this issue Aug 9, 2023 · 0 comments · Fixed by #37
Closed

Potential memory leak #36

dnozdrin opened this issue Aug 9, 2023 · 0 comments · Fixed by #37

Comments

@dnozdrin
Copy link
Contributor

dnozdrin commented Aug 9, 2023

Usage of <-time.After(c.Timeout) here may lead to memory leak.

timeout := time.After(r.calcSleep(retries))

According to the official time package docs:

After waits for the duration to elapse and then sends the current time on the returned channel. It is equivalent to NewTimer(d).C. The underlying Timer is not recovered by the garbage collector until the timer fires. If efficiency is a concern, use NewTimer instead and call Timer.Stop if the timer is no longer needed.

Possible solution:

func (r *Retrier) RunCtx(ctx context.Context, work func(ctx context.Context) error) error {
	retries := 0
	for {
		ret := work(ctx)

		switch r.class.Classify(ret) {
		case Succeed, Fail:
			return ret
		case Retry:
			if retries >= len(r.backoff) {
				return ret
			}

			timer := time.NewTimer(r.calcSleep(retries))
			if err := r.sleep(ctx, timer); err != nil {
				return err
			}

			retries++
		}
	}
}

func (r *Retrier) sleep(ctx context.Context, timer *time.Timer) error {
	select {
	case <-timer.C:
		return nil
	case <-ctx.Done():
		if !timer.Stop() {
			<-timer.C
		}

		return ctx.Err()
	}
}

References:

  1. A story of a memory leak in GO: How to properly use time.After()
  2. Use "time.After" may cause a memory leak eclipse/paho.mqtt.golang#518
  3. Golang <-time.After() is not garbage collected before expiry
  4. time: Timer.Stop documentation example easily leads to deadlocks golang/go#27169
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant