-
Notifications
You must be signed in to change notification settings - Fork 931
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: set baseinvoker's url to nil to address memory leak issues #2726
Conversation
…related references to avoid panic
fix:set url to nil when destroy base invoker
reafactor:add comment at BaseInvoker.Destroy()
Quality Gate passedIssues Measures |
func (bi *BaseInvoker) Destroy() { | ||
logger.Infof("Destroy invoker: %s", bi.GetURL()) | ||
bi.destroyed.Store(true) | ||
bi.available.Store(false) | ||
bi.url = nil |
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.
If set nil, is there any risk of a null pointer elsewhere
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.
set it nil to let the gc collect resource
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.
As u can see in other changed files, in other places whereurl
are used, I changed the order, that is, destroyed them in the end to avoid null pointer issues; But still requires careful review;
@chickenlj plz take the time to review this PR, thanks |
Bssed on PR #2223 ,the main purpose of this PR is to fix the memory leak problem caused by the url pointer in baseinvoker not being released correctly. You can refer to the following issue:
FIXES: #2701 #2339 #2031