gh-127536: Add missing locks in listobject.c (GH-127580)

We were missing locks around some list operations in the free threading
build.

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-12-03-21-07-06.gh-issue-127536.3jMMrT.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-03-21-07-06.gh-issue-127536.3jMMrT.rst
new file mode 100644 (file)
index 0000000..6e2b87f
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-03-21-07-06.gh-issue-127536.3jMMrT.rst
@@ -0,0 +1,2 @@
+Add missing locks around some list assignment operations in the free
+threading build.

diff --git a/Objects/listobject.c b/Objects/listobject.c
index 8abe9e8933420bb14cfd4b735c566b21410b09ce..3832295600a0ab006cc026fdf93ed6a5f1ecb786 100644 (file)
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -3,6 +3,7 @@
 #include "Python.h"
 #include "pycore_abstract.h"      // _PyIndex_Check()
 #include "pycore_ceval.h"         // _PyEval_GetBuiltin()
+#include "pycore_critical_section.h"  // _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED()
 #include "pycore_dict.h"          // _PyDictViewObject
 #include "pycore_freelist.h"      // _Py_FREELIST_FREE(), _Py_FREELIST_POP()
 #include "pycore_pyatomic_ft_wrappers.h"
@@ -72,6 +73,11 @@ static void
 ensure_shared_on_resize(PyListObject *self)
 {
 #ifdef Py_GIL_DISABLED
+    // We can't use _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED here because
+    // the `CALL_LIST_APPEND` bytecode handler may lock the list without
+    // a critical section.
+    assert(Py_REFCNT(self) == 1 || PyMutex_IsLocked(&_PyObject_CAST(self)->ob_mutex));
+
     // Ensure that the list array is freed using QSBR if we are not the
     // owning thread.
     if (!_Py_IsOwnedByCurrentThread((PyObject *)self) &&
@@ -957,10 +963,12 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v)
         Py_ssize_t n = PyList_GET_SIZE(a);
         PyObject *copy = list_slice_lock_held(a, 0, n);
         if (copy == NULL) {
-            return -1;
+            ret = -1;
+        }
+        else {
+            ret = list_ass_slice_lock_held(a, ilow, ihigh, copy);
+            Py_DECREF(copy);
         }
-        ret = list_ass_slice_lock_held(a, ilow, ihigh, copy);
-        Py_DECREF(copy);
         Py_END_CRITICAL_SECTION();
     }
     else if (v != NULL && PyList_CheckExact(v)) {
@@ -1437,7 +1445,9 @@ PyList_Clear(PyObject *self)
         PyErr_BadInternalCall();
         return -1;
     }
+    Py_BEGIN_CRITICAL_SECTION(self);
     list_clear((PyListObject*)self);
+    Py_END_CRITICAL_SECTION();
     return 0;
 }
 
@@ -3410,7 +3420,9 @@ list___init___impl(PyListObject *self, PyObject *iterable)
 
     /* Empty previous contents */
     if (self->ob_item != NULL) {
+        Py_BEGIN_CRITICAL_SECTION(self);
         list_clear(self);
+        Py_END_CRITICAL_SECTION();
     }
     if (iterable != NULL) {
         if (_list_extend(self, iterable) < 0) {
@@ -3583,8 +3595,10 @@ adjust_slice_indexes(PyListObject *lst,
 }
 
 static int
-list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
+list_ass_subscript_lock_held(PyObject *_self, PyObject *item, PyObject *value)
 {
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(_self);
+
     PyListObject *self = (PyListObject *)_self;
     if (_PyIndex_Check(item)) {
         Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
@@ -3592,7 +3606,7 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
             return -1;
         if (i < 0)
             i += PyList_GET_SIZE(self);
-        return list_ass_item((PyObject *)self, i, value);
+        return list_ass_item_lock_held(self, i, value);
     }
     else if (PySlice_Check(item)) {
         Py_ssize_t start, stop, step;
@@ -3612,7 +3626,7 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
                                                           step);
 
             if (step == 1)
-                return list_ass_slice(self, start, stop, value);
+                return list_ass_slice_lock_held(self, start, stop, value);
 
             if (slicelength <= 0)
                 return 0;
@@ -3678,10 +3692,8 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
 
             /* protect against a[::-1] = a */
             if (self == (PyListObject*)value) {
-                Py_BEGIN_CRITICAL_SECTION(value);
-                seq = list_slice_lock_held((PyListObject*)value, 0,
+                seq = list_slice_lock_held((PyListObject *)value, 0,
                                             Py_SIZE(value));
-                Py_END_CRITICAL_SECTION();
             }
             else {
                 seq = PySequence_Fast(value,
@@ -3695,7 +3707,7 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
                                                           step);
 
             if (step == 1) {
-                int res = list_ass_slice(self, start, stop, seq);
+                int res = list_ass_slice_lock_held(self, start, stop, seq);
                 Py_DECREF(seq);
                 return res;
             }
@@ -3751,6 +3763,24 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
     }
 }
 
+static int
+list_ass_subscript(PyObject *self, PyObject *item, PyObject *value)
+{
+    int res;
+#ifdef Py_GIL_DISABLED
+    if (PySlice_Check(item) && value != NULL && PyList_CheckExact(value)) {
+        Py_BEGIN_CRITICAL_SECTION2(self, value);
+        res = list_ass_subscript_lock_held(self, item, value);
+        Py_END_CRITICAL_SECTION2();
+        return res;
+    }
+#endif
+    Py_BEGIN_CRITICAL_SECTION(self);
+    res = list_ass_subscript_lock_held(self, item, value);
+    Py_END_CRITICAL_SECTION();
+    return res;
+}
+
 static PyMappingMethods list_as_mapping = {
     list_length,
     list_subscript,