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
A tp_dealloc of a subclassed class cannot resurrect an object #52459
Comments
The tp_dealloc of a type can chose to resurrect an object. the subtype_dealloc() in typeobject.c does this when it calls the tp_del() member and it has increased the refcount. The problem is, that if you subclass a custom C object, and that C object's tp_dealloc() chooses to resurrect a dying object, that doesn't go well with subtype_dealloc(). After calling basedealloc() (line 1002 in typeobject.c), the object's type is decrefed. But if the object was resurrected by basedealloc() this shouldn't have been done. The object will be alive, but the type will be missing a reference. This will cause a crash later. This could be fixable if we knew somehow after calling basedealloc() if the object were still alive. But we cannot check "self" because it may have died. The only way out of this conundrum that I can see is to change the signature of tp_dealloc() to return a flag, whether it did actually delete the object or not. Of course, I see no easy way around not clearing the slots, but the clearing of the dict could be postponed until after the call to basedealloc(). Since tp_dealloc _can_ resurrect objects (subtype_dealloc does it), subclassing such objects should work, and not crash the interpreter if the base class' dp_dealloc() decides to do so. |
Indeed. The io module has had to circumvent this and uses the following snippet when resurrecting an instance of a subclass of one of its types (see iobase_dealloc() in Modules/_io/iobase.c): /* When called from a heap type's dealloc, the type will be
decref'ed on return (see e.g. subtype_dealloc in typeobject.c). */
if (PyType_HasFeature(Py_TYPE(self), Py_TPFLAGS_HEAPTYPE))
Py_INCREF(Py_TYPE(self));
return; I agree it would be nice to have an automatic way of handling this. I don't see an obvious solution, though. |
Here is a patch that rectifies this situation, albeit in a somewhat 'hacky' manner. |
Another, less hacky but more intrusive, way would be to change the signature of tp_dealloc in a backwards compatible way:
typedef void (*destructor)(PyObject *, int *destroyed); The destructor can then set the flag pointed to by 'destroyed' to 1 or 0, depending on whether actual destruction took place. The caller will set the flag to '1' by default. We could then change all internal destructors to conform, and know that external destructors will continue to work in the old way. |
Updated the patch with better documentation, and recursion safety. |
Urg, that's a horrible hack. That way the iobase_dealloc code would be written: if (_PyIOBase_finalize((PyObject *) self) < 0) {
_PyObject_ResurrectFromDealloc(self);
return;
} That API function could also perhaps take care of the _Py_NewReference stuff (see the end of _PyIOBase_finalize). |
Ok, that sounds reasonable, particularly in light of that _NewReference stuff. I'll work out a different patch then. But I think the API must be public, since it would need to work from extension modules. So: if a c type decides that it wants to live after a tp_dealloc(), it must call this api ... |
Needing to work from (stdlib) extension modules and being public are too |
Incidentally, Looking at this, I noticed code such as: (textio.c) static int
_textiowrapper_clear(textio *self)
{
if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0)
return -1;
This shows something scary: During a GC run, it is possible to invoke the "close()" method on a textio object. This is dangerous, and probably not allowed at all.
Returning -1 from a tp_clear() slot has no particular meaning, and the return value appears to be ignored by gcmodule.c (also, it won't ever ber resurrected through that pathway, since it is incref'd first:
Py_INCREF(op);
clear(op);
Py_DECREF(op);
) See bpo-9141 for more info. This probably requires a defect of its own. |
How is it scary? This is not worse than invoking a __del__ method.
I don't think this is a problem. |
__del__ methods are never invoked from GC. All objects that have finalizers, and all objects reachable from them _must_ be put in gc.garbage. |
Here is a patch with the suggested change. |
I think PEP-442 makes this request obsolete: you can simply implement tp_finalize() and incref the object naturally from there. Kristjan, what do you think? |
Right. I close the issue. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: