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

Plug a packet buffer memory leak #152

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Conversation

stevenmhood
Copy link

@stevenmhood stevenmhood commented Oct 24, 2022

Decrement reference count after queueing the PacketBuffer. Since PacketBuffer objects come from a pool, this allows them to be returned to the pool instead of staying in memory and forcing new allocations.

Fixes #107

My interest in fixing this lies in this projects downstream usage in wsl-vpnkit, which grows its memory footprint until my system comes to a standstill (several GB of RAM). With this fix, memory usage stays around 20MB, give or take. Tracing the NewPacketBuffer call back through the stack exposed the Pool that underlies the PacketBuffer instances.

Copy link
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

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

This matches the way it's used in the gvisor codebase, looks good to me. Thanks a lot for the patch!

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 25, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cfergeau, stevenmhood

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cfergeau
Copy link
Collaborator

This is missing a DCO before this can be merged, https://github.com/containers/gvisor-tap-vsock/pull/152/checks?check_run_id=9081855178

Decrement reference count after queueing the PacketBuffer.  Since
PacketBuffer objects come from a pool, this allows them to be returned
to the pool instead of staying in memory and forcing new allocations.

Fixes containers#107

Signed-off-by: stevenmhood <[email protected]>
@stevenmhood
Copy link
Author

Moved this to re-request, now that the DCO passed, since the other workflows need approval to run. Not sure if you check these periodically or on-demand, either way, no rush from my side. Thanks!

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.

Memory leaks
2 participants