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

Fix memory leak #251

Merged
merged 2 commits into from Feb 5, 2020
Merged

Fix memory leak #251

merged 2 commits into from Feb 5, 2020

Conversation

thebaer
Copy link
Member

@thebaer thebaer commented Jan 29, 2020

This upgrades the gorilla/session library to v1.2.0, which gets rid of the gorilla/context dependency, which might have been causing a memory leak.

We noticed some serious memory leakage on Write.as that seemed to point to this library. One heap snapshot:

      flat  flat%   sum%        cum   cum%
  259.13MB 30.41% 30.41%   268.13MB 31.46%  net/textproto.(*Reader).ReadMIMEHeader
  105.71MB 12.40% 42.81%   105.71MB 12.40%  github.com/gorilla/context.Set
   78.53MB  9.21% 52.03%   125.53MB 14.73%  github.com/gorilla/sessions.(*Registry).Get
   55.51MB  6.51% 58.54%    82.52MB  9.68%  net/http.(*Request).WithContext
   38.01MB  4.46% 63.00%    38.01MB  4.46%  github.com/gorilla/mux.extractVars
      35MB  4.11% 67.11%       53MB  6.22%  context.WithCancel
   34.50MB  4.05% 71.16%    34.50MB  4.05%  context.WithValue
      27MB  3.17% 74.32%       27MB  3.17%  net/http.cloneURL
      26MB  3.05% 77.38%       26MB  3.05%  github.com/gorilla/sessions.NewSession
      18MB  2.11% 79.49%       18MB  2.11%  context.(*cancelCtx).Done
   16.50MB  1.94% 81.42%    16.50MB  1.94%  syscall.anyToSockaddr
      14MB  1.64% 83.07%       47MB  5.52%  github.com/gorilla/sessions.(*CookieStore).New
   13.50MB  1.58% 84.65%    51.51MB  6.04%  github.com/gorilla/mux.(*Route).Match
   11.67MB  1.37% 86.02%    13.21MB  1.55%  regexp.(*Regexp).replaceAll
    9.72MB  1.14% 87.16%    22.94MB  2.69%  regexp.(*Regexp).ReplaceAllString
    9.50MB  1.11% 88.28%   115.21MB 13.52%  github.com/gorilla/sessions.GetRegistry

Then with the help of the following articles, we tracked it down to this dependency. We upgraded the necessary library, which seems to have completely fixed the issue so far:

We'll give this a little more time to verify there isn't anything else causing a leak. But combined with #249, this should fix #133.

This gets rid of the gorilla/context dependency, which might have been
causing a memory leak.

We noticed some serious memory leakage on Write.as that seemed to point
to this library. One heap snapshot:

      flat  flat%   sum%        cum   cum%
  259.13MB 30.41% 30.41%   268.13MB 31.46%  net/textproto.(*Reader).ReadMIMEHeader
  105.71MB 12.40% 42.81%   105.71MB 12.40%  github.com/gorilla/context.Set
   78.53MB  9.21% 52.03%   125.53MB 14.73%  github.com/gorilla/sessions.(*Registry).Get
   55.51MB  6.51% 58.54%    82.52MB  9.68%  net/http.(*Request).WithContext
   38.01MB  4.46% 63.00%    38.01MB  4.46%  github.com/gorilla/mux.extractVars
      35MB  4.11% 67.11%       53MB  6.22%  context.WithCancel
   34.50MB  4.05% 71.16%    34.50MB  4.05%  context.WithValue
      27MB  3.17% 74.32%       27MB  3.17%  net/http.cloneURL
      26MB  3.05% 77.38%       26MB  3.05%  github.com/gorilla/sessions.NewSession
      18MB  2.11% 79.49%       18MB  2.11%  context.(*cancelCtx).Done
   16.50MB  1.94% 81.42%    16.50MB  1.94%  syscall.anyToSockaddr
      14MB  1.64% 83.07%       47MB  5.52%  github.com/gorilla/sessions.(*CookieStore).New
   13.50MB  1.58% 84.65%    51.51MB  6.04%  github.com/gorilla/mux.(*Route).Match
   11.67MB  1.37% 86.02%    13.21MB  1.55%  regexp.(*Regexp).replaceAll
    9.72MB  1.14% 87.16%    22.94MB  2.69%  regexp.(*Regexp).ReplaceAllString
    9.50MB  1.11% 88.28%   115.21MB 13.52%  github.com/gorilla/sessions.GetRegistry

With the help of these articles, we tracked it down to this dependency,
and upgraded the library, which seems to have completely fixed the issue
so far:

https://rover.rocks/golang-memory-leak/
https://medium.com/@walterwu_22843/golang-memory-leak-while-handling-huge-amount-of-http-request-35cc970cb75e

This should fix #133
@thebaer thebaer added this to the 0.12 milestone Jan 29, 2020
@thebaer thebaer mentioned this pull request Jan 29, 2020
@thebaer
Copy link
Member Author

thebaer commented Feb 5, 2020

After some time, I've confirmed this issue is resolved on Write.as. That seems to hold true for other instances, too (see #133). Merging now.

@thebaer thebaer merged commit fec0eb2 into develop Feb 5, 2020
@thebaer thebaer deleted the fix-memory-leak branch February 5, 2020 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Possible memory leak?
1 participant