This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Rework C types initialization
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: corona10, vstinner
Priority: normal Keywords: patch

Created on 2021-04-07 21:48 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25263 merged vstinner, 2021-04-07 21:55
PR 25265 merged vstinner, 2021-04-07 22:28
PR 25266 merged vstinner, 2021-04-07 22:57
PR 25275 closed vstinner, 2021-04-08 08:13
PR 25325 merged vstinner, 2021-04-10 01:17
PR 25336 merged vstinner, 2021-04-10 22:09
PR 25373 merged vstinner, 2021-04-12 22:36
PR 25388 merged vstinner, 2021-04-13 12:54
PR 26879 merged vstinner, 2021-06-23 12:27
PR 26880 closed vstinner, 2021-06-23 12:33
PR 26881 closed vstinner, 2021-06-23 13:04
PR 26946 merged vstinner, 2021-06-29 01:07
Messages (14)
msg390484 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-07 21:48
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.
msg390490 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-07 22:12
New changeset a518099078c8ae51860009fb801db897af9eed62 by Victor Stinner in branch 'master':
bpo-43770: Sort types in _PyTypes_Init() (GH-25263)
https://github.com/python/cpython/commit/a518099078c8ae51860009fb801db897af9eed62
msg390493 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-07 22:48
New changeset df5dc1c7a536abc2b497cb9506f8a37949838309 by Victor Stinner in branch 'master':
bpo-43770: _PyTypes_Init() inits more static types (GH-25265)
https://github.com/python/cpython/commit/df5dc1c7a536abc2b497cb9506f8a37949838309
msg390506 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-08 07:58
New changeset b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985 by Victor Stinner in branch 'master':
bpo-43770: _PyTypes_Init() inits _PyAnextAwaitable_Type (GH-25266)
https://github.com/python/cpython/commit/b98eba5bc2ffbe7a0ed49d540ebc4f756ae61985
msg390510 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-08 08:29
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:

    test_buffer test_pickle test_picklebuffer test_pickletools
    test_structmembers

For example, Modules/_testbuffer.c does not initialize its ndarray type.

Example of crash:
-----------------
Objects/object.c:892: PyObject_GetAttr: Assertion "tp->tp_flags & (1UL << 12)" failed
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x7f4d89119a00
object refcount : 4
object type     : 0x8768c0
object type name: type
object repr     : <class 'ndarray'>

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007f4d96eba740 (most recent call first):
  File "/home/vstinner/python/master/Lib/test/test_picklebuffer.py", line 80 in test_ndarray_2d
  (...)

Extension modules: _testcapi, _testbuffer (total: 2)
-----------------

Another example of crash:
-----------------
Objects/object.c:892: PyObject_GetAttr: Assertion "tp->tp_flags & (1UL << 12)" failed
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x7f174ec9aa00
object refcount : 3
object type     : 0x8768c0
object type name: type
object repr     : <class 'ndarray'>

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007f175ca42740 (most recent call first):
  File "/home/vstinner/python/master/Lib/test/pickletester.py", line 304 in __reduce_ex__
  (...)

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);
}
msg390519 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-08 09:37
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)::

* BaseException.__dict__['__setattr__'] = <slot wrapper '__setattr__' of 'BaseException' objects>
* BaseException.__setattr__ = <slot wrapper '__setattr__' of 'BaseException' objects>

With the PR:

* no '__setattr___' in BaseException.__dict__
* BaseException.__setattr__ = <slot wrapper '__setattr__' of 'object' objects>

Because of that, doctest.DocTestFinder().find(builtins) returns less items, and so test_doctest fails.
msg390742 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-10 21:15
New changeset ecf14e6557c6e4f7af9c0d6460d31fe121c22553 by Victor Stinner in branch 'master':
bpo-43770: Refactor type_new() function (GH-25325)
https://github.com/python/cpython/commit/ecf14e6557c6e4f7af9c0d6460d31fe121c22553
msg390799 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-11 21:57
New changeset 53114ffef1d4facf9aa5545e711abbbda66f672a by Victor Stinner in branch 'master':
bpo-43770: Refactor PyType_Ready() function (GH-25336)
https://github.com/python/cpython/commit/53114ffef1d4facf9aa5545e711abbbda66f672a
msg390955 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 11:33
New changeset 65f058eb081c9e1fe44115d1ac7966067e3650c7 by Victor Stinner in branch 'master':
bpo-43770: Reorder type_ready() (GH-25373)
https://github.com/python/cpython/commit/65f058eb081c9e1fe44115d1ac7966067e3650c7
msg390962 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 13:25
New changeset a328d73843cfd42d2aee1434c78df1ef2845931a by Victor Stinner in branch 'master':
bpo-43770: Cleanup type_ready() (GH-25388)
https://github.com/python/cpython/commit/a328d73843cfd42d2aee1434c78df1ef2845931a
msg390964 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-13 13:34
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:

* SourceFileLoader.tp_base = FileLoader
* SourceFileLoader.tp_init = NULL
* SourceFileLoader.tp_dict has no "__init__" key

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__"]
msg396420 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-06-23 13:40
New changeset 2a396d65b8e63300ff05e217adacf0765c502ba3 by Victor Stinner in branch 'main':
bpo-43770:  Cleanup PyModuleDef_Init() (GH-26879)
https://github.com/python/cpython/commit/2a396d65b8e63300ff05e217adacf0765c502ba3
msg396807 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-07-01 01:12
New changeset dd3adc013b21ec1338bb5fea2e2c04a4fc650306 by Victor Stinner in branch 'main':
bpo-43770: Cleanup _PyObject_GetMethod() (GH-26946)
https://github.com/python/cpython/commit/dd3adc013b21ec1338bb5fea2e2c04a4fc650306
msg402383 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-21 22:08
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 ;-)
History
Date User Action Args
2022-04-11 14:59:44adminsetgithub: 87936
2021-09-21 22:08:50vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg402383

stage: patch review -> resolved
2021-07-01 01:12:06vstinnersetmessages: + msg396807
2021-06-30 13:28:09corona10setnosy: + corona10
2021-06-29 01:07:54vstinnersetpull_requests: + pull_request25513
2021-06-23 13:40:37vstinnersetmessages: + msg396420
2021-06-23 13:04:36vstinnersetpull_requests: + pull_request25457
2021-06-23 12:33:22vstinnersetpull_requests: + pull_request25456
2021-06-23 12:27:42vstinnersetpull_requests: + pull_request25455
2021-04-13 13:34:46vstinnersetmessages: + msg390964
2021-04-13 13:25:24vstinnersetmessages: + msg390962
2021-04-13 12:54:53vstinnersetpull_requests: + pull_request24121
2021-04-13 11:33:40vstinnersetmessages: + msg390955
2021-04-12 22:36:14vstinnersetpull_requests: + pull_request24106
2021-04-11 21:57:12vstinnersetmessages: + msg390799
2021-04-10 22:09:41vstinnersetpull_requests: + pull_request24070
2021-04-10 21:15:35vstinnersetmessages: + msg390742
2021-04-10 01:17:01vstinnersetpull_requests: + pull_request24060
2021-04-08 09:37:17vstinnersetmessages: + msg390519
2021-04-08 08:29:02vstinnersetmessages: + msg390510
2021-04-08 08:13:38vstinnersetpull_requests: + pull_request24012
2021-04-08 07:58:43vstinnersetmessages: + msg390506
2021-04-07 22:57:39vstinnersetpull_requests: + pull_request24002
2021-04-07 22:48:11vstinnersetmessages: + msg390493
2021-04-07 22:28:51vstinnersetpull_requests: + pull_request24001
2021-04-07 22:12:45vstinnersetmessages: + msg390490
2021-04-07 21:55:02vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request23999
2021-04-07 21:48:22vstinnercreate