The PyErr_Fetch() documentation says "you own a reference to each object retrieved", which means we need to decrement the reference count before returning, as we have no interest in sharing ownership. We were doing this via the boost::python::handle<> objects in formatInternal(), but since d0731a3e9f0757760e618cb5a68d74e0651273b9 we haven't been calling formatInternal() for exceptions bound via IECorePython::ExceptionClass. That meant we were leaking such Python exceptions such that they would never be destroyed.
The solution, and the moral of the story, is to always hold PyObject * via a sensible RAII class like handle or object, and to always do that as early as possible.
You might be forgiven for thinking that leaking a little exception object isn't that big a deal. But Python exceptions have a __traceback__ attribute that contains the entire stack at the point the exception was raised, and that contains all the local variables from all of those stack frames. So the leak can actually include a completely arbitrary amount of stuff.
danieldresser-ie
commented
11 months ago
The
PyErr_Fetch()documentation says "you own a reference to each object retrieved", which means we need to decrement the reference count before returning, as we have no interest in sharing ownership. We were doing this via theboost::python::handle<>objects informatInternal(), but since d0731a3e9f0757760e618cb5a68d74e0651273b9 we haven't been callingformatInternal()for exceptions bound viaIECorePython::ExceptionClass. That meant we were leaking such Python exceptions such that they would never be destroyed.The solution, and the moral of the story, is to always hold
PyObject *via a sensible RAII class likehandleorobject, and to always do that as early as possible.You might be forgiven for thinking that leaking a little exception object isn't that big a deal. But Python exceptions have a
__traceback__attribute that contains the entire stack at the point the exception was raised, and that contains all the local variables from all of those stack frames. So the leak can actually include a completely arbitrary amount of stuff.