https://github.com/python/cpython/commit/4060ef36e18d712fb1022aff798827a71fdcb09a
commit: 4060ef36e18d712fb1022aff798827a71fdcb09a
branch: 3.13
author: Sam Gross <[email protected]>
committer: colesbury <[email protected]>
date: 2024-12-04T15:17:19-05:00
summary:
[3.13] gh-127536: Add missing locks in listobject.c (GH-127580) (GH-127613)
We were missing locks around some list operations in the free threading
build.
(cherry picked from commit e51da64ac3bc6cd45339864db32d05115af39ead)
files:
A Misc/NEWS.d/next/Core and
Builtins/2024-12-03-21-07-06.gh-issue-127536.3jMMrT.rst
M Objects/listobject.c
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
index 00000000000000..6e2b87fe38146b
--- /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 89abbda155a8b6..ee53a8bddbed44 100644
--- 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_pyatomic_ft_wrappers.h"
#include "pycore_interp.h" // PyInterpreterState.list
@@ -81,6 +82,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) &&
@@ -995,10 +1001,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)) {
@@ -1475,7 +1483,9 @@ PyList_Clear(PyObject *self)
PyErr_BadInternalCall();
return -1;
}
+ Py_BEGIN_CRITICAL_SECTION(self);
list_clear((PyListObject*)self);
+ Py_END_CRITICAL_SECTION();
return 0;
}
@@ -3446,7 +3456,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) {
@@ -3619,8 +3631,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);
@@ -3628,7 +3642,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;
@@ -3648,7 +3662,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;
@@ -3714,10 +3728,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,
@@ -3731,7 +3743,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;
}
@@ -3787,6 +3799,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,
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]