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
Rework C types initialization #87936
Comments
Currently, PyType_Ready() is called late on many static types, which can lead to bugs. For example, PyObject_SetAttr() can fail if the type of the object is not ready yet. PyType_Read() is responsible to initialize tp_getattro and tp_setattro members of the PyTypeObject structure. We must explicitly initialize all static types before using them. Moreover, many static types initialize explicitly : tp_getattro = PyObject_GenericGetAttr and: tp_setattro = PyObject_GenericSetAttr whereas it's the default implementation. They can be omitted. I created this issue as a placeholder for multiple changes to modify how types implemented in C are initialized. |
I tried to put assertions in PyObject_GetAttr(), _PyObject_LookupAttr(), _PyObject_GetMethod() and PyObject_SetAttr() to ensure that the type is ready. It breaks many tests, examples:
For example, Modules/_testbuffer.c does not initialize its ndarray type. Example of crash: object address : 0x7f4d89119a00 Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed Current thread 0x00007f4d96eba740 (most recent call first): Extension modules: _testcapi, _testbuffer (total: 2) Another example of crash: object address : 0x7f174ec9aa00 Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed Current thread 0x00007f175ca42740 (most recent call first): Extension modules: _testcapi, _testbuffer, numpy.core._multiarray_umath, numpy.core._multiarray_tests, numpy.linalg.lapack_lite, numpy.linalg._umath_linalg, numpy.fft._pocketfft_internal, numpy.random._common, numpy.random.bit_generator, numpy.random._bounded_integers, numpy.random._mt19937, numpy.random.mtrand, numpy.random._philox, numpy.random._pcg64, numpy.random._sfc64, numpy.random._generator (total: 16) The C API tries to make the type ready automatically. For example, PyObject_Hash() calls PyType_Ready() if needed and then retry tp_hash: Py_hash_t
PyObject_Hash(PyObject *v)
{
PyTypeObject *tp = Py_TYPE(v);
if (tp->tp_hash != NULL)
return (*tp->tp_hash)(v);
/* To keep to the general practice that inheriting
* solely from object in C code should work without
* an explicit call to PyType_Ready, we implicitly call
* PyType_Ready here and then check the tp_hash slot again
*/
if (tp->tp_dict == NULL) {
if (PyType_Ready(tp) < 0)
return -1;
if (tp->tp_hash != NULL)
return (*tp->tp_hash)(v);
}
/* Otherwise, the object can't be hashed */
return PyObject_HashNotImplemented(v);
} |
PR 25275: this is a subtle difference if PyTypeObject.tp_setattro is set statically to PyObject_GenericSetAttr() or if it's inherited by PyType_Ready(). Reference (master)::
With the PR:
Because of that, doctest.DocTestFinder().find(builtins) returns less items, and so test_doctest fails. |
If I modify type_ready() to call type_ready_inherit() before type_ready_fill_dict(), some types create a between between their C slot (tp_init) and the Python API in tp_dict (tp_dict["__init__"]). Example with importlib: class FileLoader(...):
def __init__(...):
... => FileLoader.tp_init = slot_tp_init class SourceFileLoader(FileLoader):
... When PyType_Ready() is called on SourceFileLoader, we get:
When inherit_slots() is called, SourceFileLoader.tp_init is set to slot_tp_init(). When add_operators() is called, SourceFileLoader.tp_dict["__init__"] is set to PyDescr_NewWrapper(slot_tp_init). Problem: we a loop! tp_dict["__init__"] => slot_tp_init => tp_dict["__init__"] |
The code is way more complicated than what I expected. I hope that my work on this issue will help future developers who will try to understand the code. But I prefer to stop the refactoring at this point. I pushed enough changes ;-) |
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: