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

Fixes #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, since the destructor removes all entries before
deleting the collection.

Author: LukasBreitwieser

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)

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()

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();