PyROOT] Fix infinite loop in TMemoryRegulator::ClearProxiedObjects()
#15106
Conversation
guitargeek
requested review from
bellenot,
dpiparo and
vepadulano
as code owners
|
Starting build on |
guitargeek
changed the title
TMemoryRegulator::ClearProxiedObjects()
Test Results 12 files 12 suites 1d 23h 1m 18s ⏱️ Results for commit e1bdab8. ♻️ This comment has been updated with latest results. |
|
Build failed on ROOT-ubuntu2204/nortcxxmod. Failing tests: |
There is an infinite loop in
`PyROOT::TMemoryRegulator::ClearProxiedObjects()`:
```C++
while (!fObjectMap.empty()) {
auto elem = fObjectMap.begin();
auto cppobj = elem->first;
auto klassid = elem->second;
auto pyclass = CreateScopeProxy(klassid);
auto pyobj = (CPPInstance *)MemoryRegulator::RetrievePyObject(cppobj, pyclass);
if (pyobj && (pyobj->fFlags & CPPInstance::kIsOwner)) {
...
} 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);
}
}
```
In the second code branch, the object is not removed from the `fObjMap`,
if `UnregisterPyObject` doesn't call the unregister hook because it
quits early. This can happen if there is not C++ object corresponding to
the regulated python object, causing an infinite loop because
`fObjectMap` never gets empty.
Although this seems like an obvious logic error, this was only noticed
after the cppyy upgrade. Probably the code path was not hit before.
Addresses
root-project#15085 (comment).
Most of these helpers were single-use, so it's better to inline them in the implementations.
guitargeek
force-pushed
the
infinite_loop
branch
from
c4e8095
to
e1bdab8
Compare
|
Starting build on |
|
I restarted the jobs to make sure the failure on Windows was a glitch. A test timedout for a reason which was not clear to me. |
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
There is an infinite loop in
PyROOT::TMemoryRegulator::ClearProxiedObjects():In the second code branch, the object is not removed from the
fObjMap,if
UnregisterPyObjectdoesn't call the unregister hook because itquits early. This can happen if there is not C++ object corresponding to
the regulated python object, causing an infinite loop because
fObjectMapnever gets empty.Although this seems like an obvious logic error, this was only noticed
after the cppyy upgrade. Probably the code path was not hit before.
Addresses
#15085 (comment).
This PR also includes a second refactoring commit to reduce the amount of C++ code in PyROOT.
This could maybe also fix the problem with the distributed RDataFrame test timeouts!