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: properly close buffers to prevent memory leaks #1146

Merged
merged 2 commits into from
Mar 10, 2023
Merged

fix: properly close buffers to prevent memory leaks #1146

merged 2 commits into from
Mar 10, 2023

Conversation

derklaro
Copy link
Member

Motivation

The current way we handle data bufs is prone to memory "leaks". They are not noticeable by the user directly, as the underlying memory of a buffer gets cleaned when it becomes phantom-reachable, but if a process is allowed to allocate a lot of memory, the consumed memory will rise until the garbage collector takes action (which can be very late considering that we set ZGC as the default in our start scripts).

Modification

Change the complete internal code in a way that no more memory leaks are occurring. Note that this change might be error prone, as external plugins might use buffers in an invalid way that causes issues after the change. External code is now required to acquire and release a buffer to ensure it's not getting closed, rather than keeping a reference which prevents the buffer from getting released.

Result

No more internal memory leaks.

@derklaro derklaro added v: 4.X This pull should be included in the 4.0 release t: fix A pull request introducing a fix for a bug. labels Feb 27, 2023
@derklaro derklaro added this to the 4.0.0-RC9 milestone Feb 27, 2023
@derklaro derklaro self-assigned this Feb 27, 2023
@0utplay 0utplay merged commit 45bfb03 into nightly Mar 10, 2023
@0utplay 0utplay deleted the leak branch March 10, 2023 16:25
derklaro added a commit that referenced this pull request Mar 14, 2023
### Motivation
The current way we handle data bufs is prone to memory "leaks". They are
not noticeable by the user directly, as the underlying memory of a
buffer gets cleaned when it becomes phantom-reachable, but if a process
is allowed to allocate a lot of memory, the consumed memory will rise
until the garbage collector takes action (which can be very late
considering that we set ZGC as the default in our start scripts).

### Modification
Change the complete internal code in a way that no more memory leaks are
occurring. Note that this change might be error prone, as external
plugins might use buffers in an invalid way that causes issues after the
change. External code is now required to acquire and release a buffer to
ensure it's not getting closed, rather than keeping a reference which
prevents the buffer from getting released.

### Result
No more internal memory leaks.
derklaro added a commit that referenced this pull request Apr 17, 2023
### Motivation
The current way we handle data bufs is prone to memory "leaks". They are
not noticeable by the user directly, as the underlying memory of a
buffer gets cleaned when it becomes phantom-reachable, but if a process
is allowed to allocate a lot of memory, the consumed memory will rise
until the garbage collector takes action (which can be very late
considering that we set ZGC as the default in our start scripts).

### Modification
Change the complete internal code in a way that no more memory leaks are
occurring. Note that this change might be error prone, as external
plugins might use buffers in an invalid way that causes issues after the
change. External code is now required to acquire and release a buffer to
ensure it's not getting closed, rather than keeping a reference which
prevents the buffer from getting released.

### Result
No more internal memory leaks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: fix A pull request introducing a fix for a bug. v: 4.X This pull should be included in the 4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants