bpo-25532: Protect against infinite loops in inspect.unwrap() #1717
Conversation
|
@takluyver, thanks for your PR! By analyzing the history of the files in this pull request, we identified @akuchling, @1st1 and @zestyping to be potential reviewers. |
|
+1 from me, so this just needs a Misc/NEWS entry noting the fix. |
|
Added a NEWS entry :-) |
|
Hah, that'll teach me to review patches late at night - I missed a more important omission, which is to add a test case that triggers the new check :) |
|
I take it you don't want the test to use |
|
How does that test look? |
Lib/inspect.py
Outdated
| def _is_wrapper(f): | ||
| return hasattr(f, '__wrapped__') and not stop(f) | ||
| f = func # remember the original func for error reporting | ||
| memo = {id(f)} # Memoise by id to tolerate non-hashable objects |
There was a problem hiding this comment.
While we are here: we should switch to use a dictionary {id(o): o}. The same approach that deepcopy uses to ensure that objects stay alive long enough.
|
Why can't you use |
|
+1 to the |
|
Thanks for the patch @takluyver, and thanks for the suggested improvements @1st1 and @pppery :) |
|
🐍🍒⛏🤖 Thanks @takluyver for the PR, and @ncoghlan for merging it 🌮🎉.I'm working now to backport this PR to: 3.6. |
|
Sorry, @takluyver and @ncoghlan, I could not cleanly backport this to |
…ythonGH-1717) Some objects (like test mocks) auto-generate new objects on attribute access, which can lead to an infinite loop in inspect.unwrap(). Ensuring references are retained to otherwise temporary objects and capping the size of the memo dict turns this case into a conventional exception instead.. (cherry picked from commit f9169ce)
|
GH-3778 is a backport of this pull request to the 3.6 branch. |
…H-1717) (#3778) Some objects (like test mocks) auto-generate new objects on attribute access, which can lead to an infinite loop in inspect.unwrap(). Ensuring references are retained to otherwise temporary objects and capping the size of the memo dict turns this case into a conventional exception instead.. (cherry picked from commit f9169ce)
9 participants
Some objects, such as
unittest.mock.call, generate a new similar object every time you accessobj.__wrapped__. This isn't caught by the memoise loop protection, because each object has a different ID. So a call likeinspect.getsourcelines(unittest.mock.call)goes into an infinite loop, using ever more memory as it creates new objects.This implements @ncoghlan's suggestion in issue 22532: if we've unwrapped something
sys.getrecursionlimit()times, assume it's an infinite chain and give up. Things likegetsourcelines()will fail, but an exception is better than using up all the memory.IPython's inspection machinery already has a similar check, with an arbitrary limit of 100 unwraps. That returns the original object if the limit is exceeded, rather than throwing an error.
https://github.com/ipython/ipython/blob/6.0.0/IPython/core/oinspect.py#L279