Skip to content

gh-132713: Fix repr(list) race condition #132801

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2025
Merged

gh-132713: Fix repr(list) race condition #132801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Lib/test/test_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,19 @@ def test_list_resize_overflow(self):
with self.assertRaises((MemoryError, OverflowError)):
lst *= size

def test_repr_mutate(self):
class Obj:
@staticmethod
def __repr__():
try:
mylist.pop()
Copy link
Member Author

Choose a reason for hiding this comment

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

__repr__() doesn't use self (static method) and so doesn't crash if self becomes a dangling pointer (borrowed reference). If __repr__() uses `self (regular method), it works around the bug since self stores a strong reference.

I didn't see how to write a test which does crash without the fix.

I played with weakref.ref to check indirectly if we are called with a strong reference, but it made the test too complicated/fragile for little benefits. I gave up on this way.

except IndexError:
pass
return 'obj'

mylist = [Obj() for _ in range(5)]
self.assertEqual(repr(mylist), '[obj, obj, obj]')

def test_repr_large(self):
# Check the repr of large list objects
def check(n):
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-04-22-16-38-43.gh-issue-132713.mBWTSZ.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix ``repr(list)`` race condition: hold a strong reference to the item while
calling ``repr(item)``. Patch by Victor Stinner.
12 changes: 11 additions & 1 deletion Objects/listobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ list_repr_impl(PyListObject *v)
/* "[" + "1" + ", 2" * (len - 1) + "]" */
Py_ssize_t prealloc = 1 + 1 + (2 + 1) * (Py_SIZE(v) - 1) + 1;
PyUnicodeWriter *writer = PyUnicodeWriter_Create(prealloc);
PyObject *item = NULL;
if (writer == NULL) {
goto error;
}
Expand All @@ -594,6 +595,13 @@ list_repr_impl(PyListObject *v)
/* Do repr() on each element. Note that this may mutate the list,
so must refetch the list size on each iteration. */
for (Py_ssize_t i = 0; i < Py_SIZE(v); ++i) {
item = list_get_item_ref(v, i);
if (item == NULL) {
// List truncated while iterating on it
PyErr_Clear();
break;
}

if (i > 0) {
if (PyUnicodeWriter_WriteChar(writer, ',') < 0) {
goto error;
Expand All @@ -603,9 +611,10 @@ list_repr_impl(PyListObject *v)
}
}

if (PyUnicodeWriter_WriteRepr(writer, v->ob_item[i]) < 0) {
if (PyUnicodeWriter_WriteRepr(writer, item) < 0) {
goto error;
}
Py_CLEAR(item);
}

if (PyUnicodeWriter_WriteChar(writer, ']') < 0) {
Expand All @@ -616,6 +625,7 @@ list_repr_impl(PyListObject *v)
return PyUnicodeWriter_Finish(writer);

error:
Py_XDECREF(item);
PyUnicodeWriter_Discard(writer);
Py_ReprLeave((PyObject *)v);
return NULL;
Expand Down
Loading