GeezerVison is showing github.com — Add tests to prevent regressions with the combination of `ctypes` and metaclasses. · Issue #125783 · python/cpython
Add tests to prevent regressions with the combination of ctypes and metaclasses. #125783DescriptionFeature or enhancement
Proposal:
There were breaking changes to ctypes in Python 3.13.
Projects like comtypes and pyglet, which implement functionality by combining ctypes and metaclasses, no longer work unless their codebases are updated.
We discussed what changes such projects need to make to their codebases in gh-124520.
I believe the easiest and most effective way to prevent regressions from happening again is to add tests on the cpython side.
I wrote a simple test like the one below.
# Lib/test/test_ctypes/test_c_simple_type_meta.py?
import unittest
import ctypes
from ctypes import POINTER, c_void_p
from ._support import PyCSimpleType, _CData, _SimpleCData
class PyCSimpleTypeAsMetaclassTest(unittest.TestCase):
def tearDown(self):
# to not leak references, we must clean _pointer_type_cache
ctypes._reset_cache()
def test_early_return_in_dunder_new_1(self):
# this implementation is used in `IUnknown` of comtypes.
class _ct_meta(type):
def __new__(cls, name, bases, namespace):
self = super().__new__(cls, name, bases, namespace)
if bases == (c_void_p,):
return self
if issubclass(self, _PtrBase):
return self
if bases == (object,):
_ptr_bases = (self, _PtrBase)
else:
_ptr_bases = (self, POINTER(bases[0]))
p = _p_meta(f"POINTER({self.__name__})", _ptr_bases, {})
ctypes._pointer_type_cache[self] = p
return self
class _p_meta(PyCSimpleType, _ct_meta):
pass
class _PtrBase(c_void_p, metaclass=_p_meta):
pass
class _CtBase(object, metaclass=_ct_meta):
pass
class _Sub(_CtBase):
pass
class _Sub2(_Sub):
pass
def test_early_return_in_dunder_new_2(self):
# this implementation is used in `CoClass` of comtypes.
class _ct_meta(type):
def __new__(cls, name, bases, namespace):
self = super().__new__(cls, name, bases, namespace)
if isinstance(self, _p_meta):
return self
p = _p_meta(
f"POINTER({self.__name__})", (self, c_void_p), {}
)
ctypes._pointer_type_cache[self] = p
return self
class _p_meta(PyCSimpleType, _ct_meta):
pass
class _Base(object):
pass
class _Sub(_Base, metaclass=_ct_meta):
pass
class _Sub2(_Sub):
passIf no errors occur when this test is executed, we can assume that no regressions have occurred with the combination of ctypes and metaclasses.
However, I feel that this may not be enough. What should we specify as the target for the assert?
Has this already been discussed elsewhere?
No response given
Links to previous discussion of this feature:
#124520
Linked PRs
Activityencukou commented on Oct 22, 2024 Before calling something a “breaking change”, please check that you're not using internal API. Things like this are fragile:
- using an underscore-prefixed name
- calling
__new__ but not the corresponding __init__
- instantiating an internal type (one with an underscore-prefixed name/module, even if it's also available as
type(something_public))
- Omitting
super().__new__ in an overridden __new__, and the same for __init__
I appreciate that Python's backwards compatibility policy is unclear on some of these techniques; should we clarify it?
I also appreciate that what you're doing isn't possible with public API. I will try to not break your use case (and time permitting, to add supported API). But let's be clear that your warranty is void, so to say. Expect that with some CPython version will break comtypes again, and we'll need to work together to fix things. Ideally, test with alpha & beta releases so we can react as soon as possible -- IMO, that is the most effective way to prevent regressions here :)
All that said, it would definitely be good to add tests like this, to make sure the internals don't change unexpectedly -- especially if people are relying on the internals.
Even better: we could expose some public API for adding things to _pointer_type_cache. Could we solve your use case with a public register_pointer_type function -- or even better, a hook like register_pointer_type_factory, which could create a pointer type on demand (including pointers to pointers, for any level of recursion)? junkmd commented on Oct 22, 2024 ContributorAuthor More actions As you pointed out, it is true that the implementation of comtypes is much more fragile than packages that rely on other standard Python libraries.
Thomas Heller, the originator of ctypes, was also the originator of comtypes, so I speculate that he used private ctypes APIs with a certain level of caution in the implementation of comtypes (although this is just speculation, as he has retired from CPython).
I don't think we can clearly define a backward compatibility policy for Python here.
I believe we focus on improving ctypes while ensuring that projects depending on it do not break critically.
I agree with transitioning from directly manipulating private elements to using the public API.
However, I would like to first backport this test to the bugfix status versions (3.12 and 3.13), ensuring that this implementation works compatibly with Python 3.14 and beyond.
For now, I think it’s reasonable to continue using _pointer_type_cache, and once the public API is implemented, we can refactor the test to use that instead. junkmd commented on Oct 22, 2024 ContributorAuthor More actions Would defining a class with a metaclass be sufficient for the test?
For example, would adding assertions for mro, isinstance, and issubclass increase robustness without hindering improvements to the production code? junkmd commented on Oct 23, 2024 ContributorAuthor More actions I came up with the idea of adding assertions to confirm superclass relationships to prevent projects like comtypes from breaking critically.
For example, ensuring that POINTER(IDispatch) is a subclass of POINTER(IUnknown) and POINTER(Scripting.Dictionary) is a subclass of CoClass.
Even if the registration method for pointer types changes in the future, as long as these assertions don't fail, those projects should continue to work. added a commit that references this issue on Oct 25, 2024gh-125783: Add tests to prevent regressions with the combination of `… junkmd commented on Oct 26, 2024 ContributorAuthor More actions Once the backport PRs (#125987 and #125988) are merged, I plan to add tests for cases where generating pointer types in __init__.
Since this approach cannot be used in 3.12, the backport would be limited to 3.13. encukou commented on Oct 29, 2024 Thank you! The backports are merged now. added a commit that references this issue on Nov 1, 2024gh-125783: Add more tests to prevent regressions with the combination… junkmd commented on Nov 6, 2024 ContributorAuthor More actions The PRs to add tests that creates and registers pointer types within the __new__ and __init__ methods of the metaclass has been merged.
I believe there is nothing more to do in this issue, so I’ll close it.
There may be some areas that haven’t been tested in terms of using COM with ctypes, and these will be addressed in separate issues, such as gh-126384.
I’m also interested in adding a public API like register_pointer_type, but I plan to first add tests for any untested parts of the public ctypes API currently used in comtypes before tackling that.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|