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.

Sign up for GitHub

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

Jump to bottom

Document Potential for Memory Leak #59

Closed
jvatic opened this issue Dec 7, 2015 · 4 comments
Closed

Document Potential for Memory Leak #59

jvatic opened this issue Dec 7, 2015 · 4 comments
Assignees
@elithrar
Labels
documentation

Comments

@jvatic
Copy link

jvatic commented Dec 7, 2015

Run while true; do curl -s "http://localhost:12345" > /dev/null; done against the server in https://gist.github.com/jvatic/a584622cc2346f754d24 and watch memory usage steadily climb.
@jvatic jvatic mentioned this issue Dec 7, 2015
Closed
@jvatic
Copy link
Author

jvatic commented Dec 7, 2015

Narrowed it down a bit further, updated gist

@jvatic jvatic changed the title CookieStore memory leak memory leak Dec 7, 2015
@jvatic
Copy link
Author

jvatic commented Dec 7, 2015

The issue is the new registry allocated when calling sessions.GetRegistry is saved in gorilla/context and isn't released until context.Clear(req) is called. This is mentioned in the docs but not in the README.

@jvatic jvatic closed this as completed Dec 7, 2015
jvatic added a commit to flynn/flynn that referenced this issue Dec 7, 2015
@jvatic
dashboard: Fix memory leak
dcbd6c5 
See gorilla/sessions#59

Closes #1776

Signed-off-by: Jesse Stuart <jesse@jessestuart.ca>
@elithrar
Copy link
Contributor

elithrar commented Dec 7, 2015

Hi @jvatic - I'll update the README to mention this as well.

The website (based on doc.go) is the canonical reference but we added a short version as a README since a lot of users come directly via GitHub.

@elithrar elithrar self-assigned this Dec 7, 2015
@elithrar elithrar changed the title memory leak Document Potential for Memory Leak Dec 7, 2015
@elithrar elithrar reopened this Dec 7, 2015
elithrar added a commit that referenced this issue Dec 22, 2015
@elithrar
[docs] Added mention of context.ClearHandler as per #59; formatting f…
e621204 
…ixes.
@elithrar elithrar mentioned this issue Dec 22, 2015
Merged
elithrar added a commit that referenced this issue Dec 23, 2015
@elithrar
Merge pull request #61 from gorilla/readme-notes
99b16d2 
[docs] Added mention of context.ClearHandler as per #59
@elithrar
Copy link
Contributor

Addressed in 99b16d2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@elithrar elithrar

Labels
documentation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@elithrar @jvatic