From 87e8037d07c235f3572d23405f0ffac13af04b07 Mon Sep 17 00:00:00 2001 From: Lukas Johannes Breitwieser Date: Fri, 5 Jun 2026 15:22:27 +0200 Subject: [PATCH 1/3] [Python] Fix double free of objects added to TH1::fFunctions Fixes https://github.com/root-project/root/issues/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. --- .../pyroot/pythonizations/test/CMakeLists.txt | 1 + .../pythonizations/test/th1_addfunction.py | 15 +++++++++++++++ hist/hist/src/TH1.cxx | 16 ++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 bindings/pyroot/pythonizations/test/th1_addfunction.py diff --git a/bindings/pyroot/pythonizations/test/CMakeLists.txt b/bindings/pyroot/pythonizations/test/CMakeLists.txt index b1d8d00554388..8138f58bcca23 100644 --- a/bindings/pyroot/pythonizations/test/CMakeLists.txt +++ b/bindings/pyroot/pythonizations/test/CMakeLists.txt @@ -65,6 +65,7 @@ ROOT_ADD_PYUNITTEST(pyroot_pyz_th1 th1.py) # The above tests a deadlock. It should complete in about 1s. If we don't reduce the timeout, we need to wait 1500 s. set_tests_properties(pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-th1 PROPERTIES TIMEOUT 30) ROOT_ADD_PYUNITTEST(pyroot_pyz_th1_fillN th1_fillN.py PYTHON_DEPS numpy) +ROOT_ADD_PYUNITTEST(pyroot_pyz_th1_addfunction th1_addfunction.py) ROOT_ADD_PYUNITTEST(pyroot_pyz_th2_fillN th2_fillN.py PYTHON_DEPS numpy) ROOT_ADD_PYUNITTEST(pyroot_pyz_th2 th2.py) ROOT_ADD_PYUNITTEST(pyroot_pyz_th3 th3.py) diff --git a/bindings/pyroot/pythonizations/test/th1_addfunction.py b/bindings/pyroot/pythonizations/test/th1_addfunction.py new file mode 100644 index 0000000000000..c3ee3b9124607 --- /dev/null +++ b/bindings/pyroot/pythonizations/test/th1_addfunction.py @@ -0,0 +1,15 @@ +import unittest + +import ROOT + +class TH1AddFunction(unittest.TestCase): + def test_adding_a_function_does_not_segfault(self): + """ + This test verifies that box is only freed once + """ + h = ROOT.TH1F("h", "h", 10, 0, 1) + box = ROOT.TBox(0.1, 0.1, 0.9, 0.9) + h.GetListOfFunctions().Add(box) + +if __name__ == "__main__": + unittest.main() diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index c80cd293a0a36..1d2ec747d8542 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -623,6 +623,14 @@ TH1::TH1() { fDirectory = nullptr; fFunctions = new TList; + // 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. + fFunctions->SetOwner(true); fNcells = 0; fIntegral = nullptr; fPainter = nullptr; @@ -804,6 +812,14 @@ void TH1::Build() SetTitle(fTitle.Data()); fFunctions = new TList; + // 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. + fFunctions->SetOwner(true); UseCurrentStyle(); From a7420ff627255f294d85bdfba1908de69095f481 Mon Sep 17 00:00:00 2001 From: Lukas Johannes Breitwieser Date: Mon, 8 Jun 2026 10:32:15 +0200 Subject: [PATCH 2/3] Fix typos in code comments --- hist/hist/src/TH1.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index 1d2ec747d8542..d23135539e419 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -628,7 +628,7 @@ TH1::TH1() // (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 + // solution still works, since the destructor removes all entries before // deleting the collection. fFunctions->SetOwner(true); fNcells = 0; @@ -817,7 +817,7 @@ void TH1::Build() // (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 + // solution still works, since the destructor removes all entries before // deleting the collection. fFunctions->SetOwner(true); From 07b91f068c86e3e4676ccc29ebad1a77ab2b2bc0 Mon Sep 17 00:00:00 2001 From: Lukas Johannes Breitwieser Date: Mon, 8 Jun 2026 14:16:39 +0200 Subject: [PATCH 3/3] Apply ruff formatting suggestions --- bindings/pyroot/pythonizations/test/th1_addfunction.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bindings/pyroot/pythonizations/test/th1_addfunction.py b/bindings/pyroot/pythonizations/test/th1_addfunction.py index c3ee3b9124607..1764ce9acda5a 100644 --- a/bindings/pyroot/pythonizations/test/th1_addfunction.py +++ b/bindings/pyroot/pythonizations/test/th1_addfunction.py @@ -2,6 +2,7 @@ import ROOT + class TH1AddFunction(unittest.TestCase): def test_adding_a_function_does_not_segfault(self): """ @@ -11,5 +12,6 @@ def test_adding_a_function_does_not_segfault(self): box = ROOT.TBox(0.1, 0.1, 0.9, 0.9) h.GetListOfFunctions().Add(box) + if __name__ == "__main__": unittest.main()