]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-129967: Fix race condition in `repr(set)` (gh-129978)
authorSam Gross <colesbury@gmail.com>
Tue, 11 Feb 2025 22:29:27 +0000 (17:29 -0500)
committerGitHub <noreply@github.com>
Tue, 11 Feb 2025 22:29:27 +0000 (17:29 -0500)
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 <thomas@python.org>
Lib/test/test_free_threading/test_set.py [new file with mode: 0644]
Misc/NEWS.d/next/Core_and_Builtins/2025-02-10-20-01-56.gh-issue-129967.J60tEl.rst [new file with mode: 0644]
Objects/setobject.c

diff --git a/Lib/test/test_free_threading/test_set.py b/Lib/test/test_free_threading/test_set.py
new file mode 100644 (file)
index 0000000..a66e03b
--- /dev/null
@@ -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 (file)
index 0000000..69ec03d
--- /dev/null
@@ -0,0 +1,2 @@
+Fix a race condition in the :term:`free threading` build when ``repr(set)``
+is called concurrently with ``set.clear()``.
index 26ab352ca6d989d6c8d498925ac5ae2677549b97..1978ae2b165d18585f421166f12396c91f13ff67 100644 (file)
@@ -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);