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..1764ce9acda5a --- /dev/null +++ b/bindings/pyroot/pythonizations/test/th1_addfunction.py @@ -0,0 +1,17 @@ +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..d23135539e419 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, since 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, since the destructor removes all entries before + // deleting the collection. + fFunctions->SetOwner(true); UseCurrentStyle();