Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyROOT] Fix infinite loop in TMemoryRegulator::ClearProxiedObjects() #15106

Merged
merged 2 commits into from Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion bindings/pyroot/pythonizations/CMakeLists.txt
Expand Up @@ -112,7 +112,6 @@ set(cpp_sources
src/TMemoryRegulator.cxx
src/TObjectPyz.cxx
src/TTreePyz.cxx
src/PyzCppHelpers.cxx
src/CPPInstancePyz.cxx
src/TPyDispatcher.cxx
inc/TPyDispatcher.h
Expand Down
7 changes: 3 additions & 4 deletions bindings/pyroot/pythonizations/src/CPPInstancePyz.cxx
Expand Up @@ -17,7 +17,6 @@
#include "../../cppyy/CPyCppyy/src/CustomPyTypes.h"

#include "PyROOTPythonize.h"
#include "PyzCppHelpers.hxx"
#include "TBufferFile.h"

using namespace CPyCppyy;
Expand Down Expand Up @@ -79,7 +78,7 @@ PyObject *op_reduce(PyObject *self, PyObject * /*args*/)
if (selfClass == s_bfClass) {
buff = (TBufferFile *)CPyCppyy::Instance_AsVoidPtr(self);
} else {
auto className = GetScopedFinalNameFromPyObject(self);
auto className = Cppyy::GetScopedFinalName(selfClass);
if (className.find("__cppyy_internal::Dispatcher") == 0) {
PyErr_Format(PyExc_IOError, "generic streaming of Python objects whose class derives from a C++ class is not supported. "
"Please refer to the Python pickle documentation for instructions on how to define "
Expand All @@ -94,7 +93,7 @@ PyObject *op_reduce(PyObject *self, PyObject * /*args*/)
if (s_buff.WriteObjectAny(CPyCppyy::Instance_AsVoidPtr(self),
TClass::GetClass(className.c_str())) != 1) {
PyErr_Format(PyExc_IOError, "could not stream object of type %s",
GetScopedFinalNameFromPyObject(self).c_str());
Cppyy::GetScopedFinalName(selfClass).c_str());
return 0;
}
buff = &s_buff;
Expand All @@ -104,7 +103,7 @@ PyObject *op_reduce(PyObject *self, PyObject * /*args*/)
// on reading back in (see CPPInstanceExpand defined above)
PyObject *res2 = PyTuple_New(2);
PyTuple_SET_ITEM(res2, 0, PyBytes_FromStringAndSize(buff->Buffer(), buff->Length()));
PyTuple_SET_ITEM(res2, 1, PyBytes_FromString(GetScopedFinalNameFromPyObject(self).c_str()));
PyTuple_SET_ITEM(res2, 1, PyBytes_FromString(Cppyy::GetScopedFinalName(selfClass).c_str()));

PyObject *result = PyTuple_New(2);
Py_INCREF(s_expand);
Expand Down
11 changes: 10 additions & 1 deletion bindings/pyroot/pythonizations/src/GenericPyz.cxx
Expand Up @@ -14,17 +14,26 @@
#include "CPyCppyy/API.h"

#include "../../cppyy/CPyCppyy/src/CPyCppyy.h"
#include "../../cppyy/CPyCppyy/src/CPPInstance.h"
#include "../../cppyy/CPyCppyy/src/Utility.h"

#include "PyROOTPythonize.h"
#include "PyzCppHelpers.hxx"

#include "TClass.h"
#include "TInterpreter.h"
#include "TInterpreterValue.h"

#include <map>

namespace {

std::string GetScopedFinalNameFromPyObject(const PyObject *pyobj)
{
return Cppyy::GetScopedFinalName(((CPyCppyy::CPPInstance *)pyobj)->ObjectIsA());
}

} // namespace

using namespace CPyCppyy;

// We take as unique identifier the declId of the class to
Expand Down
54 changes: 0 additions & 54 deletions bindings/pyroot/pythonizations/src/PyzCppHelpers.cxx

This file was deleted.

27 changes: 0 additions & 27 deletions bindings/pyroot/pythonizations/src/PyzCppHelpers.hxx

This file was deleted.

15 changes: 5 additions & 10 deletions bindings/pyroot/pythonizations/src/TClassPyz.cxx
Expand Up @@ -17,7 +17,6 @@
#include "../../cppyy/CPyCppyy/src/Utility.h"

#include "PyROOTPythonize.h"
#include "PyzCppHelpers.hxx"

// ROOT
#include "TClass.h"
Expand Down Expand Up @@ -51,15 +50,11 @@ PyObject *TClassDynamicCastPyz(PyObject *self, PyObject *args)
Utility::GetBuffer(pyobject, '*', 1, address, false);
}

// Now use binding to return a usable class
TClass *klass = nullptr;
if (up) {
// Upcast: result is a base
klass = (TClass *)GetTClass(pyclass)->DynamicCast(TClass::Class(), CPyCppyy::Instance_AsVoidPtr(pyclass));
} else {
// Downcast: result is a derived
klass = (TClass *)GetTClass(self)->DynamicCast(TClass::Class(), cl1);
}
// Now use binding to return a usable class. Upcast: result is a base.
// Downcast: result is a derived.
Cppyy::TCppType_t cpptype = ((CPyCppyy::CPPInstance*)(up ? pyclass : self))->ObjectIsA();
TClass *tcl = TClass::GetClass(Cppyy::GetScopedFinalName(cpptype).c_str());
TClass *klass = (TClass *)tcl->DynamicCast(TClass::Class(), up ? CPyCppyy::Instance_AsVoidPtr(pyclass) : cl1);

return CPyCppyy::Instance_FromVoidPtr(address, klass->GetName());
}
Expand Down
5 changes: 4 additions & 1 deletion bindings/pyroot/pythonizations/src/TMemoryRegulator.cxx
Expand Up @@ -108,7 +108,10 @@ void PyROOT::TMemoryRegulator::ClearProxiedObjects()
else {
// Non-owning proxy, just unregister to clean tables.
// The proxy deletion by Python will have no effect on C++, so all good
MemoryRegulator::UnregisterPyObject(pyobj, pyclass);
bool ret = MemoryRegulator::UnregisterPyObject(pyobj, pyclass);
if (!ret) {
fObjectMap.erase(elem);
}
}
}
}
23 changes: 22 additions & 1 deletion bindings/pyroot/pythonizations/src/TObjectPyz.cxx
Expand Up @@ -17,11 +17,32 @@
#include "../../cppyy/CPyCppyy/src/Utility.h"

#include "PyROOTPythonize.h"
#include "PyzCppHelpers.hxx"

// ROOT
#include "TObject.h"

namespace {

// Convert generic python object into a boolean value
PyObject *BoolNot(PyObject *value)
{
if (PyObject_IsTrue(value) == 1) {
Py_DECREF(value);
Py_RETURN_FALSE;
} else {
Py_XDECREF(value);
Py_RETURN_TRUE;
}
}

// Call method with signature: obj->meth(arg1)
PyObject *CallPyObjMethod(PyObject *obj, const char *meth, PyObject *arg1)
{
return PyObject_CallMethod(obj, meth, "O", arg1);
}

} // namespace

using namespace CPyCppyy;

// Implement Python's __eq__ with TObject::IsEqual
Expand Down
11 changes: 10 additions & 1 deletion bindings/pyroot/pythonizations/src/TTreePyz.cxx
Expand Up @@ -24,7 +24,6 @@
#include "CPyCppyy/API.h"

#include "PyROOTPythonize.h"
#include "PyzCppHelpers.hxx"

// ROOT
#include "TClass.h"
Expand All @@ -38,6 +37,16 @@
#include "TStreamerElement.h"
#include "TStreamerInfo.h"

namespace {

// Get the TClass of the C++ object proxied by pyobj
TClass *GetTClass(const PyObject *pyobj)
{
return TClass::GetClass(Cppyy::GetScopedFinalName(((CPyCppyy::CPPInstance*)pyobj)->ObjectIsA()).c_str());
}

} // namespace

using namespace CPyCppyy;

static TBranch *SearchForBranch(TTree *tree, const char *name)
Expand Down