Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.2k
Description
I have spotted what I believe is a bug in the implementation ofdefaultdict's__repr__ method (code).
Long story short, the current implementation does not properly reset the recursion guard after delegating to the factory's__repr__ (when the factory's__repr__ calls back intodefaultdict's__repr__). As far as I can tell, this comes fromPy_ReprLeave not being called under the right conditions.
I've made this reproducer, I think it's as simple as it can get:
fromcollectionsimportdefaultdictclassProblematicFactory:def__call__(self)->dict[str,str]:return {}def__repr__(self):print(f"dd:{repr(dd)}")returnf"ProblematicFactory for{repr(dd)}"dd=defaultdict[str,dict[str,str]](ProblematicFactory())print(repr(dd.default_factory))
Running this gives...
% python3 repro.pydd: defaultdict(..., {})... LOTS OF THISdd: defaultdict(..., {})Traceback (most recent call last): File "/Users/thomas.kowalski/Documents/cpython/repro.py", line 13, in <module> print(repr(dd.default_factory)) ~~~~^^^^^^^^^^^^^^^^^^^^ File "/Users/thomas.kowalski/Documents/cpython/repro.py", line 8, in __repr__ print(f"dd: {repr(dd)}") ~~~~^^^^ File "/Users/thomas.kowalski/Documents/cpython/repro.py", line 10, in __repr__ return f"ProblematicFactory for {repr(dd)}" ~~~~^^^^ File "/Users/thomas.kowalski/Documents/cpython/repro.py", line 10, in __repr__ return f"ProblematicFactory for {repr(dd)}" ~~~~^^^^ File "/Users/thomas.kowalski/Documents/cpython/repro.py", line 10, in __repr__ return f"ProblematicFactory for {repr(dd)}" ~~~~^^^^ [Previous line repeated 995 more times]Going back to the current code:
intstatus=Py_ReprEnter(dd->default_factory);if (status!=0) {// if we already are in a __repr__ call for default factoryif (status<0) {Py_DECREF(baserepr);returnNULL;// we early-return without "leaving" which is correct }defrepr=PyUnicode_FromString("...");}else// we are NOT in a __repr__ call for default factorydefrepr=PyObject_Repr(dd->default_factory);// no early return, we fall through// we may or may not be in a __repr__ call for default factory// resetting here is not OK, it's not our responsibilityPy_ReprLeave(dd->default_factory);
As far as I can tell, the fix would be to move the finalPy_ReprLeave to theelse body so that if we already are in the factory's__repr__ we don't reset it ourselves, we let whoever entered it reset it.
Note I have a branch ready with that fix and can open it here if/once the bug is confirmed.