Object finalization isn't handled correctly #1127
Description
I noticed this while debugging the apparent failure reported at python/cpython#134170 (comment). This seems like a bug in mypyc, not CPython.
Here's a small repro:
import gc
class Evil:
def __del__(self):
gc.collect()
Evil()This results in a crash on my end when running off mypyc's master branch. Looking at the native code, I see this:
static void
Evil_dealloc(a___EvilObject *self)
{
if (!PyObject_GC_IsFinalized((PyObject *)self)) {
Py_TYPE(self)->tp_finalize((PyObject *)self);
}
PyObject_GC_UnTrack(self);
CPy_TRASHCAN_BEGIN(self, Evil_dealloc)
Evil_clear(self);
Py_TYPE(self)->tp_free((PyObject *)self);
CPy_TRASHCAN_END(self)
}
static void
Evil_finalize(PyObject *self)
{
PyObject *type, *value, *traceback;
PyErr_Fetch(&type, &value, &traceback);
CPyDef_Evil_____del__(self);
if (PyErr_Occurred() != NULL) {
PyObject *del_str = PyUnicode_FromString("__del__");
PyObject *del_method = (del_str == NULL) ? NULL : _PyType_Lookup(Py_TYPE(self), del_str);
PyErr_WriteUnraisable(del_method);
Py_XDECREF(del_method);
Py_XDECREF(del_str);
}
PyErr_Restore(type, value, traceback);
}There are a few problems here. Firstly, when an exception is raised in the finalizer on a debug build of CPython, there's an assertion failure at the _PyType_Lookup call:
python: Objects/typeobject.c:6093: _PyType_LookupStackRefAndVersion: Assertion `!PyErr_Occurred()' failed.
This is because it's not always safe to call _PyType_Lookup with an exception set:
Regarding the segfault, I think that's caused by two other issues with the destructor:
- The object is seemingly never untracked from the garbage collector. So, any Python code executed inside the finalizer or destructor can possibly execute
gc.collect()or trigger a garbage collection implicitly, which will cause the GC to believe that the object needs to be destroyed again. This isn't a problem aftertp_free(or really,PyObject_GC_Del) because that will untrack it for you. There should be aPyObject_GC_UnTrackcall somewhere in here to sidestep this problem. - Resurrection isn't handled correctly. Instead of manually calling the
tp_finalizeslot, usePyObject_CallFinalizerFromDeallocfor executing it. This will manage resurrection for you. If something else resurrects it (such as during a garbage collection inside Python calls), the one who resurrected needs to take responsibility for freeing the object, and the "current call" should no-op and leave the object intact.
With all that in mind, I think Evil_dealloc needs to look something like this:
static void
Evil_dealloc(a___EvilObject *self)
{
PyObject *exc = PyErr_GetRaisedException();
if (PyObject_CallFinalizerFromDealloc(self) < 0) {
goto done;
}
PyObject_GC_UnTrack(self);
CPy_TRASHCAN_BEGIN(self, Evil_dealloc)
Evil_clear(self);
Py_TYPE(self)->tp_free((PyObject *)self);
CPy_TRASHCAN_END(self)
done:
if (PyErr_Occurred()) {
PyErr_WriteUnraisable(self);
}
PyErr_SetRaisedException();
}The unraisable handling should also be removed from tp_finalize; the destructor should be in charge of handling exceptions, not the finalizer.