Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 259730b

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

File tree

Expand file treeCollapse file tree

2 files changed

+88
-40
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+88
-40
lines changed

‎Include/internal/pycore_pyatomic_ft_wrappers.h

Copy file name to clipboardExpand all lines: Include/internal/pycore_pyatomic_ft_wrappers.h
+6Lines 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

Copy file name to clipboardExpand all lines: Objects/listobject.c
+82-40Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.