mock.seal has infinite recursion with mutually recursive class references

[dseomn]

Bug report

This code seems to cause infinite recursion in unittest.mock.seal:

from unittest import mock
class Foo:
  pass
class Bar:
  foo = Foo
Foo.bar = Bar
foo = mock.create_autospec(Foo)
mock.seal(foo)

$ python3 foo.py 
Traceback (most recent call last):
  File "/tmp/tmp.yKOznvlUIG/foo.py", line 8, in <module>
    mock.seal(foo)
  File "/usr/lib/python3.9/unittest/mock.py", line 2874, in seal
    seal(m)
  File "/usr/lib/python3.9/unittest/mock.py", line 2874, in seal
    seal(m)
  File "/usr/lib/python3.9/unittest/mock.py", line 2874, in seal
    seal(m)
  [Previous line repeated 973 more times]
  File "/usr/lib/python3.9/unittest/mock.py", line 2866, in seal
    m = getattr(mock, attr)
  File "/usr/lib/python3.9/unittest/mock.py", line 655, in __getattr__
    result = create_autospec(
  File "/usr/lib/python3.9/unittest/mock.py", line 2627, in create_autospec
    mock = Klass(parent=_parent, _new_parent=_parent, _new_name=_new_name,
  File "/usr/lib/python3.9/unittest/mock.py", line 2034, in __init__
    _safe_super(MagicMixin, self).__init__(*args, **kw)
  File "/usr/lib/python3.9/unittest/mock.py", line 1074, in __init__
    _safe_super(CallableMixin, self).__init__(
  File "/usr/lib/python3.9/unittest/mock.py", line 437, in __init__
    self._mock_add_spec(spec, spec_set, _spec_as_instance, _eat_self)
  File "/usr/lib/python3.9/unittest/mock.py", line 500, in _mock_add_spec
    res = _get_signature_object(spec,
  File "/usr/lib/python3.9/unittest/mock.py", line 107, in _get_signature_object
    return func, inspect.signature(sig_func)
  File "/usr/lib/python3.9/inspect.py", line 3113, in signature
    return Signature.from_callable(obj, follow_wrapped=follow_wrapped)
  File "/usr/lib/python3.9/inspect.py", line 2862, in from_callable
    return _signature_from_callable(obj, sigcls=cls,
  File "/usr/lib/python3.9/inspect.py", line 2333, in _signature_from_callable
    wrapped_sig = _get_signature_of(obj.func)
  File "/usr/lib/python3.9/inspect.py", line 2329, in _signature_from_callable
    return _signature_from_builtin(sigcls, obj,
  File "/usr/lib/python3.9/inspect.py", line 2149, in _signature_from_builtin
    return _signature_fromstr(cls, func, s, skip_bound_arg)
  File "/usr/lib/python3.9/inspect.py", line 2009, in _signature_fromstr
    _signature_strip_non_python_syntax(s)
  File "/usr/lib/python3.9/inspect.py", line 1962, in _signature_strip_non_python_syntax
    for t in token_stream:
  File "/usr/lib/python3.9/tokenize.py", line 525, in _tokenize
    pseudomatch = _compile(PseudoToken).match(line, pos)
  File "/usr/lib/python3.9/tokenize.py", line 99, in _compile
    return re.compile(expr, re.UNICODE)
  File "/usr/lib/python3.9/re.py", line 252, in compile
    return _compile(pattern, flags)
  File "/usr/lib/python3.9/re.py", line 292, in _compile
    flags = flags.value
  File "/usr/lib/python3.9/types.py", line 178, in __get__
    return self.fget(instance)
RecursionError: maximum recursion depth exceeded

Your environment

• CPython versions tested on: 3.9.9
• Operating system and architecture: Debian testing, amd64

[AbhigyanBose]

There seems to be two places where we get infinite recursion.

1. https://github.com/python/cpython/blob/00f22e8cc234aa52ec1f28094a170d7b87d0d08f/Lib/unittest/mock.py#L2716-L2718

The instance wasn't being passed correctly, it is easy to fix by specifying the parameter name like instance=instance.

2. https://github.com/python/cpython/blob/00f22e8cc234aa52ec1f28094a170d7b87d0d08f/Lib/unittest/mock.py#L2925-L2926

The recursion here tries to mock all attributes recursively(if there's a class or object). So in this chain of calls if there's a cycle we end up caught in an infinite recursion.

Possible solutions:

1. We can't look up the list of superclasses directly due to all superclasses being mocks. So to preserve this recursive functionality we would have to keep a list/set of all superclasses encountered and check each new object/class that is sealed to ensure we haven't already come across that type.
2. We set some sort of a limit on the recursion.

Let me know if anyone has a better solution in mind, I'd be happy to submit a PR implementing the solution that's chosen.

[tirkarthi]

cc: @mariocj89

[mariocj89]

Right, because "foo.bar.foo.bar.foo" is not the same as "foo.bar.foo" Which I honestly find surprising. Is that something we should fix? Otherwise, this is really infinite and there is no way for seal to handle it, as "foo.bar.foo" is genuinely a different mock from "foo.bar.foo.bar.foo".

If we could fix that we could then have seal check if a mock has been already marked with seal and if so skip it.

Otherwise, I think the only alternative is indeed to just set a limit (at which point we might just go with the current limit by the interpreter). We could capture RecursionError and provide a nicer message.

[dseomn]

Otherwise, this is really infinite and there is no way for seal to handle it, as "foo.bar.foo" is genuinely a different mock from "foo.bar.foo.bar.foo".

Is there a technical reason seal couldn't handle it if it really is infinite? Assuming there's some lazy evaluation somewhere, could seal set an attribute on a parent somewhere that's read during lazy evaluation to call seal on a newly evaluated child at that point, instead of seal trying to evaluate all the children right away?

[mariocj89]

Assuming there's some lazy evaluation somewhere, could seal set an attribute on a parent somewhere that's read during lazy evaluation to call seal on a newly evaluated child at that point

That's indeed a great idea. To check if a mock is sealed, we check itself and its parents. When we set something as sealed we just need to do it in one single place.