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

Make it possible to dispose of unconsumed collection buffers #502

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Oct 7, 2023

BEGINRELEASENOTES

ENDRELEASENOTES

Fixes #500

@tmadlener tmadlener changed the title [WIP] Add unittest to cover parts the CollectionBufferFactory Make it possible to dispose of unconsumed collection buffers Oct 10, 2023
@tmadlener tmadlener force-pushed the fix-mem-leak-buffers branch from 9f8aaa0 to e0c71cf Compare October 11, 2023 06:50
@tmadlener
Copy link
Collaborator Author

This should fix #500 (still have to test that on a larger scale) but it is a bit cumbersome and the "proper" solution would be to make the CollectionReadBuffers owners of their data and giving them move-only semantics such that the flow of ownership of the data is expressed much more clearly.

I would be happy to merge this as is to fix the current leak and then address the "proper" solution after v1.0, because I am not entirely sure whether it can be made to work with the EventStore.

@tmadlener tmadlener force-pushed the fix-mem-leak-buffers branch from e0c71cf to 1d877f7 Compare October 11, 2023 07:41
@hegner
Copy link
Collaborator

hegner commented Oct 11, 2023

Is it WIP or are you finished with this now? I am asking because of more commits coming in. Thanks

@tmadlener
Copy link
Collaborator Author

Just finished a few more larger scale tests and valgrind and this fixes the leaks of unconsumed collection buffers.

@hegner hegner self-requested a review October 11, 2023 14:14
@hegner hegner merged commit 9def6a3 into AIDASoft:master Oct 11, 2023
17 checks passed
@tmadlener tmadlener deleted the fix-mem-leak-buffers branch November 8, 2023 12:13
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.

ROOTFrameData leaks collection buffers that are not requested by the Frame
2 participants