[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: Let's call it done with the current sys.getrecursionlimit() behaviour, and if that turns out to be problematic in practice, someone will hopefully file a new issue about it. -- resolution: -> fixed stage: patch review -> resolved

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Serhiy Storchaka added the comment: Ah, sorry, I backported and merged the PR with the "needs backport to 3.6" label before seeing the discussion about the limit over here. -- ___ Python tracker

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Serhiy Storchaka added the comment: New changeset 02c3cdcef84edd8c71e9fe189873dea216acfc1d by Serhiy Storchaka in branch '3.6': [3.6] bpo-25532: Protect against infinite loops in inspect.unwrap() (GH-1717) (#3778)

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Change by Serhiy Storchaka : -- pull_requests: +3765 stage: backport needed -> patch review ___ Python tracker ___

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Patrick Grafe added the comment: Is this ready to get backported to Python 3.5 and 3.6? I see the tags on the issue, but I'm not clear on the process for actually backporting patches. -- nosy: +Patrick Grafe ___ Python tracker

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Yury Selivanov added the comment: > I'd be fine with replacing the sys.getrecursionlimit() call with a module > level constant like "_UNWRAP_LIMIT = 500". TBH I'm OK either way. `sys.getrecursionlimit()` is 1000 (at least on my machine), which might be a bit too much for this specific

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: Ah, sorry - I merged the PR before seeing the discussion about the limit over here. I'd be fine with replacing the sys.getrecursionlimit() call with a module level constant like "_UNWRAP_LIMIT = 500". If someone desperately needed a higher limit for some

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Changes by Nick Coghlan : -- stage: needs patch -> backport needed versions: +Python 3.7 ___ Python tracker ___

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: New changeset f9169ce6b48c7cc7cc62d9eb5e4ee1ac7066d14b by Nick Coghlan (Thomas Kluyver) in branch 'master': bpo-25532: Protect against infinite loops in inspect.unwrap() (#1717) https://github.com/python/cpython/commit/f9169ce6b48c7cc7cc62d9eb5e4ee1ac7066d14b

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Thomas Kluyver added the comment: I could go either way on that. It's not hard to imagine it as a recursive algorithm, and using the recursion limit provides a simple configuration escape hatch if someone has a desperate need for something wrapped 3000 times for some strange reason. But it

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Yury Selivanov added the comment: > Since the limit is arbitrary anyway, sys.getrecursionlimit() seems like a > reasonable choice. The recursion limit can be changed for whatever purposes and I'm not sure that it's a good idea that it will affect unwrap behaviour in such cases. I'd suggest

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Thomas Kluyver added the comment: This was just reported in IPython as well (https://github.com/ipython/ipython/issues/10578 ). I've prepared a pull request based on Nick's proposal: https://github.com/python/cpython/pull/1717 -- nosy: +takluyver

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Changes by Thomas Kluyver : -- pull_requests: +1806 ___ Python tracker ___ ___

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Hugo Geoffroy added the comment: Another argument for having the fix in `unwrap` rather than `signature` is that this bug does not actually seem to be called by `signature`, as the doctest module calls `unwrap` for "inspect.isroutine(inspect.unwrap(val))". Also, this call does not even check

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: My current inclination is that the right fix here is to have a hard limit in inspect.unwrap() itself, on the basis of "we won't eat all the memory on your machine, even when other code produces an infinite chain of distinct __wrapped__ attributes" is a

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: The infinite loop reported here is an inspect.signature bug - it's supposed to prevent infinite loops, but we didn't account for cases where "obj.__wrapped__" implicitly generates a fresh object every time. unittest.mock.Mock is the only case of that in the

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Hugo Geoffroy added the comment: You are right, the fix would be better suited in `unwrap`. But, still, shouldn't any `__getattr__` implementation take care of not returning, for the `__wrapped__` attribute, a dynamic wrapper that provides the same attribute ? `__wrapped__` is commonly

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Nick Coghlan added the comment: An alternative approach would be to change inspect.unwrap() to use getattr_static() rather than the usual getattr(). The downside of that would be potentially breaking true object proxies, like the 3rd party wrapt module and weakref.proxy. However, what if

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Hugo Geoffroy added the comment: This patch blacklists `__wrapped__` (using the same form as the first comment, with a more explicit exception message) in `unittest.mock._Call.__getattr__`. I also documented the change and added a tests that checks `assertFalse(hasattr(call,

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Changes by Berker Peksag : -- stage: -> needs patch type: -> behavior ___ Python tracker ___

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Florian Bruhin added the comment: FWIW, this also affects pytest users: https://github.com/pytest-dev/pytest/issues/1217 -- nosy: +The Compiler ___ Python tracker

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Chris Withers added the comment: Cool, what needs to happen for __wrapped__ in to be blacklisted in call? Separately, inspect.unwrap probably needs to use something less fragile than a set of ids to track whether it's stuck in a loop. What is the actual usecase for __wrapped__ how deeply

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Michael Foord added the comment: I'm happy to blacklist __wrapped__ in call. I don't see how call can detect that kind of introspection in the general case though. Limiting call depth would be one option, but any limit is going to be arbitrary and I don't "like" that as a solution.

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Michael Foord added the comment: Have you read the docs for call? :-) call.foo("foo") generates a call object representing a method call to the method foo. So you can do. m = Mock() m.foo("foo") self.assertEqual(m.mock_calls, call.foo("foo")) (etc) See also the call.call_list

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Michael Foord added the comment: Preventing the use of "__" attributes would limit the usefulness of call with MagicMock and the use of magic methods. That might be an acceptable trade-off, but would break existing tests for people. (i.e. it's a backwards incompatible change.) --

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Chris Withers added the comment: Indeed, I guess Venusian will get confused, but not sure thats solvable as there will be obvious bugs indicating that call shouldn't be imported at module-level. This does feel like the problem is actually with inspect.unwrap: there's evidence of an attempt

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Michael Foord added the comment: For mock I think your proposed fix is fine. call is particularly magic as merely importing it into modules can cause introspection problems like this (venusian is a third party library that has a similar issue), so working around these problems as they arise

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Chris Withers added the comment: Ah yes, I can see how Venusian would get confused. How about making the check a more generic: if attr.startswith('__'): raise AttributeError(attr) ? That said, why does call have a __getattr__? Where is that intended to be used? --

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

New submission from Chris Withers: The following results in an infinite loop: >>> from inspect import unwrap >>> from unittest.mock import call >>> unwrap(call) This can occur when you use doctest.DocTestSuite to load doc tests from a module where unittest.mock.call has been imported.

[issue25532] infinite loop when running inspect.unwrap over unittest.mock.call

Chris Withers added the comment: A naive solution is to chat unittest.mock._Call's __getattr__ to be as follows: def __getattr__(self, attr): if attr == '__wrapped__': raise AttributeError('__wrapped__') if self.name is None: return _Call(name=attr, from_kall=False)