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: set baseinvoker's url to nil to address memory leak issues #2726

Merged
merged 4 commits into from
Aug 30, 2024

Conversation

No-SilverBullet
Copy link
Contributor

@No-SilverBullet No-SilverBullet commented Aug 9, 2024

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

Copy link

sonarcloud bot commented Aug 9, 2024

func (bi *BaseInvoker) Destroy() {
logger.Infof("Destroy invoker: %s", bi.GetURL())
bi.destroyed.Store(true)
bi.available.Store(false)
bi.url = nil
Copy link
Member

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

Copy link
Contributor

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

Copy link
Contributor Author

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;

@No-SilverBullet
Copy link
Contributor Author

@chickenlj plz take the time to review this PR, thanks

@chickenlj chickenlj merged commit 4329b00 into apache:main Aug 30, 2024
5 checks passed
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 this pull request may close these issues.

release the allocated memory of the ‘url’ field when destroy invoker
6 participants