[PyROOT] Fix memory leak in TTree __getattr__ pythonization

As reported on the forum:
https://root-forum.cern.ch/t/memory-leak-in-pyroot-when-using-user-defined-c-macros-from-python-and-ttree-friends/59432

I fixed the memory leak in the Pythonization is in the usual way how I
fix problems with the PyROOT CPython extension: re-implementing the
offending parts in C++ and hoping that the problem is gone. Which it is!

bindings/pyroot/pythonizations/python/ROOT/_pythonization/_ttree.py

@@ -130,7 +130,7 @@
 */
 '''
 
-from libROOTPythonizations import AddBranchAttrSyntax, SetBranchAddressPyz, BranchPyz
+from libROOTPythonizations import GetBranchAttr, SetBranchAddressPyz, BranchPyz
 from . import pythonization
 
 # TTree iterator
@@ -168,6 +168,15 @@ def _Branch(self, *args):
 
     return res
 
+def _TTree__getattr__(self, key):
+
+    import cppyy.ll
+
+    out, cast_type = GetBranchAttr(self, key)
+    if cast_type:
+        out = cppyy.ll.cast[cast_type](out)
+    return out
+
 @pythonization('TTree')
 def pythonize_ttree(klass, name):
     # Parameters:
@@ -183,7 +192,7 @@ def pythonize_ttree(klass, name):
     klass.__iter__ = _TTree__iter__
 
     # tree.branch syntax
-    AddBranchAttrSyntax(klass)
+    klass.__getattr__ = _TTree__getattr__
 
     # SetBranchAddress
     klass._OriginalSetBranchAddress = klass.SetBranchAddress

bindings/pyroot/pythonizations/src/PyROOTModule.cxx

@@ -42,7 +42,7 @@ PyObject *gRootModule = 0;
 static PyMethodDef gPyROOTMethods[] = {
    {(char *)"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_VARARGS,
     (char *)"Add a custom pickling mechanism for Cppyy Python proxy objects"},
-   {(char *)"AddBranchAttrSyntax", (PyCFunction)PyROOT::AddBranchAttrSyntax, METH_VARARGS,
+   {(char *)"GetBranchAttr", (PyCFunction)PyROOT::GetBranchAttr, METH_VARARGS,
     (char *)"Allow to access branches as tree attributes"},
    {(char *)"AddTClassDynamicCastPyz", (PyCFunction)PyROOT::AddTClassDynamicCastPyz, METH_VARARGS,
     (char *)"Cast the void* returned by TClass::DynamicCast to the right type"},

bindings/pyroot/pythonizations/src/PyROOTPythonize.h

@@ -20,7 +20,7 @@ PyObject *AddCPPInstancePickling(PyObject *self, PyObject *args);
 
 PyObject *AddPrettyPrintingPyz(PyObject *self, PyObject *args);
 
-PyObject *AddBranchAttrSyntax(PyObject *self, PyObject *args);
+PyObject *GetBranchAttr(PyObject *self, PyObject *args);
 PyObject *BranchPyz(PyObject *self, PyObject *args);
 PyObject *SetBranchAddressPyz(PyObject *self, PyObject *args);
 

bindings/pyroot/pythonizations/src/TTreePyz.cxx

@@ -75,30 +75,30 @@ static TLeaf *SearchForLeaf(TTree *tree, const char *name, TBranch *branch)
    return leaf;
 }
 
-static PyObject *BindBranchToProxy(TTree *tree, const char *name, TBranch *branch)
+static std::pair<void *, std::string> ResolveBranch(TTree *tree, const char *name, TBranch *branch)
 {
    // for partial return of a split object
    if (branch->InheritsFrom(TBranchElement::Class())) {
       TBranchElement *be = (TBranchElement *)branch;
       if (be->GetCurrentClass() && (be->GetCurrentClass() != be->GetTargetClass()) && (0 <= be->GetID())) {
          Long_t offset = ((TStreamerElement *)be->GetInfo()->GetElements()->At(be->GetID()))->GetOffset();
-         return CPyCppyy::Instance_FromVoidPtr(be->GetObject() + offset, be->GetCurrentClass()->GetName());
+         return {be->GetObject() + offset, be->GetCurrentClass()->GetName()};
       }
    }
 
    // for return of a full object
    if (branch->IsA() == TBranchElement::Class() || branch->IsA() == TBranchObject::Class()) {
       TClass *klass = TClass::GetClass(branch->GetClassName());
       if (klass && branch->GetAddress())
-         return CPyCppyy::Instance_FromVoidPtr(*(void **)branch->GetAddress(), branch->GetClassName());
+         return {*(void **)branch->GetAddress(), branch->GetClassName()};
 
       // try leaf, otherwise indicate failure by returning a typed null-object
       TObjArray *leaves = branch->GetListOfLeaves();
       if (klass && !tree->GetLeaf(name) && !(leaves->GetSize() && (leaves->First() == leaves->Last())))
-         return CPyCppyy::Instance_FromVoidPtr(nullptr, branch->GetClassName());
+         return {nullptr, branch->GetClassName()};
    }
 
-   return nullptr;
+   return {nullptr, ""};
 }
 
 static PyObject *WrapLeaf(TLeaf *leaf)
@@ -142,8 +142,18 @@ static PyObject *WrapLeaf(TLeaf *leaf)
 }
 
 // Allow access to branches/leaves as if they were data members
-PyObject *GetAttr(PyObject *self, PyObject *pyname)
+// Returns a Python tuple where the first element is either the desired
+// CPyCppyy proxy, or an address that still needs to be wrapped by the caller
+// in a proxy using cppyy.ll.cast. In the latter laster, the second tuple
+// element is the target type name. Otherwise, the second element is an empty
+// string.
+PyObject *PyROOT::GetBranchAttr(PyObject * /*self*/, PyObject *args)
 {
+   PyObject *self = nullptr;
+   PyObject *pyname = nullptr;
+
+   PyArg_ParseTuple(args, "OU:GetBranchAttr", &self, &pyname);
+
    const char *name_possibly_alias = PyUnicode_AsUTF8(pyname);
    if (!name_possibly_alias)
       return 0;
@@ -166,41 +176,35 @@ PyObject *GetAttr(PyObject *self, PyObject *pyname)
 
    if (branch) {
       // found a branched object, wrap its address for the object it represents
-      auto proxy = BindBranchToProxy(tree, name, branch);
-      if (proxy != nullptr)
-         return proxy;
+      void * finalAddress = nullptr;
+      std::string finalType;
+      std::tie(finalAddress, finalType) = ResolveBranch(tree, name, branch);
+      if (!finalType.empty()) {
+         PyObject *outTuple = PyTuple_New(2);
+         //PyObject *proxy = CPyCppyy::Instance_FromVoidPtr(finalAddress, finalType);
+         PyTuple_SET_ITEM(outTuple, 0, PyLong_FromLongLong((intptr_t)finalAddress)); // does this work on 32 bit??
+         PyTuple_SET_ITEM(outTuple, 1, CPyCppyy_PyText_FromString((finalType + "*").c_str()));
+         return outTuple;
+      }
    }
 
    // if not, try leaf
-   TLeaf *leaf = SearchForLeaf(tree, name, branch);
-
-   if (leaf) {
+   if (TLeaf *leaf = SearchForLeaf(tree, name, branch)) {
       // found a leaf, extract value and wrap with a Python object according to its type
       auto wrapper = WrapLeaf(leaf);
-      if (wrapper != nullptr)
-         return wrapper;
+      if (wrapper != nullptr) {
+         PyObject *outTuple = PyTuple_New(2);
+         PyTuple_SET_ITEM(outTuple, 0, wrapper);
+         PyTuple_SET_ITEM(outTuple, 1, CPyCppyy_PyText_FromString(""));
+         return outTuple;
+      }
    }
 
    // confused
    PyErr_Format(PyExc_AttributeError, "\'%s\' object has no attribute \'%s\'", tree->IsA()->GetName(), name);
    return 0;
 }
 
-////////////////////////////////////////////////////////////////////////////
-/// \brief Allow branches to be accessed as attributes of a tree.
-/// \param[in] self Always null, since this is a module function.
-/// \param[in] args Pointer to a Python tuple object containing the arguments
-/// received from Python.
-///
-/// Allow access to branches/leaves as if they were Python data attributes of the tree
-/// (e.g. mytree.branch)
-PyObject *PyROOT::AddBranchAttrSyntax(PyObject * /* self */, PyObject *args)
-{
-   PyObject *pyclass = PyTuple_GetItem(args, 0);
-   Utility::AddToClass(pyclass, "__getattr__", (PyCFunction)GetAttr, METH_O);
-   Py_RETURN_NONE;
-}
-
 ////////////////////////////////////////////////////////////////////////////
 /// \brief Add pythonization for TTree::SetBranchAddress.
 /// \param[in] self Always null, since this is a module function.