gh-103092: Port some `_ctypes` data types to heap types by neonene · Pull Request #113630 · python/cpython

Currently, PyType_From* functions refuse or ignore creating classes whose metaclass overrides tp_new (#103968):

>>> import ctypes
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    import ctypes
  File "C:\cp\Lib\ctypes\__init__.py", line 8, in <module>
    from _ctypes import Union, Structure, Array
TypeError: Metaclasses with custom tp_new are not supported.

The ctypes extension needs some workaround for that.

UPDATE: Added TypeError message above.

• Issue: Isolate Stdlib Extension Modules #103092

@vstinner Could you review this behavior change?

This is an attempt without hacking type slots.

Problem: It is possible that accessing metaclasses cause a crash with bad arguments (e.g. type(c_int).__init__(...)), which this PR does not tackle.

cc PEP 687 experts: @erlend-aasland @kumaraditya303

Leaving an alternative PR (#113774) which does not have the problem above. However, tp_new slot is hacked. Is there any better way to isolate _ctypes?

cc @encukou

To make this easier to review -- and to make the issue easier to see for as many reviewers as possible, would you mind sending a PR that adds the *_Type pointers to ctypes_state, but initializes them pointers to the existing static types for now? Also change all the *_Check calls -- those should look the same regardless of how the metaclass issue is solved. The next PR can then focus only on the problematic case.

IMO, the correct way with the current heap type creation API is to move the initialization to tp_init as you do, but ensure that tp_init was called exactly once: that is, all operations should fail cleanly if it was not run on a given type, and it should fail (or do nothing) if called a second time.

Another option is to improve the type creation API -- e.g. design a slot that works like __subclass_init__ but CPython guarantees to call it exactly once.

@vstinner Could you review this behavior change?

I would prefer a smaller PR. You moved 7 types. Can you start with a PR which change less types and less files at once?

@encukou Is it worth opening a new issue for that? Maybe a custom meta tp_init can be checked without switching to a heap type?

Oh, I think it's worth writing a PEP for that :)
If you want to help there, a good first step would be a proof-of-concept PR (without an issue).

Sorry, I'm not qualified, as I'm volunteering anonymously.
A fix for issues around #113055 without porting _ctypes is also fine with me.

Oh, I think it's worth writing a PEP for that :)

Why do you think that a PEP is needed for this specific change? Many static types were converted to heap types in the stdlib without a specific PEP.

Oh, I think it's worth writing a PEP for that :)

Why do you think that a PEP is needed for this specific change? Many static types were converted to heap types in the stdlib without a specific PEP.

I think @encukou meant a PEP for this proposed idea in #113630 (comment) :)

Another option is to improve the type creation API -- e.g. design a slot that works like subclass_init but CPython guarantees to call it exactly once.

How are ctypes types than other stdlib extensions types? Is there a metaclass? Why do they need special code for __init__()? I don't understand and the PR doesn't say much about it.

How are ctypes types than other stdlib extensions types? Is there a metaclass? Why do they need special code for __init__()? I don't understand and the PR doesn't say much about it.

_ctypes is not straight-forward to adapt, contrary to pretty much the rest of the extension modules in the stdlib. There is a lot of metaclass hacks.

Ah, PyCStructType_Type is special: it's a "meta type/class" says a comment. If it's special, I suggest to create a dedicated issue to collect more details about this specific type and how it's used. Currently, the PR is linked to a generic "Isolate Stdlib Extension Modules" issue.

pycstruct_type_slots defines the Py_tp_new slot as PyCStructType_new(). This function calls StructUnionType_new() which does the black magic:

/*
  PyCStructType_Type - a meta type/class.  Creating a new class using this one as
  __metaclass__ will call the constructor StructUnionType_new/init.  It replaces
  the tp_dict member with a new instance of StgDict, and initializes the C
  accessible fields somehow.
*/

Extracts:

    /* create the new instance (which is a class,
       since we are a metatype!) */

and:

    /* replace the class dict by our updated stgdict, which holds info
       about storage requirements of the instances */

+1 for a dedicated issue; there is a lot of issues connected to the isolation of _ctypes.

I created the issue gh-114314 "Convert _ctypes PyCStgDict meta static type to a meta heap type".

To make this easier to review -- and to make the issue easier to see for as many reviewers as possible, would you mind sending a PR that adds the *_Type pointers to ctypes_state, but initializes them pointers to the existing static types for now? Also change all the *_Check calls

I wrote PR #114316 for that. It's partially based on this PR.

Unlike this PR, the issue #114314 seems to consider how to refactor the hacks around PyCStgDict, without customizing tp_new/init. If so, that also sounds reasonable to me.

Any way is fine with me to avoid the error related to #103968:

>>> import ctypes
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    import ctypes
  File "C:\cp\Lib\ctypes\__init__.py", line 8, in <module>
    from _ctypes import Union, Structure, Array
TypeError: Metaclasses with custom tp_new are not supported.

Closing the draft in favor of #116458.