Skip to content

Commit

Permalink
[3.13] pythongh-127536: Add missing locks in listobject.c (pythonGH-1…
Browse files Browse the repository at this point in the history
…27580) (pythonGH-127613)

We were missing locks around some list operations in the free threading
build.
(cherry picked from commit e51da64)
  • Loading branch information
colesbury authored Dec 4, 2024
1 parent a1c52d1 commit 4060ef3
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add missing locks around some list assignment operations in the free
threading build.
50 changes: 40 additions & 10 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) &&
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -3619,16 +3631,18 @@ 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);
if (i == -1 && PyErr_Occurred())
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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4060ef3

Please sign in to comment.