Skip to content

Fix RooFit memory leaks with TIterator usage#7631

Merged
linev merged 7 commits intoroot-project:masterfrom
linev:roofit_tidy2
Mar 22, 2021
Merged

Fix RooFit memory leaks with TIterator usage#7631
linev merged 7 commits intoroot-project:masterfrom
linev:roofit_tidy2

Conversation

@linev
Copy link
Copy Markdown
Member

@linev linev commented Mar 22, 2021

Fixing #7619
In very many places in RooFit temporary TIterator instances not deleted.
In such places I replace it by

TIter iter = collection->MakeIterator();

In some sense TIter class is equivalent to std::unique_ptr<TIterator>

@linev linev self-assigned this Mar 22, 2021
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@linev linev linked an issue Mar 22, 2021 that may be closed by this pull request
[RF] Multiple number of memory leaks in RooFit #7619
Closed
@linev linev requested review from guitargeek and removed request for Axel-Naumann and couet March 22, 2021 08:09
lmoneta
lmoneta approved these changes Mar 22, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

Thank you Sergei for changing this. It looks good to me.
The only comment is that I would have changed to use range based iterators.

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2021-03-22T08:30:07.906Z] collect2: fatal error: ld terminated with signal 9 [Killed]

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-performance-centos8-multicore/default.
Running on null:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-fedora30/cxx14.
Running on root-fedora30-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 9 more

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

  • [2021-03-22T08:56:11.943Z] C:\build\workspace\root-pullrequests-build\root\roofit\RDataFrameHelpers\test\testActionHelpers.cxx(22,73): error C3493: 'nEvent' cannot be implicitly captured because no default capture mode has been specified [C:\build\workspace\root-pullrequests-build\build\roofit\RDataFrameHelpers\test\testActionHelpers.vcxproj]
  • [2021-03-22T08:56:21.935Z] C:\build\workspace\root-pullrequests-build\root\roofit\RDataFrameHelpers\test\testActionHelpers.cxx(23,73): error C3493: 'nEvent' cannot be implicitly captured because no default capture mode has been specified [C:\build\workspace\root-pullrequests-build\build\roofit\RDataFrameHelpers\test\testActionHelpers.vcxproj]

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

And 26 more

@linev
Copy link
Copy Markdown
Member Author

linev commented Mar 22, 2021

@phsft-bot build

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Mar 22, 2021

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@linev linev merged commit c7f23f1 into root-project:master Mar 22, 2021
@linev linev deleted the roofit_tidy2 branch March 22, 2021 15:24
@lmoneta lmoneta mentioned this pull request Apr 19, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lmoneta lmoneta lmoneta approved these changes

@hageboeck hageboeck Awaiting requested review from hageboeck

@guitargeek guitargeek Awaiting requested review from guitargeek guitargeek is a code owner

Assignees

@linev linev

Labels

affects:master bug in:RooFit priority:high

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[RF] Multiple number of memory leaks in RooFit

3 participants

@linev @phsft-bot @lmoneta