Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maximum recursion depth exceeded in __getattr__(). #103272

Closed
felixxm opened this issue Apr 5, 2023 · 9 comments
Closed

Maximum recursion depth exceeded in __getattr__(). #103272

felixxm opened this issue Apr 5, 2023 · 9 comments
Labels
3.12 bugs and security fixes release-blocker type-bug An unexpected behavior, bug, or error

Comments

@felixxm
Copy link
Contributor

felixxm commented Apr 5, 2023

Bug report

We're hitting RecursionError: maximum recursion depth exceeded in Django test suite with Python 3.12.0a7 when accessing an attribute with a custom __getattr__() method:

ERROR: test_access_warning (deprecation.test_storages.DefaultStorageDeprecationTests.test_access_warning)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/deprecation/test_storages.py", line 128, in test_access_warning
    settings.DEFAULT_FILE_STORAGE
  File "/django/django/conf/__init__.py", line 83, in __getattr__
    if (_wrapped := self._wrapped) is empty:
                    ^^^^^^^^^^^^^
  File "/django/django/conf/__init__.py", line 83, in __getattr__
    if (_wrapped := self._wrapped) is empty:
                    ^^^^^^^^^^^^^
  File "/django/django/conf/__init__.py", line 83, in __getattr__
    if (_wrapped := self._wrapped) is empty:
                    ^^^^^^^^^^^^^
  [Previous line repeated 790 more times]
RecursionError: maximum recursion depth exceeded

See affected test and LazySettings.__getattr__().

Bisected to the aa0a73d.

I'd try to prepare a small regression test.

Your environment

  • CPython versions tested on: Python 3.12.0a7
  • Operating system and architecture: x86_64 GNU/Linux

Linked PRs

@felixxm felixxm added the type-bug An unexpected behavior, bug, or error label Apr 5, 2023
@AlexWaygood
Copy link
Member

Bisected to the aa0a73d.

Cc. @wangxiang-hz and @Fidget-Spinner

@Fidget-Spinner
Copy link
Member

Thanks for finding this. I'll try to debug once there's a minimal reproducer. Sorry I'm a little swamped for the next 2 weeks.

@sunmy2019
Copy link
Member

NB: when running this test with debug build, it crashes.

Testing against Django installed in '/home/ubuntu/django-main/django' with up to 8 processes
Found 6 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
.python: Python/ceval.c:821: _PyEval_EvalFrameDefault: Assertion `!_PyErr_Occurred(tstate)' failed.
Fatal Python error: Aborted

Current thread 0x00007f2f2750e740 (most recent call first):
  File "/home/ubuntu/django-main/django/conf/__init__.py", line 81 in __getattr__
  File "/home/ubuntu/django-main/tests/deprecation/test_storages.py", line 128 in test_access_warning
  File "/home/ubuntu/cpython-main/Lib/unittest/case.py", line 589 in _callTestMethod
  File "/home/ubuntu/cpython-main/Lib/unittest/case.py", line 634 in run
  File "/home/ubuntu/cpython-main/Lib/unittest/case.py", line 690 in __call__
  File "/home/ubuntu/django-main/django/test/testcases.py", line 292 in _setup_and_call
  File "/home/ubuntu/django-main/django/test/testcases.py", line 257 in __call__
  File "/home/ubuntu/cpython-main/Lib/unittest/suite.py", line 122 in run
  File "/home/ubuntu/cpython-main/Lib/unittest/suite.py", line 84 in __call__
  File "/home/ubuntu/cpython-main/Lib/unittest/runner.py", line 240 in run
  File "/home/ubuntu/django-main/django/test/runner.py", line 972 in run_suite
  File "/home/ubuntu/django-main/django/test/runner.py", line 1044 in run_tests
  File "/home/ubuntu/django-main/tests/./runtests.py", line 429 in django_tests
  File "/home/ubuntu/django-main/tests/./runtests.py", line 770 in <module>

Extension modules: pywatchman.bser, markupsafe._speedups (total: 2)
Aborted (core dumped)

@sunmy2019
Copy link
Member

I got a minimal repo!

class A:
    def __init__(self) -> None:
        self.bar = 0

    def __getattribute__(self, name):
        return super().__getattribute__(name)

    def __getattr__(self, name):
        if self.bar == 0:
            raise ValueError

    @property
    def foo(self):
        return self.__getattr__("foo")


A().foo

Running in older Python version, we got

Traceback (most recent call last):
  File "/home/ubuntu/django-main/tests/../a.py", line 17, in <module>
    A().foo
  File "/home/ubuntu/django-main/tests/../a.py", line 6, in __getattribute__
    return super().__getattribute__(name)
  File "/home/ubuntu/django-main/tests/../a.py", line 14, in foo
    return self.__getattr__("foo")
  File "/home/ubuntu/django-main/tests/../a.py", line 10, in __getattr__
    raise ValueError
ValueError

Call flow:

__getattribute__ called with name='foo'
property `foo` called

__getattribute__ called with name='__getattr__'
__getattr__ called with name='foo'
__getattribute__ called with name='bar'

But with the new python in main branch, (with out pydebug)

Exception ignored in tp_clear of: <class 'type'>
Traceback (most recent call last):
  File "/home/ubuntu/django-main/tests/../a.py", line 6, in __getattribute__
    return super().__getattribute__(name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
SystemError: <method-wrapper '__getattribute__' of A object at 0x7fc8af29e090> returned a result with an exception set
Traceback (most recent call last):
  File "/home/ubuntu/django-main/tests/../a.py", line 17, in <module>
    A().foo
  File "/home/ubuntu/django-main/tests/../a.py", line 9, in __getattr__
    if self.bar == 0:
       ^^^^^^^^
  File "/home/ubuntu/django-main/tests/../a.py", line 9, in __getattr__
    if self.bar == 0:
       ^^^^^^^^
  File "/home/ubuntu/django-main/tests/../a.py", line 9, in __getattr__
    if self.bar == 0:
       ^^^^^^^^
  [Previous line repeated 71 more times]
  File "/home/ubuntu/django-main/tests/../a.py", line 10, in __getattr__
    raise ValueError
ValueError

More precuriously, when I try to add some debug lines like this:

class A:
    def __init__(self) -> None:
        self.bar = 0

    def __getattribute__(self, name):
        return super().__getattribute__(name)

    def __getattr__(self, name):
        print(name)   ######### <----------- Here
        if self.bar == 0:
            raise ValueError

    @property
    def foo(self):
        return self.__getattr__("foo")


A().foo

The new python outputs,

foo
Traceback (most recent call last):
  File "/home/ubuntu/django-main/tests/../a.py", line 18, in <module>
    A().foo
  File "/home/ubuntu/django-main/tests/../a.py", line 9, in __getattr__
    print(name)
  File "/home/ubuntu/django-main/tests/../a.py", line 6, in __getattribute__
    return super().__getattribute__(name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/django-main/tests/../a.py", line 15, in foo
    return self.__getattr__("foo")
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/django-main/tests/../a.py", line 11, in __getattr__
    raise ValueError
ValueError

Everything just works fine? Looks like something in C is corrupting ❓ ❓ ❓

@sunmy2019
Copy link
Member

sunmy2019 commented Apr 6, 2023

Now with pydebug on, I got the output from new python: (crashes with/without debug line)

foo
python: Python/ceval.c:821: _PyEval_EvalFrameDefault: Assertion `!_PyErr_Occurred(tstate)' failed.
Aborted (core dumped)

I can confirm, for this minimum repo, crash starts at aa0a73d

@hauntsaninja
Copy link
Contributor

hauntsaninja commented Apr 7, 2023

Thanks for the report and for narrowing this down!

The change that regressed this has been reverted and we've added a regression test, so I think this can be closed

@zzzeek
Copy link

zzzeek commented May 24, 2023

WHEW, hi all, been bisecting all day (including getting better at bisecting large C code bases) to get here.

SQLAlchemy has been having failures on this in 3.12.0a7 and seem to be good in 3.12.0b1, just wanted to make sure this was logged.

FTR, here's our test case:

import sys


class Thing:
    @property
    def comparator(self):
        raise TypeError("a type error")

    def __getattr__(self, name):
        return getattr(self.comparator, name)

t1 = Thing()

try:
    t1.comparator
except TypeError:
    print("got typeerror as expected")

with the bug it throws a RecursionError

@wangxiang-hz
Copy link
Contributor

3.12.0a7

I have confirmed that version 3.12.0a7 was released on April 4th and the bug does exist in this version. However, the bug has been fixed in version 3.12.0b1. Therefore, it is highly likely that your error is related to this issue.

@sunmy2019
Copy link
Member

your error is related to this issue

It is. Test coverage is added in #103272

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes release-blocker type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

7 participants