[Python] Set memory policy to "strict"

The ROOT Python interfaces have many memory leaks, which is a major pain
point for people using it for long-running scripts in batch jobs.

One source of memory leaks was indentified to be the "heuristic memory
policy" of cppyy. This means that cppyy assumes that every non-const
pointer member function argument was interpreted as the object taking
ownership if the argument.

For examle, take the non-owning RooLinkedList container. It has a
`RooLinkedList::Add(RooAbsArg *arg)` method. ROOT wrongly assumes that
this means the RooLinkedList takes ownership of arg, and it drops the
ROOT overship. Nobody feels responsible for deleting the object
anymore, and there is a memory leak or `arg`.

That particular leak was reported in this forum post:
https://root-forum.cern.ch/t/memory-leak-in-fits/56249

Function parameters of type `T *` are very common in ROOT, and only
rarely do they imply ownership transfer. So changing the memory policy
to "strict" would surely fix also many other memory leaks that are not
reported so far. In fact, upstream cppyy doesn't even enable this
heuristic memory policy anymore by default! So moving ROOT also to the
strict memory policy closes the gap between ROOT and cppyy.

The potential drawback of this change are crashes in usercode if memory
is not properly managed. But these problems should either be fixed by:

  * the user

  * dedicated pythonizations for these methods to manage shared
    ownership via Python reference counters (i.e., setting the parameter
    as an attribute of the object that the member function was called
    on)

This follows up on PR root-project#4294, in particular it reverts 3a12063.

Note that owning **TCollection**, **TSeqCollection**, and **TList**
instances are Pythonized to preserve the old behavior of dropping Python
ownership of added elements, as that was the case where the memory
heuristic was correct.

Author: guitargeek

README/ReleaseNotes/v640/index.md

@@ -45,7 +45,7 @@ The following people have contributed to this new version:
 ## Removals
 
 * The `TH1K` class was removed. `TMath::KNNDensity` can be used in its stead.
-* The `TObject` equality operator pythonization (`TObject.__eq__`) that was deprecated in ROOT 6.38 and scheduled for removal in ROOT 6.40 is removed.
+* The `TObject` equality operator Pythonization (`TObject.__eq__`) that was deprecated in ROOT 6.38 and scheduled for removal in ROOT 6.40 is removed.
 * Comparing C++ `nullptr` objects with `None` in Python now raises a `TypeError`, as announced in the ROOT 6.38 release notes. Use truth-value checks like `if not x` or `x is None` instead.
 * The `TGLIncludes.h` and `TGLWSIncludes.h` that were deprecated in ROOT 6.38 and scheduled for removal are gone now. Please include your required headers like `<GL/gl.h>` or `<GL/glu.h>` directly.
 * The GLEW headers (`GL/eglew.h`, `GL/glew.h`, `GL/glxew.h`, and `GL/wglew.h`) that were installed when building ROOT with `builtin_glew=ON` are no longer installed. This is done because ROOT is moving away from GLEW for loading OpenGL extensions.
@@ -236,6 +236,113 @@ The new implementation is statistically consistent and recommended.
 
 - ROOT dropped support for Python 3.9, meaning ROOT now requires at least Python 3.10.
 
+
+### Change in memory ownership heuristics
+
+In previous ROOT versions, if a C++ member function took a non-`const` raw pointer, e.g.
+```C++
+MyClass::add(T* obj)
+```
+then calling this method from Python on an instance
+```Python
+my_instance.add(obj)
+```
+would assume that ownership of `obj` is transferred to `my_instance`.
+
+In practice, many such functions do not actually take ownership.
+As a result, this heuristic caused several memory leaks.
+
+Starting with ROOT 6.40, the ROOT Python interfaces no longer assumes ownership transfer for non-`const` raw pointer arguments.
+
+#### What does this mean for you?
+
+Because Python no longer automatically relinquishes ownership, some code that previously relied on the old heuristic may now expose:
+
+  * **Dangling references** on the C++ side
+  * **Double deletes**
+
+These issues must now be fixed by managing object lifetimes explicitly.
+
+#### Dangling references
+
+A dangling reference occurs when C++ holds a pointer or reference to an object that has already been deleted on the Python side.
+
+*Example*
+```Python
+obj = ROOT.MyClass()
+my_instance.foo(obj)
+del obj  # C++ may still hold a pointer: dangling reference
+```
+
+When the Python object is deleted, the memory associated with the C++ object
+ is also freed. But the C++ side may want to delete the object as well, for
+instance if the destructor of the class of `my_instance` also calls `delete obj`.
+
+*Possible remedies*
+
+ 1. **Keep the Python object alive**
+
+    Assign the object to a Python variable that lives at least as long as the C++ reference.
+
+    *Example:* Python lifeline
+    ```Python
+    obj = ROOT.MyClass()
+    my_instance.foo(obj)
+    # Setting `obj` as an attribute of `my_instance` makes sure that it's not
+    # garbage collected before `my_instance` is deleted:
+    my_instance._owned_obj = obj
+    del obj  # Reference counter for `obj` doesn't hit zero here
+    ```
+    Setting the lifeline reference could also be done in a user-side Pythonization of `MyClass::foo`.
+
+ 3. **Transfer ownership explicitly to C++**
+      * Drop the ownership on the Python side:
+        ```Python
+        ROOT.SetOwnership(obj, False)
+        ```
+      * Ensure that the C++ side will take ownership instead, e.g. by explicitly calling `delete` in the class destructor or using smart pointers like `std::unique_ptr`.
+
+ 3. **Rely on Pythonizations that imply ownership transfer**
+
+    If the object is stored in a non-owning collection such as a default-constructed `TCollection` (e.g. `TList`), you can make the collection owning before adding any elements:
+    ```Python
+    coll.SetOwner(True)
+    ```
+    This will imply ownership transfer to the C++ side when adding elements with `TCollection::Add()`.
+
+#### Note on **TCollection** Pythonization
+
+`TCollection`-derived classes are Pythonized such that when an object is added to an owning collection via `TCollection::Add()`, Python ownership is automatically dropped.
+
+If you identify other cases where such a Pythonization would be beneficial, please report them via a GitHub issue. Users can also implement custom Pythonizations outside ROOT if needed.
+
+#### Double deletes
+
+A double delete indicates that C++ already owns the object, but Python still attempts to delete it.
+
+In this case, you do not need to ensure C++ ownership, as it already exists.
+Instead, ensure that Python does not delete the object.
+
+*Possible remedies*
+
+ 1. Drop Python ownership explicitly:
+    ```Python
+    ROOT.SetOwnership(obj, False)
+    ```
+
+ 2. Pythonize the relevant member function to automatically drop ownership on the Python side (similar to the `TCollection` Pythonization described above).
+
+#### Temporary compatibility option
+
+You can temporarily restore the old heuristic by calling:
+```Python
+ROOT.SetHeuristicMemoryPolicy(True)
+```
+
+after importing ROOT.
+
+This option is intended **for debugging only and will be removed in ROOT 6.44**.
+
 ### UHI
 #### Backwards incompatible changes
 - `TH1.values()` now returns a **read-only** NumPy array by default. Previously it returned a writable array that allowed modifying histogram contents implicitly.

bindings/pyroot/pythonizations/python/ROOT/_facade.py

@@ -83,6 +83,18 @@ def __init__(self, module, is_ipython):
         self.__class__.__getattr__ = self._getattr
         self.__class__.__setattr__ = self._setattr
 
+    def SetHeuristicMemoryPolicy(self, enabled):
+        import textwrap
+        import warnings
+
+        msg = """ROOT.SetHeuristicMemoryPolicy() is deprecated and will be removed in ROOT 6.44.
+        Since ROOT 6.40, the heuristic memory policy is disabled by default, and with
+        ROOT 6.44 it won't be possible to re-enable it with ROOT.SetHeuristicMemoryPolicy(),
+        which was only meant to be used during a transition period and will be removed.
+        """
+        warnings.warn(textwrap.dedent(msg), FutureWarning, stacklevel=0)
+        return self._cppyy._backend.SetHeuristicMemoryPolicy(enabled)
+
     def AddressOf(self, obj):
         # Return an indexable buffer of length 1, whose only element
         # is the address of the object.
@@ -172,7 +184,6 @@ def _finalSetup(self):
             "bind_object",
             "as_cobject",
             "addressof",
-            "SetHeuristicMemoryPolicy",
             "SetImplicitSmartPointerConversion",
             "SetOwnership",
         ]
@@ -197,11 +208,6 @@ def _finalSetup(self):
         if not self.gROOT.IsBatch() and self.PyConfig.StartGUIThread:
             self.app.init_graphics(self._cppyy.gbl.gEnv, self._cppyy.gbl.gSystem)
 
-        # Set memory policy to kUseHeuristics.
-        # This restores the default in PyROOT which was changed
-        # by new Cppyy
-        self.SetHeuristicMemoryPolicy(True)
-
         # The automatic conversion of ordinary obejcts to smart pointers is
         # disabled for ROOT because it can cause trouble with overload
         # resolution. If a function has overloads for both ordinary objects and

bindings/pyroot/pythonizations/test/root_module.py

@@ -105,6 +105,27 @@ def test_import_nested_submodules(self):
         if root_module_has("RooFit.Evaluator"):
             from ROOT.RooFit import Evaluator
 
+    def test_deprecated_features(self):
+        """
+        Verify that deprecated features are removed in the release where we
+        announced that they will be removed.
+
+        This serves as a reminder to us to delete the deprecated code after
+        increasing the version number.
+
+        Once a deprecated feature was deleted, we can remove the corresponding
+        check in this test.
+        """
+        import ROOT
+
+        version = tuple(int(n) for n in ROOT.__version__.split("."))
+
+        if version >= (6, 44, 0):
+            # Verify that SetHeuristicMemoryPolicy is not available
+            with self.assertRaises(AttributeError):
+                ROOT.SetHeuristicMemoryPolicy(True)
+                ROOT.SetHeuristicMemoryPolicy(False)
+
 
 if __name__ == "__main__":
     unittest.main()