Skip to content

Commit 77c6567

Browse files
guitargeekdpiparo
authored andcommitted
[PyROOT] Disable memory regulator
This feature arguably causes more harm than good. The idea of the memory regulator was to fix user errors like this: ``` ROOT.gInterpreter.Declare(""" void consume(TTree & t) { delete &t; } """) t = ROOT.TTree("tree", "tree") ROOT.consume(t) print(t) ``` Instead of a segfault because `t` was deleted, currently ROOT will print: ``` None ``` Basically, it detected if any object registered in `gROOT` was deleted somewhere on the C++ side to notify CPyCppyy, which reacts by swapping a cppyy-specific "None" type into the Python proxy. However, such proxies are in a state that causes the garbage collector to segfault in specific environments, and this is not the only problem the memory regulator has. It was also the source of many poorly understood crashes in the past. One other problem is that it only works for types that are registered in `gROOT`, like TTree, TFile, TH1, etc. If a user tries to access any other C++ class via Python after it was deleted, it will still segfault. So the memory regulator only sporadically solves the problem and results in an inconsistent user experience: the "ROOT-tracked" classes behave even more different than they already do. Therefore, this commit suggests to disable the PyROOT memory regulator. (cherry picked from commit 880d681)
1 parent 2061235 commit 77c6567

File tree

4 files changed

+0
-171
lines changed

4 files changed

+0
-171
lines changed

bindings/pyroot/pythonizations/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ set(cpp_sources
121121
src/RPyROOTApplication.cxx
122122
src/GenericPyz.cxx
123123
src/TClassPyz.cxx
124-
src/TMemoryRegulator.cxx
125124
src/TObjectPyz.cxx
126125
src/TTreePyz.cxx
127126
src/CPPInstancePyz.cxx

bindings/pyroot/pythonizations/src/PyROOTModule.cxx

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
// Bindings
1313
#include "PyROOTPythonize.h"
1414
#include "RPyROOTApplication.h"
15-
#include "TMemoryRegulator.h"
1615

1716
// Cppyy
1817
#include "CPyCppyy/API.h"
@@ -63,20 +62,6 @@ PyObject *RegisterExecutorAlias(PyObject * /*self*/, PyObject *args)
6362

6463
} // namespace PyROOT
6564

66-
namespace {
67-
68-
PyROOT::RegulatorCleanup &GetRegulatorCleanup()
69-
{
70-
// The object is thread-local because it can happen that we call into
71-
// C++ code (from the PyROOT CPython extension, from CPyCppyy or from cling)
72-
// from different Python threads. A notable example is within a distributed
73-
// RDataFrame application running on Dask.
74-
thread_local PyROOT::RegulatorCleanup m;
75-
return m;
76-
}
77-
78-
} // namespace
79-
8065
// Methods offered by the interface
8166
static PyMethodDef gPyROOTMethods[] = {
8267
{(char *)"AddCPPInstancePickling", (PyCFunction)PyROOT::AddCPPInstancePickling, METH_VARARGS,
@@ -164,9 +149,6 @@ extern "C" PyObject *PyInit_libROOTPythonizations()
164149
PyEval_InitThreads();
165150
#endif
166151

167-
// Memory management
168-
gROOT->GetListOfCleanups()->Add(&GetRegulatorCleanup());
169-
170152
// Make sure the interpreter is initialized once gROOT has been initialized
171153
TInterpreter::Instance();
172154

bindings/pyroot/pythonizations/src/TMemoryRegulator.cxx

Lines changed: 0 additions & 84 deletions
This file was deleted.

bindings/pyroot/pythonizations/src/TMemoryRegulator.h

Lines changed: 0 additions & 68 deletions
This file was deleted.

0 commit comments

Comments
 (0)