python / cpython

The Python programming language
https://www.python.org
Other
62.87k stars 30.12k forks source link

ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments #73456

Open e1d2052b-073a-490c-9940-80dad2000cd3 opened 7 years ago

e1d2052b-073a-490c-9940-80dad2000cd3 commented 7 years ago
BPO 29270
Nosy @ncoghlan, @vstinner, @serhiy-storchaka, @eryksun, @https://github.com/hakril, @waveform80
Files
  • issue_29270_01.patch
  • 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: ```python assignee = None closed_at = None created_at = labels = ['ctypes', 'type-bug', '3.9', '3.10', '3.11'] title = 'ctypes: fail to create a _ctypes._SimpleCData subclass using a closure like calling super() without arguments' updated_at = user = 'https://github.com/waveform80' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'none' closed = False closed_date = None closer = None components = ['ctypes'] creation = creator = 'Dave Jones' dependencies = [] files = ['46285'] hgrepos = [] issue_num = 29270 keywords = ['patch'] message_count = 14.0 messages = ['285444', '285450', '285454', '285469', '285473', '285474', '285475', '285480', '286287', '286310', '313985', '355368', '360252', '416219'] nosy_count = 7.0 nosy_names = ['ncoghlan', 'vstinner', 'serhiy.storchaka', 'eryksun', 'hakril', 'Dave Jones', 'Asdger Gdsfe'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue29270' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    Linked PRs

    e1d2052b-073a-490c-9940-80dad2000cd3 commented 7 years ago

    While investigating a bug report in one of my libraries (https://github.com/waveform80/picamera/issues/355) I've come across a behaviour that appears in Python 3.6 but not prior versions. Specifically, calling super() in a sub-class of a ctypes scalar type appears to fail at the class definition stage. A minimal test case is as follows:

        import ctypes as ct
    
        class SuperTest(ct.c_uint32):
            def __repr__(self):
                return super().__repr__()

    This works happily under python 3.2, 3.4, and 3.5 (that I've tested), and also under 2.7 (with the appropriate modification to super's arguments). However, under 3.6 it elicits the following exception:

        Traceback (most recent call last):
          File "py36_ctypes.py", line 3, in <module>
            class SuperTest(ct.c_uint32):
        TypeError: __class__ set to <class '__main__.SuperTest'> defining 'SuperTest' as <class '__main__.SuperTest'>

    Reading through the "What's New" list in 3.6, I thought this might be something to do with the PEP-487 implementation (given it modified class construction), but having read through the PEP and associated patches I'm not so sure as I can't see anything that affects the setting of the "__class__" attribute (but don't rule it out on that basis; I'm no expert!).

    I'll admit that sub-classing one of ctypes' scalar types is a little odd, but given this works in prior versions and there doesn't appear to be anything in the documentation banning the practice (that I've found?) this might constitute a bug?

    eryksun commented 7 years ago

    In 3.6, type_new in Objects/typeobject.c sets the classcell in the dict if it's a cell object. It happens that CreateSwappedType in Modules/_ctypes/_ctypes.c re-uses the dict to create the swapped type (e.g. big endian), which in turn updates the classcell. Thus in builtin_build_class in Python/bltinmodule.c, the check cell_cls != cls ends up being true, which leads to the observed TypeError. CreateSwappedType should be able to avoid this by either deleting "classcell" from the dict or creating a copy without it before passing it to type_new.

    eryksun commented 7 years ago

    Here's a patch that deletes __classcell__ from the dict before calling type_new.

    ncoghlan commented 7 years ago

    Eryk's diagnosis sounds right to me, and the suggested patch will get this back to working as well as it did in Python 3.5.

    However, it's worth noting that that state itself was already broken when it comes to zero-argument super() support on the type definition with reversed endianness:

    >>> import ctypes as ct
    >>> class SuperText(ct.c_uint32):
    ...     def __repr__(self):
    ...         return super().__repr__()
    ... 
    >>> SuperText.__ctype_le__(1)
    <SuperText object at 0x7fde526daea0>
    >>> SuperText.__ctype_be__(1)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 3, in __repr__
    TypeError: super(type, obj): obj must be an instance or subtype of type

    The apparently nonsensical error message comes from both the original type and the type with swapped endianness having the same representation:

    >>> SuperText.__ctype_le__
    <class '__main__.SuperText'>
    >>> SuperText.__ctype_be__
    <class '__main__.SuperText'>
    e1d2052b-073a-490c-9940-80dad2000cd3 commented 7 years ago

    I confess I'm going to have to read a bit more about Python internals before I can understand Eryk's analysis (this is my first encounter with "cell objects"), but many thanks for the rapid analysis and patch!

    I'm not too concerned about the state being broken with reversed endianness; I don't think that's going to affect any of my use-cases in the near future, but it's certainly useful to know in case it does come up.

    eryksun commented 7 years ago

    OK, this is completely broken and needs a more thoughtful solution than my simpleminded hack. Here's a practical example of the problem, tested in 3.5.2:

        class MyInt(ctypes.c_int):
            def __repr__(self):
                return super().__repr__()
    
        class Struct(ctypes.BigEndianStructure):
            _fields_ = (('i', MyInt),)
        >>> s = Struct()
        >>> s.i
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File "<stdin>", line 3, in __repr__
        TypeError: super(type, obj): obj must be an instance or subtype of type
    ncoghlan commented 7 years ago

    Yeah, re-using Python-level method objects in different types is genuinely invalid when combined with __class or zero-argument super(), as there's no way to make the __class closure refer to two different classes at runtime - it will always refer back to the original defining class.

    And while ctypes could be updated to do the right thing for functions, classmethod, staticmethod, abstractmethod, property, etc, there's nothing systematic it can do that would work for arbitrary descriptors (any of which may have a zero-argument-super-using method definition hidden inside their internal state).

    eryksun commented 7 years ago

    Resolving this would be straightforward if we could use a subclass for the swapped type, but ctypes simple types behave differently for subclasses. A simple subclass doesn't automatically call the getfunc to get a converted value when returned as a function result, struct field, or array index.

    serhiy-storchaka commented 7 years ago

    See also similar issue with scrapy: https://github.com/scrapy-plugins/scrapy-djangoitem/issues/18.

    ncoghlan commented 7 years ago

    The scrapy case looks to just be the new metaclass constraint that's already covered in the "Porting to Python 3.6" guide: https://github.com/scrapy/scrapy/pull/2509/files

    The ctypes case is more complicated, as its actually *reusing* the same class namespace to define two different classes, which is fundamentally incompatible with the way zero-argument super works.

    061eb4cc-5744-43cc-9734-427da896e6ef commented 6 years ago

    Hey 3.6 is pretty old now so can we get this patch merged I'd really like this code to start working again, appreciate all your hard work!

    class Something(ctypes.c_ulong):
      def __repr__(self):
        return super(Something, self).value
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __class__ set to <class '__main__.Something'> defining 'Something' as <class '__main__.Something'>
    9cfe691a-913e-4643-94db-096fea7a67b0 commented 4 years ago

    Hello,

    I have a Python2 project that relies heavily on ctypes and I cannot migrate this project to Python3 due to this bug. (I target 3.6) I have some experience with CPython and submitting patchs and I would like to know what I can do to help moving this issue forward.

    Thanks !

    9cfe691a-913e-4643-94db-096fea7a67b0 commented 4 years ago

    Hello,

    As this issue may never be fixed for python3.6. I wanted to post a solution to bypass the bug. It may be useful for the next person stumbling on this as I have.

    The __class__ closure is only created if a function use the word super(). This closure allow to call super() without argument.

    By using another name than super() the closure is not created and your code can work. Only downside is that you need to call super in its explicit form super(Cls, self). But it is better that not working at all (and it is compatible python2).

    Here is a sample:

    super_bypass_issue29270 = super
    
    class Something(ctypes.c_ulong):
      def __repr__(self):
        return "BYPASS: " + super_bypass_issue29270(Something, self).__repr__()
    
    s = Something(42)
    print(s)

    BYPASS: \<Something object at 0x00000134C9BA6848>

    vstinner commented 2 years ago

    CreateSwappedType() is an helper function used by the _ctypes.PyCSimpleType type. Python script reproducing this ctypes bug: ---

    class PyCSimpleType(type):
        def __new__(cls, name, bases, dct):
            print(f"PyCSimpleType: create {name} class")
            cell = dct.get('__classcell__', None)
            # type.__new__() sets __classcell__
            x = super().__new__(cls, name, bases, dct)
            if cell is not None:
                print(f"PyCSimpleType: after first type.__new__() call: __classcell__={cell.cell_contents}")
    
            name2 = name + "_Swapped"
            dct2 = dict(dct, __qualname__=name2)
            # Calling type.__new__() again with the same cell object overrides
            # __classcell__
            x.__ctype_be__ = super().__new__(cls, name2, bases, dct2)
            if cell is not None:
                print(f"PyCSimpleType: after second type.__new__() call: __classcell__={cell.cell_contents}")
    
            return x
    
    class BaseItem:
        pass
    
    class Item(BaseItem, metaclass=PyCSimpleType):
        def get_class(self):
            # get '__class__' to create a closure
            return __class__
    # Alternative to create a closure:
    #def __repr__(self):
    #    return super().__repr__()

    ---

    Output: ---

    PyCSimpleType: create Item class
    PyCSimpleType: after first type.__new__() call: __classcell__=<class '__main__.Item'>
    PyCSimpleType: after second type.__new__() call: __classcell__=<class '__main__.Item_Swapped'>
    Traceback (most recent call last):
      File "meta.py", line 23, in <module>
        class Item(BaseItem, metaclass=PyCSimpleType):
    TypeError: __class__ set to <class '__main__.Item_Swapped'> defining 'Item' as <class '__main__.Item'>

    It's not a bug in Python types, but a bug specific to the _ctypes.PyCSimpleType type which prevents creating subclasses which use closures ("__class__", "super()", etc.).

    furkanonder commented 1 year ago

    PR is ready for review.