Skip to content

Commit 4060ef3

Browse files
authored
[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 e51da64)
1 parent a1c52d1 commit 4060ef3

File tree

2 files changed

+42
-10
lines changed

2 files changed

+42
-10
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Add missing locks around some list assignment operations in the free
2+
threading build.

Objects/listobject.c

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "Python.h"
44
#include "pycore_abstract.h" // _PyIndex_Check()
55
#include "pycore_ceval.h" // _PyEval_GetBuiltin()
6+
#include "pycore_critical_section.h" // _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED()
67
#include "pycore_dict.h" // _PyDictViewObject
78
#include "pycore_pyatomic_ft_wrappers.h"
89
#include "pycore_interp.h" // PyInterpreterState.list
@@ -81,6 +82,11 @@ static void
8182
ensure_shared_on_resize(PyListObject *self)
8283
{
8384
#ifdef Py_GIL_DISABLED
85+
// We can't use _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED here because
86+
// the `CALL_LIST_APPEND` bytecode handler may lock the list without
87+
// a critical section.
88+
assert(Py_REFCNT(self) == 1 || PyMutex_IsLocked(&_PyObject_CAST(self)->ob_mutex));
89+
8490
// Ensure that the list array is freed using QSBR if we are not the
8591
// owning thread.
8692
if (!_Py_IsOwnedByCurrentThread((PyObject *)self) &&
@@ -995,10 +1001,12 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v)
9951001
Py_ssize_t n = PyList_GET_SIZE(a);
9961002
PyObject *copy = list_slice_lock_held(a, 0, n);
9971003
if (copy == NULL) {
998-
return -1;
1004+
ret = -1;
1005+
}
1006+
else {
1007+
ret = list_ass_slice_lock_held(a, ilow, ihigh, copy);
1008+
Py_DECREF(copy);
9991009
}
1000-
ret = list_ass_slice_lock_held(a, ilow, ihigh, copy);
1001-
Py_DECREF(copy);
10021010
Py_END_CRITICAL_SECTION();
10031011
}
10041012
else if (v != NULL && PyList_CheckExact(v)) {
@@ -1475,7 +1483,9 @@ PyList_Clear(PyObject *self)
14751483
PyErr_BadInternalCall();
14761484
return -1;
14771485
}
1486+
Py_BEGIN_CRITICAL_SECTION(self);
14781487
list_clear((PyListObject*)self);
1488+
Py_END_CRITICAL_SECTION();
14791489
return 0;
14801490
}
14811491

@@ -3446,7 +3456,9 @@ list___init___impl(PyListObject *self, PyObject *iterable)
34463456

34473457
/* Empty previous contents */
34483458
if (self->ob_item != NULL) {
3459+
Py_BEGIN_CRITICAL_SECTION(self);
34493460
list_clear(self);
3461+
Py_END_CRITICAL_SECTION();
34503462
}
34513463
if (iterable != NULL) {
34523464
if (_list_extend(self, iterable) < 0) {
@@ -3619,16 +3631,18 @@ adjust_slice_indexes(PyListObject *lst,
36193631
}
36203632

36213633
static int
3622-
list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
3634+
list_ass_subscript_lock_held(PyObject *_self, PyObject *item, PyObject *value)
36233635
{
3636+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(_self);
3637+
36243638
PyListObject *self = (PyListObject *)_self;
36253639
if (_PyIndex_Check(item)) {
36263640
Py_ssize_t i = PyNumber_AsSsize_t(item, PyExc_IndexError);
36273641
if (i == -1 && PyErr_Occurred())
36283642
return -1;
36293643
if (i < 0)
36303644
i += PyList_GET_SIZE(self);
3631-
return list_ass_item((PyObject *)self, i, value);
3645+
return list_ass_item_lock_held(self, i, value);
36323646
}
36333647
else if (PySlice_Check(item)) {
36343648
Py_ssize_t start, stop, step;
@@ -3648,7 +3662,7 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
36483662
step);
36493663

36503664
if (step == 1)
3651-
return list_ass_slice(self, start, stop, value);
3665+
return list_ass_slice_lock_held(self, start, stop, value);
36523666

36533667
if (slicelength <= 0)
36543668
return 0;
@@ -3714,10 +3728,8 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
37143728

37153729
/* protect against a[::-1] = a */
37163730
if (self == (PyListObject*)value) {
3717-
Py_BEGIN_CRITICAL_SECTION(value);
3718-
seq = list_slice_lock_held((PyListObject*)value, 0,
3731+
seq = list_slice_lock_held((PyListObject *)value, 0,
37193732
Py_SIZE(value));
3720-
Py_END_CRITICAL_SECTION();
37213733
}
37223734
else {
37233735
seq = PySequence_Fast(value,
@@ -3731,7 +3743,7 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
37313743
step);
37323744

37333745
if (step == 1) {
3734-
int res = list_ass_slice(self, start, stop, seq);
3746+
int res = list_ass_slice_lock_held(self, start, stop, seq);
37353747
Py_DECREF(seq);
37363748
return res;
37373749
}
@@ -3787,6 +3799,24 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value)
37873799
}
37883800
}
37893801

3802+
static int
3803+
list_ass_subscript(PyObject *self, PyObject *item, PyObject *value)
3804+
{
3805+
int res;
3806+
#ifdef Py_GIL_DISABLED
3807+
if (PySlice_Check(item) && value != NULL && PyList_CheckExact(value)) {
3808+
Py_BEGIN_CRITICAL_SECTION2(self, value);
3809+
res = list_ass_subscript_lock_held(self, item, value);
3810+
Py_END_CRITICAL_SECTION2();
3811+
return res;
3812+
}
3813+
#endif
3814+
Py_BEGIN_CRITICAL_SECTION(self);
3815+
res = list_ass_subscript_lock_held(self, item, value);
3816+
Py_END_CRITICAL_SECTION();
3817+
return res;
3818+
}
3819+
37903820
static PyMappingMethods list_as_mapping = {
37913821
list_length,
37923822
list_subscript,

0 commit comments

Comments
 (0)