-
Notifications
You must be signed in to change notification settings - Fork 225
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 in ContextSerivce. #34
Conversation
|
||
// WithCloseHandler returns a Handler cancelling the context when the client | ||
// connection close unexpectedly. | ||
func WithCloseHandler(h ContextHandler) ContextHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you calling this anywhere? Or just bringing the function into the toolkit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of copy/paste straight from rs/xhandler
, could you just use their xhandler.CloseHandler
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to use additional library? and we have different types.
Just tried - we could not use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I thought they interface was compatible, but since we have a ServeHTTPContext
where they have ServeHTTPC
, you're right that it's not an easy drop-in.
Go ahead and leave this for now.
Looks good! Thanks, @gaplyk! |
Fix memory leak in ContextSerivce.
Make context.Background for each request to the server. (When it's global, this cause memory leak)
Remove ContextWithIP and ContextWithForwardForIP. Could be used in ContextMiddleware
Add WithCloseHandler middleware