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

[RF] Fix memory leaks in testRooCrystalBall #16067

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

guitargeek
Copy link
Contributor

The memory leaks were introduced on purpose in that test, but they are probably not necessary anymore.

Also, suppress unneeded logging and format code.

The memory leaks were introduced on purpose in that test, but they are
probably not necessary anymore.

Also, suppress unneeded logging and format code.
@guitargeek
Copy link
Contributor Author

The Windows failures are unrelated.

Copy link

Test Results

    13 files      13 suites   2d 23h 20m 46s ⏱️
 2 652 tests  2 650 ✅ 0 💤 2 ❌
32 658 runs  32 656 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 0d5ee5b.

@pcanal
Copy link
Member

pcanal commented Jul 19, 2024

Why were the leak introduced on purpose? Why are they no longer neeeded?

@guitargeek
Copy link
Contributor Author

guitargeek commented Jul 19, 2024

I wrote that test the first week that I worked in the ROOT team, and I had not much of a clue about RooFit or ROOT 😆 I don't remember anything in detail. In hindsight, it's probably because these RooFit objects reference each other and something used to depend on the order of destruction.

I randomly saw this again today and figured it's probably not needed anymore.

@guitargeek guitargeek merged commit 7730491 into root-project:master Jul 19, 2024
16 of 18 checks passed
@guitargeek guitargeek deleted the test_cb branch July 19, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants