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 #89

Closed
dnozdrin opened this issue Mar 14, 2023 · 0 comments · Fixed by #90
Closed

Potential memory leak #89

dnozdrin opened this issue Mar 14, 2023 · 0 comments · Fixed by #90

Comments

@dnozdrin
Copy link
Contributor

dnozdrin commented Mar 14, 2023

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

case <-time.After(c.Timeout):

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:

			timeout := time.NewTimer(c.Timeout)

			select {
			case <-timeout.C:
				mu.Lock()
				defer mu.Unlock()

				span.SetStatus(codes.Error, string(StatusTimeout))

				failures[c.Name] = string(StatusTimeout)
				status = getAvailability(status, c.SkipOnErr)
			case res := <-resCh:
				if !timeout.Stop() {
					<-timeout.C
				}

With time.After (200K healthcheck requests):

Showing nodes accounting for 12875.52kB, 100% of 12875.52kB total
Showing top 10 nodes out of 48
      flat  flat%   sum%        cum   cum%
 5120.39kB 39.77% 39.77%  5699.05kB 44.26%  time.NewTimer
    3075kB 23.88% 63.65%     3075kB 23.88%  runtime.allocm
 1026.94kB  7.98% 71.63%  1026.94kB  7.98%  regexp/syntax.(*compiler).inst
  578.66kB  4.49% 76.12%   578.66kB  4.49%  time.startTimer
  513.50kB  3.99% 80.11%   513.50kB  3.99%  regexp.onePassCopy
  512.75kB  3.98% 84.09%   512.75kB  3.98%  sync.(*Pool).pinSlow
  512.20kB  3.98% 88.07%   512.20kB  3.98%  runtime.malg
  512.05kB  3.98% 92.05%   512.05kB  3.98%  runtime.acquireSudog
  512.02kB  3.98% 96.02%  1024.06kB  7.95%  runtime.gcBgMarkWorker
  512.01kB  3.98%   100%   512.01kB  3.98%  regexp/syntax.(*Regexp).CapNames

With time.NewTimer (200K healthcheck requests):

Showing nodes accounting for 8288.16kB, 100% of 8288.16kB total
Showing top 10 nodes out of 37
      flat  flat%   sum%        cum   cum%
 3587.50kB 43.28% 43.28%  3587.50kB 43.28%  runtime.allocm
 1536.61kB 18.54% 61.82%  1536.61kB 18.54%  runtime.malg
  600.58kB  7.25% 69.07%   600.58kB  7.25%  github.com/go-playground/validator/v10.init
     515kB  6.21% 75.28%      515kB  6.21%  vendor/golang.org/x/net/http2/hpack.(*headerFieldTable).addEntry
  512.31kB  6.18% 81.47%   512.31kB  6.18%  regexp/syntax.(*compiler).inst (inline)
  512.12kB  6.18% 87.64%   512.12kB  6.18%  net/http.ListenAndServe
  512.02kB  6.18% 93.82%   512.02kB  6.18%  regexp.onePassCopy
  512.01kB  6.18%   100%   512.01kB  6.18%  regexp.makeOnePass.func1
         0     0%   100%  1024.03kB 12.36%  github.com/go-playground/validator/v10.init.0
         0     0%   100%   512.31kB  6.18%  github.com/jinzhu/inflection.compile

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
  3. Golang <-time.After() is not garbage collected before expiry
  4. time: Timer.Stop documentation example easily leads to deadlocks
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