CPython 3.8.4 regression on __setattr__ in multiinheritance with metaclasses

[057fe7f5-76fb-4880-b15e-ffcb7aabd237]

BPO	41295
Nosy	@gvanrossum, @doko42, @scoder, @tiran, @ambv, @davidism, @miss-islington, @horazont, @david-caro, @kam193
PRs	python/cpython#21473 python/cpython#21528 python/cpython#21540 python/cpython#21541
Files	bpo41295.py

^{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 = created_at = labels = ['type-bug', '3.8', '3.9', '3.10'] title = 'CPython 3.8.4 regression on __setattr__ in multiinheritance with metaclasses' updated_at = user = 'https://github.com/kam193' ``` bugs.python.org fields: ```python activity = actor = 'scoder' assignee = 'none' closed = True closed_date = closer = 'scoder' components = [] creation = creator = 'kam193' dependencies = [] files = ['49316'] hgrepos = [] issue_num = 41295 keywords = ['patch', '3.8regression', '3.9regression'] message_count = 19.0 messages = ['373637', '373640', '373641', '373642', '373644', '373646', '373766', '373803', '373897', '373920', '373921', '373922', '373923', '373924', '373934', '373936', '373965', '373967', '374018'] nosy_count = 10.0 nosy_names = ['gvanrossum', 'doko', 'scoder', 'christian.heimes', 'lukasz.langa', 'davidism', 'miss-islington', 'jssfr', 'David Caro', 'kam193'] pr_nums = ['21473', '21528', '21540', '21541'] priority = 'critical' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue41295' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

[057fe7f5-76fb-4880-b15e-ffcb7aabd237]

CPython 3.8.4 broke previously correct custom __setattr__ implementation, when metaclass inheritances from multiple classes, including not-meta.

Following code:

class Meta(type):
    def __setattr__(cls, key, value):
        type.__setattr__(cls, key, value)

class OtherClass:
    pass

class DefaultMeta(OtherClass, Meta):
    pass

obj = DefaultMeta('A', (object,), {})
obj.test = True
print(obj.test)

Works in Python up to 3.8.3, but in 3.8.4 it raises:

Traceback (most recent call last):
  File "repr.py", line 13, in <module>
    obj.test = True
  File "repr.py", line 3, in __setattr__
    type.__setattr__(cls, key, value)
TypeError: can't apply this __setattr__ to DefaultMeta object

This change affects e.g. https://github.com/pallets/flask-sqlalchemy/issues/852

[tiran]

The error message is coming from hackcheck function in Objects/typeobject.c, https://github.com/python/cpython/blob/b4cd77de05e5bbaa6a4be90f710b787e0790c36f/Objects/typeobject.c#L5811-L5845 . The issue could be related to bpo-39960. Checking now...

[tiran]

The regression was introduced in commit 8912c182455de83e27d5c120639ec91b18247913

$ git checkout v3.8.4
$ ./configure -C
$ make
$ $ ./python bpo41295.py 
Traceback (most recent call last):
  File "bpo41295.py", line 14, in <module>
    obj.test = True
  File "bpo41295.py", line 3, in __setattr__
    type.__setattr__(cls, key, value)
TypeError: can't apply this __setattr__ to DefaultMeta object

$ git revert 8912c182455de83e27d5c120639ec91b18247913
[detached HEAD 8d3c7847a0] Revert "bpo-39960: Allow heap types in the "Carlo Verre" hack check that override "tp_setattro()" (GH-21092) (GH-21339)"
 3 files changed, 11 insertions(+), 117 deletions(-)
$ make
$ ./python bpo41295.py 
True

[tiran]

Stefan, please take a look.

[e98a6e43-25c8-4e1f-8eba-4b9486e14aef]

It appears to be solved in Flask-SQLAlchemy's development version already, as the mixins now inherit from type. We caught the issue when we started applying flake8 (possibly through flake8-bugbear).

[e508c9b1-f137-4a74-8873-03ddfcb92496]

Just sent a PR that fixes the issue (a first approach), let me know if it's addressing the issue correctly or if there's a better way.

Thanks!

[doko42]

David, which issue number is this?

[e508c9b1-f137-4a74-8873-03ddfcb92496]

David, which issue number is this?

It's bpo-41295, pr bpo-21473

[scoder]

PR 21528 works for all four test cases that we now have (1x test_capi.py, 3x test_descr.py).

Any comments? We need to merge a fix before Monday to meet the deadline of the planned hotfix release.

@kam193, could you please also test that change with your real code?

[miss-islington]

New changeset c53b310e5926266ce267c44a168165cacd786d6e by scoder in branch 'master': bpo-41295: Reimplement the Carlo Verre "hackcheck" (GH-21528) https://github.com/python/cpython/commit/c53b310e5926266ce267c44a168165cacd786d6e

[miss-islington]

New changeset 38d930f2ccbff6f93c4c54a7a6a1759266136504 by Miss Islington (bot) in branch '3.8': bpo-41295: Reimplement the Carlo Verre "hackcheck" (GH-21528) https://github.com/python/cpython/commit/38d930f2ccbff6f93c4c54a7a6a1759266136504

[miss-islington]

New changeset 01ab9634601fc1a4f9ac5d72ddc022239d2543fe by Miss Islington (bot) in branch '3.9': bpo-41295: Reimplement the Carlo Verre "hackcheck" (GH-21528) https://github.com/python/cpython/commit/01ab9634601fc1a4f9ac5d72ddc022239d2543fe

[gvanrossum]

We have a buildbot failure: test_asyncio altered the execution environment. What does that mean? Victor?

[057fe7f5-76fb-4880-b15e-ffcb7aabd237]

@Stefan: Today I run unit tests for Flask-SQLAlchemy (v2.4.3) as well as for aioxmpp (another library reported this problem https://github.com/horazont/aioxmpp/issues/342). In both cases patch is successful: tests fail on Python 3.8.4 but pass on patched Python.

As for me it looks done. Thanks for everyone involved!

[2b99b0db-b61c-4858-a34e-49165cbfc65c]

@kam193 Thanks for running the aioxmpp tests. I built the patched python yesterday, but I didn’t manage to get a virtualenv with it up&running.

Good to hear that the fix works as expected!

[scoder]

test_asyncio altered the execution environment

That happened several times before in previous builds. I think there's just a part of the asyncio tests that is unreliable. I left a comment in bpo-41273 since it might be related, the sporadic failures started appearing in the build following the merge.

[057fe7f5-76fb-4880-b15e-ffcb7aabd237]

@Jonas: I know, running those tests isn't so easy, I think a few libraries are incompatible with python 3.9/3.10 yet or I didn't find right prerequisites. But fortunately, everything is running on patched 3.8, so after back-porting I can run it

[gvanrossum]

@Łukasz: this should be good for the 3.8.5 release.

[ambv]

Released.