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

PROTON-2200 Fix memory leak in go message unmarshal #231

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

d98762625
Copy link
Contributor

When using this library in an application to receive messages from QPID, we found our containers were getting OOM killed in kubernetes.

This fixes the memory leak we uncovered.

@astitcher
Copy link
Member

This fix looks good to me @alanconway what do you think?

Copy link
Contributor

@alanconway alanconway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, merge it!

go/pkg/amqp/url_test.go Outdated Show resolved Hide resolved
@alanconway
Copy link
Contributor

/lgtm

@d98762625 d98762625 changed the title WIP: Fix memory leak in go message unmarshal Fix memory leak in go message unmarshal Apr 8, 2020
@d98762625
Copy link
Contributor Author

I don't know if I should or not, but I am unable to merge this PR myself. I assume we just wait for one of the maintainers to merge it?

@jiridanek jiridanek changed the title Fix memory leak in go message unmarshal PROTON-2200 Fix memory leak in go message unmarshal Apr 27, 2020
@jiridanek
Copy link
Contributor

Created a Jira. I am deleting the commits concerning url_test.go which was fixed in a different way meanwhile. Should be good to go when CI passes.

@jiridanek jiridanek merged commit 494a1a8 into apache:master Apr 27, 2020
@d98762625 d98762625 deleted the fix-memory-leak branch April 30, 2020 12:23
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.

4 participants