Skip to content

[Python] Fix double free of objects added to TH1::fFunctions#22493

Merged
LukasBreitwieser merged 3 commits into
root-project:masterfrom
LukasBreitwieser:issue-22417
Jun 9, 2026
Merged

[Python] Fix double free of objects added to TH1::fFunctions#22493
LukasBreitwieser merged 3 commits into
root-project:masterfrom
LukasBreitwieser:issue-22417

Conversation

@LukasBreitwieser

Copy link
Copy Markdown
Contributor

Provide the correct signal to the python bindings that calling th1->GetListOfFunctions()->Add(object) will take ownership of the object.

ROOT's Python bindings use TCollection::IsOwner to decide whether objects inserted into a collection (like TH1::fFunctions) should be deleted (see _TCollection_Add pythonization). To ensure the Python bindings get the correct ownership signal, set the ownership on TH1::fFunctions. Although, in reality, TH1's destructor actually handles ownership, this solution still works, sinde the destructor removes all entries before deleting the collection.

This PR fixes #22417.

Fixes root-project#22417
Provide the correct signal to the python bindings that calling
`th1->GetListOfFunctions()->Add(object)` will take ownership of
the object.

ROOT's Python bindings use TCollection::IsOwner to decide whether objects
inserted into a collection (like `TH1::fFunctions`) should be deleted
(see `_TCollection_Add` pythonization).  To ensure the Python bindings
get the correct ownership signal, set the ownership on TH1::fFunctions.
Although, in reality, TH1's destructor actually handles ownership, this
solution still works, sinde the destructor removes all entries before
deleting the collection.
Comment thread hist/hist/src/TH1.cxx
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 21h 12m 41s ⏱️
 3 863 tests  3 863 ✅ 0 💤 0 ❌
76 312 runs  76 312 ✅ 0 💤 0 ❌

Results for commit 07b91f0.

♻️ This comment has been updated with latest results.

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have re-checked the source code and I agree this should be the right approach without change of existing functionality. Thanks!

Comment thread hist/hist/src/TH1.cxx Outdated
Comment thread hist/hist/src/TH1.cxx Outdated
@LukasBreitwieser

Copy link
Copy Markdown
Contributor Author

Thank you for your review @vepadulano!

@LukasBreitwieser LukasBreitwieser merged commit b2cb5ef into root-project:master Jun 9, 2026
33 checks passed
@dpiparo

dpiparo commented Jun 9, 2026

Copy link
Copy Markdown
Member

/backport to 6.40

@root-project-bot

Copy link
Copy Markdown

Preparing to backport PR #22493 to branch 6.40 requested by dpiparo

@root-project-bot

Copy link
Copy Markdown

Something went wrong with the creation of the PR to backport to 6.40: @dpiparo please see the logs

@root-project-bot

Copy link
Copy Markdown

This PR has been backported to

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.

6.40: TBox added to TH1::GetListOfFunctions() appears to be freed before Write

4 participants