8000 gh-112087: Make list_{concat, repeat, inplace_repeat, ass_item) to b… · python/cpython@259730b · GitHub
[go: up one dir, main page]
Skip to content

Commit 259730b

Browse files
corona10corona10
authored
gh-112087: Make list_{concat, repeat, inplace_repeat, ass_item) to be thread-safe (gh-115605)
1 parent 5407146 commit 259730b

File tree

2 files changed

+88
-40
lines changed

2 files changed

+88
-40
lines changed

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,17 @@ extern "C" {
2323
#define FT_ATOMIC_LOAD_SSIZE(value) _Py_atomic_load_ssize(&value)
2424
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) \
2525
_Py_atomic_load_ssize_relaxed(&value)
26+
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
27+
_Py_atomic_store_ptr_relaxed(&value, new_value)
28+
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) \
29+
_Py_atomic_store_ptr_release(&value, new_value)
2630
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
2731
_Py_atomic_store_ssize_relaxed(&value, new_value)
2832
#else
2933
#define FT_ATOMIC_LOAD_SSIZE(value) value
3034
#define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value
35+
#define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
36+
#define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
3137
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
3238
#endif
3339

‎Objects/listobject.c

Lines changed: 82 additions & 40 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_pyatomic_ft_wrappers.h"
67
#include "pycore_interp.h" // PyInterpreterState.list
78
#include "pycore_list.h" // struct _Py_list_freelist, _PyListIterObject
89
#include "pycore_long.h" // _PyLong_DigitCount
@@ -20,14 +21,6 @@ class list "PyListObject *" "&PyList_Type"
2021

2122
_Py_DECLARE_STR(list_err, "list index out of range");
2223

23-
#ifdef Py_GIL_DISABLED
24-
# define LOAD_SSIZE(value) _Py_atomic_load_ssize_relaxed(&value)
25-
# define STORE_SSIZE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value)
26-
#else
27-
# define LOAD_SSIZE(value) value
28-
# define STORE_SSIZE(value, new_value) value = new_value
29-
#endif
30-
3124
#ifdef WITH_FREELISTS
3225
static struct _Py_list_freelist *
3326
get_list_freelist(void)
@@ -563,20 +556,12 @@ PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh)
563556
}
564557

565558
static PyObject *
566-
list_concat(PyObject *aa, PyObject *bb)
559+
list_concat_lock_held(PyListObject *a, PyListObject *b)
567560
{
568-
PyListObject *a = (PyListObject *)aa;
569561
Py_ssize_t size;
570562
Py_ssize_t i;
571563
PyObject **src, **dest;
572564
PyListObject *np;
573-
if (!PyList_Check(bb)) {
574-
PyErr_Format(PyExc_TypeError,
575-
"can only concatenate list (not \"%.200s\") to list",
576-
Py_TYPE(bb)->tp_name);
577-
return NULL;
578-
}
579-
#define b ((PyListObject *)bb)
580565
assert((size_t)Py_SIZE(a) + (size_t)Py_SIZE(b) < PY_SSIZE_T_MAX);
581566
size = Py_SIZE(a) + Py_SIZE(b);
582567
if (size == 0) {
@@ -590,23 +575,39 @@ list_concat(PyObject *aa, PyObject *bb)
590575
dest = np->ob_item;
591576
for (i = 0; i < Py_SIZE(a); i++) {
592577
PyObject *v = src[i];
593-
dest[i] = Py_NewRef(v);
578+
FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v));
594579
}
595580
src = b->ob_item;
596581
dest = np->ob_item + Py_SIZE(a);
597582
for (i = 0; i < Py_SIZE(b); i++) {
598583
PyObject *v = src[i];
599-
dest[i] = Py_NewRef(v);
584+
FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v));
600585
}
601586
Py_SET_SIZE(np, size);
602587
return (PyObject *)np;
603-
#undef b
604588
}
605589

606590
static PyObject *
607-
list_repeat(PyObject *aa, Py_ssize_t n)
591+
list_concat(PyObject *aa, PyObject *bb)
608592
{
593+
if (!PyList_Check(bb)) {
594+
PyErr_Format(PyExc_TypeError,
595+
"can only concatenate list (not \"%.200s\") to list",
596+
Py_TYPE(bb)->tp_name);
597+
return NULL;
598+
}
609599
PyListObject *a = (PyListObject *)aa;
600+
PyListObject *b = (PyListObject *)bb;
601+
PyObject *ret;
602+
Py_BEGIN_CRITICAL_SECTION2(a, b);
603+
ret = list_concat_lock_held(a, b);
604+
Py_END_CRITICAL_SECTION2();
605+
return ret;
606+
}
607+
608+
static PyObject *
609+
list_repeat_lock_held(PyListObject *a, Py_ssize_t n)
610+
{
610611
const Py_ssize_t input_size = Py_SIZE(a);
611612
if (input_size == 0 || n <= 0)
612613
return PyList_New(0);
@@ -626,15 +627,15 @@ list_repeat(PyObject *aa, Py_ssize_t n)
626627
_Py_RefcntAdd(elem, n);
627628
PyObject **dest_end = dest + output_size;
628629
while (dest < dest_end) {
629-
*dest++ = elem;
630+
FT_ATOMIC_STORE_PTR_RELAXED(*dest++, elem);
630631
}
631632
}
632633
else {
633634
PyObject **src = a->ob_item;
634635
PyObject **src_end = src + input_size;
635636
while (src < src_end) {
636637
_Py_RefcntAdd(*src, n);
637-
*dest++ = *src++;
638+
FT_ATOMIC_STORE_PTR_RELAXED(*dest++, *src++);
638639
}
639640

640641
_Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
@@ -645,6 +646,17 @@ list_repeat(PyObject *aa, Py_ssize_t n)
645646
return (PyObject *) np;
646647
}
647648

649+
static PyObject *
650+
list_repeat(PyObject *aa, Py_ssize_t n)
651+
{
652+
PyObject *ret;
653+
PyListObject *a = (PyListObject *)aa;
654+
Py_BEGIN_CRITICAL_SECTION(a);
655+
ret = list_repeat_lock_held(a, n);
656+
Py_END_CRITICAL_SECTION();
657+
return ret;
658+
}
659+
648660
static void
649661
list_clear(PyListObject *a)
650662
{
@@ -657,11 +669,12 @@ list_clear(PyListObject *a)
657669
this list, we make it empty first. */
658670
Py_ssize_t i = Py_SIZE(a);
659671
Py_SET_SIZE(a, 0);
660-
a->ob_item = NULL;
672+
FT_ATOMIC_STORE_PTR_RELEASE(a->ob_item, NULL);
661673
a->allocated = 0;
662674
while (--i >= 0) {
663675
Py_XDECREF(items[i]);
664676
}
677+
// TODO: Use QSBR technique, if the list is shared between threads,
665678
PyMem_Free(items);
666679

667680
// Note that there is no guarantee that the list is actually empty
@@ -798,9 +811,8 @@ PyList_SetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v)
798811
}
799812

800813
static PyObject *
801-
list_inplace_repeat(PyObject *_self, Py_ssize_t n)
814+
list_inplace_repeat_lock_held(PyListObject *self, Py_ssize_t n)
802815
{
803-
PyListObject *self = (PyListObject *)_self;
804816
Py_ssize_t input_size = PyList_GET_SIZE(self);
805817
if (input_size == 0 || n == 1) {
806818
return Py_NewRef(self);
@@ -829,21 +841,51 @@ list_inplace_repeat(PyObject *_self, Py_ssize_t n)
829841
return Py_NewRef(self);
830842
}
831843

844+
static PyObject *
845+
list_inplace_repeat(PyObject *_self, Py_ssize_t n)
846+
{
847+
PyObject *ret;
848+
PyListObject *self = (PyListObject *) _self;
849+
Py_BEGIN_CRITICAL_SECTION(self);
850+
ret = list_inplace_repeat_lock_held(self, n);
851+
Py_END_CRITICAL_SECTION();
852+
return ret;
853+
}
854+
832855
static int
833-
list_ass_item(PyObject *aa, Py_ssize_t i, PyObject *v)
856+
list_ass_item_lock_held(PyListObject *a, Py_ssize_t i, PyObject *v)
834857
{
835-
PyListObject *a = (PyListObject *)aa;
836858
if (!valid_index(i, Py_SIZE(a))) {
837859
PyErr_SetString(PyExc_IndexError,
838860
"list assignment index out of range");
839861
return -1;
840862
}
841-
if (v == NULL)
842-
return list_ass_slice(a, i, i+1, v);
843-
Py_SETREF(a->ob_item[i], Py_NewRef(v));
863+
PyObject *tmp = a->ob_item[i];
864+
if (v == NULL) {
865+
Py_ssize_t size = Py_SIZE(a);
866+
for (Py_ssize_t idx = i; idx < size - 1; idx++) {
867+
FT_ATOMIC_STORE_PTR_RELAXED(a->ob_item[idx], a->ob_item[idx + 1]);
868+
}
869+
Py_SET_SIZE(a, size - 1);
870+
}
871+
else {
872+
FT_ATOMIC_STORE_PTR_RELEASE(a->ob_item[i], Py_NewRef(v));
873+
}
874+
Py_DECREF(tmp);
844875
return 0;
845876
}
846877

878+
static int
879+
list_ass_item(PyObject *aa, Py_ssize_t i, PyObject *v)
880+
{
881+
int ret;
882+
PyListObject *a = (PyListObject *)aa;
883+
Py_BEGIN_CRITICAL_SECTION(a);
884+
ret = list_ass_item_lock_held(a, i, v);
885+
Py_END_CRITICAL_SECTION();
886+
return ret;
887+
}
888+
847889
/*[clinic input]
848890
@critical_section
849891
list.insert
@@ -2979,7 +3021,7 @@ list___sizeof___impl(PyListObject *self)
29793021
/*[clinic end generated code: output=3417541f95f9a53e input=b8030a5d5ce8a187]*/
29803022
{
29813023
size_t res = _PyObject_SIZE(Py_TYPE(self));
2982-
Py_ssize_t allocated = LOAD_SSIZE(self->allocated);
3024+
Py_ssize_t allocated = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->allocated);
29833025
res += (size_t)allocated * sizeof(void*);
29843026
return PyLong_FromSize_t(res);
29853027
}
@@ -3382,23 +3424,23 @@ static PyObject *
33823424
listiter_next(PyObject *self)
33833425
{
33843426
_PyListIterObject *it = (_PyListIterObject *)self;
3385-
Py_ssize_t index = LOAD_SSIZE(it->it_index);
3427+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index);
33863428
if (index < 0) {
33873429
return NULL;
33883430
}
33893431

33903432
PyObject *item = list_get_item_ref(it->it_seq, index);
33913433
if (item == NULL) {
33923434
// out-of-bounds
3393-
STORE_SSIZE(it->it_index, -1);
3435+
FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, -1);
33943436
#ifndef Py_GIL_DISABLED
33953437
PyListObject *seq = it->it_seq;
33963438
it->it_seq = NULL;
33973439
Py_DECREF(seq);
33983440
#endif
33993441
return NULL;
34003442
}
3401-
STORE_SSIZE(it->it_index, index + 1);
3443+
FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index + 1);
34023444
return item;
34033445
}
34043446

@@ -3407,7 +3449,7 @@ listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored))
34073449
{
34083450
assert(self != NULL);
34093451
_PyListIterObject *it = (_PyListIterObject *)self;
3410-
Py_ssize_t index = LOAD_SSIZE(it->it_index);
3452+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index);
34113453
if (index >= 0) {
34123454
Py_ssize_t len = PyList_GET_SIZE(it->it_seq) - index;
34133455
if (len >= 0)
@@ -3537,7 +3579,7 @@ listreviter_next(PyObject *self)
35373579
{
35383580
listreviterobject *it = (listreviterobject *)self;
35393581
assert(it != NULL);
3540-
Py_ssize_t index = LOAD_SSIZE(it->it_index);
3582+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index);
35413583
if (index < 0) {
35423584
return NULL;
35433585
}
@@ -3546,10 +3588,10 @@ listreviter_next(PyObject *self)
35463588
assert(PyList_Check(seq));
35473589
PyObject *item = list_get_item_ref(seq, index);
35483590
if (item != NULL) {
3549-
STORE_SSIZE(it->it_index, index - 1);
3591+
FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, index - 1);
35503592
return item;
35513593
}
3552-
STORE_SSIZE(it->it_index, -1);
3594+
FT_ATOMIC_STORE_SSIZE_RELAXED(it->it_index, -1);
35533595
#ifndef Py_GIL_DISABLED
35543596
it->it_seq = NULL;
35553597
Py_DECREF(seq);
@@ -3561,7 +3603,7 @@ static PyObject *
35613603
listreviter_len(PyObject *self, PyObject *Py_UNUSED(ignored))
35623604
{
35633605
listreviterobject *it = (listreviterobject *)self;
3564-
Py_ssize_t index = LOAD_SSIZE(it->it_index);
3606+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(it->it_index);
35653607
Py_ssize_t len = index + 1;
35663608
if (it->it_seq == NULL || PyList_GET_SIZE(it->it_seq) < len)
35673609
len = 0;

0 commit comments

Comments
 (0)
0