-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[RF] Fix memory leaks in RooFitHS3 by completely avoiding manual memory management #9690
Conversation
Starting build on |
Build failed on mac1015/python3. Warnings:
|
bd0324b
to
5589595
Compare
Starting build on |
5589595
to
1112404
Compare
Starting build on |
The usecase for this overload is to transfer the ownership of RooAbsArgs from `std::unique_ptr` to another RooAbsArg, without having to call `release()` on the smart pointer, which is a code smell because ownership should always be clear without the smart pointer having to release it.
Calling `new` and `delete` is now completely avoided in RooFitHS3, and `std::unique_ptr` is always used instead.
1112404
to
3c309f1
Compare
Starting build on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you for the fix !
This PR fixes tons of memory leaks in RooFitHS3 by never using manual memory allocation in RooFitHS3.
To facilitate this, an overload of
RooAbsArg::addOwnedComponents
was added that takes transfers the ownership via smart pointers (otherwise one would have to use raw owning pointers orstd::unique_ptr<T>::release()
which I want to avoid).If the CI passes, I'll squash the second and third commit and add commit messages.