NotificationsYou must be signed in to change notification settings
Fork33.4k
Star69.8k

[2.7] bpo-18533: Avoid RuntimeError from repr() of recursive dictview [GH-4823]#5357

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

serhiy-storchaka merged 1 commit intopython:2.7frombennorth:backport-d7773d9-2.7

Feb 26, 2018

Merged

[2.7] bpo-18533: Avoid RuntimeError from repr() of recursive dictview [GH-4823]#5357

serhiy-storchaka merged 1 commit intopython:2.7frombennorth:backport-d7773d9-2.7

Feb 26, 2018

Conversation

Copy link

Contributor

bennorth commentedJan 27, 2018•
edited by bedevere-bot
Loading

As suggested ina message on BPO, this is a backport of#4823 to the 2.7 branch. Compared to the changes introduced in#4823, this PR:

Works with the existing explicitseq_str indictview_repr().
UsesPyString... (instead ofPyUnicode...).
Sets up the test-case dictionaries to containviewvalues() andviewitems() (instead ofvalues() anditems()).
Checks forRuntimeError (instead ofRecursionError).
Updates the news entry accordingly.

I kept the test onOrderedDict even though 2.7 behaves correctly already; please let me know if it would be cleaner to omit this part.

https://bugs.python.org/issue18533

the-knights-who-say-ni added the CLA signed label

Jan 27, 2018

bedevere-bot added the awaiting review label

Jan 27, 2018

bennorth mentioned this pull request

Jan 27, 2018

bpo-18533: Avoid RuntimeError from repr() of recursive dictview#4823

Merged

Copy link

Member

matrixise commentedJan 30, 2018

I have executed the tests with

./python -m test -R 3:3

and there is no reference leaks.

serhiy-storchaka reviewed

Feb 26, 2018

View reviewed changes

Objects/dictobject.c Outdated

Copy link

Member

serhiy-storchakaFeb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Py_DECREF(seq) could be called just afterPyObject_Repr(). This will simplify the code.

Copy link

ContributorAuthor

bennorthFeb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for suggestion; that is indeed cleaner. I've just force-pushed a fresh commit including that change.

bpo-18533: Avoid RuntimeError from repr() of recursive dictview (pyth…

a27a0d4

…on#4823)dictview_repr(): Use a Py_ReprEnter() / Py_ReprLeave() pair to checkfor recursion, and produce "..." if so.test_recursive_repr(): Check for a string rather than aRuntimeError.  (Test cannot be any tighter as contents areimplementation-dependent.)test_deeply_nested_repr(): Add new test, replacing the originaltest_recursive_repr().  It checks that a RuntimeError is raised inthe case of a non-recursive but deeply nested structure.OrderedDictTests: Add new test case, to test behavior on OrderedDictinstances containing their own viewvalues() or viewitems().  This testpasses without the patch to dictview_repr(), but it failed in (atleast some versions of) Python 3, so including it here forcompleteness.(cherry picked from commitd7773d9)