From: Sam Gross Date: Tue, 11 Feb 2025 22:29:27 +0000 (-0500) Subject: gh-129967: Fix race condition in `repr(set)` (gh-129978) X-Git-Tag: v3.14.0a6~459 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a7427f2db937adb4c787754deb4c337f1894fe86;p=thirdparty%2FPython%2Fcpython.git gh-129967: Fix race condition in `repr(set)` (gh-129978) The call to `PySequence_List()` could temporarily unlock and relock the set, allowing the items to be cleared and return the incorrect notation `{}` for a empty set (it should be `set()`). Co-authored-by: T. Wouters --- diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py new file mode 100644 index 000000000000..a66e03bcc4c9 --- /dev/null +++ b/Lib/test/test_free_threading/test_set.py @@ -0,0 +1,41 @@ +import unittest + +from threading import Thread, Barrier +from unittest import TestCase + +from test.support import threading_helper + + +@threading_helper.requires_working_threading() +class TestSet(TestCase): + def test_repr_clear(self): + """Test repr() of a set while another thread is calling clear()""" + NUM_ITERS = 10 + NUM_REPR_THREADS = 10 + barrier = Barrier(NUM_REPR_THREADS + 1) + s = {1, 2, 3, 4, 5, 6, 7, 8} + + def clear_set(): + barrier.wait() + s.clear() + + def repr_set(): + barrier.wait() + set_reprs.append(repr(s)) + + for _ in range(NUM_ITERS): + set_reprs = [] + threads = [Thread(target=clear_set)] + for _ in range(NUM_REPR_THREADS): + threads.append(Thread(target=repr_set)) + for t in threads: + t.start() + for t in threads: + t.join() + + for set_repr in set_reprs: + self.assertIn(set_repr, ("set()", "{1, 2, 3, 4, 5, 6, 7, 8}")) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-10-20-01-56.gh-issue-129967.J60tEl.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-10-20-01-56.gh-issue-129967.J60tEl.rst new file mode 100644 index 000000000000..69ec03d2ae38 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-10-20-01-56.gh-issue-129967.J60tEl.rst @@ -0,0 +1,2 @@ +Fix a race condition in the :term:`free threading` build when ``repr(set)`` +is called concurrently with ``set.clear()``. diff --git a/Objects/setobject.c b/Objects/setobject.c index 26ab352ca6d9..1978ae2b165d 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -535,9 +535,18 @@ set_repr_lock_held(PySetObject *so) return PyUnicode_FromFormat("%s()", Py_TYPE(so)->tp_name); } - keys = PySequence_List((PyObject *)so); - if (keys == NULL) + // gh-129967: avoid PySequence_List because it might re-lock the object + // lock or the GIL and allow something to clear the set from underneath us. + keys = PyList_New(so->used); + if (keys == NULL) { goto done; + } + + Py_ssize_t pos = 0, idx = 0; + setentry *entry; + while (set_next(so, &pos, &entry)) { + PyList_SET_ITEM(keys, idx++, Py_NewRef(entry->key)); + } /* repr(keys)[1:-1] */ listrepr = PyObject_Repr(keys);