Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
@tmadlener tmadlener deleted the fix-mem-leak-buffers branch November 8, 2023 12:13
# for free to join this conversation on GitHub. Already have an account? # 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