From 4b8ac83521c30577c4b0cfd3f37f43c30703b7d9 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 3 Feb 2024 00:20:55 +0900 Subject: [PATCH 1/4] gh-112087: Make list_{count, index, contains} to be thread-safe. --- Objects/listobject.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 82a4ba952de07d..b263305fee7366 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -478,18 +478,21 @@ list_length(PyObject *a) static int list_contains(PyObject *aa, PyObject *el) { - PyListObject *a = (PyListObject *)aa; - PyObject *item; - Py_ssize_t i; - int cmp; - for (i = 0, cmp = 0 ; cmp == 0 && i < Py_SIZE(a); ++i) { - item = PyList_GET_ITEM(a, i); - Py_INCREF(item); - cmp = PyObject_RichCompareBool(item, el, Py_EQ); + for (Py_ssize_t i = 0; ; i++) { + PyObject *item = PyList_GetItemRef(aa, i); + if (item == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { + // out-of-bounds + PyErr_Clear(); + return 0; + } + int cmp = PyObject_RichCompareBool(item, el, Py_EQ); Py_DECREF(item); + if (cmp != 0) { + return cmp; + } } - return cmp; + return 0; } static PyObject * @@ -2737,8 +2740,11 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, stop = 0; } for (i = start; i < stop && i < Py_SIZE(self); i++) { - PyObject *obj = self->ob_item[i]; - Py_INCREF(obj); + PyObject *obj = PyList_GetItemRef((PyObject *)self, i); + if (obj == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { + PyErr_Clear(); + break; + } int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); Py_DECREF(obj); if (cmp > 0) @@ -2764,15 +2770,17 @@ list_count(PyListObject *self, PyObject *value) /*[clinic end generated code: output=b1f5d284205ae714 input=3bdc3a5e6f749565]*/ { Py_ssize_t count = 0; - Py_ssize_t i; - - for (i = 0; i < Py_SIZE(self); i++) { - PyObject *obj = self->ob_item[i]; + for (Py_ssize_t i = 0; ; i++) { + PyObject *obj = PyList_GetItemRef((PyObject *)self, i); + if (obj == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { + PyErr_Clear(); + break; + } if (obj == value) { count++; + Py_DECREF(obj); continue; } - Py_INCREF(obj); int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); Py_DECREF(obj); if (cmp > 0) From a702b046b22db76d7e1db4350f1c6b7e5c112ef9 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 3 Feb 2024 19:04:31 +0900 Subject: [PATCH 2/4] Address code review --- Objects/listobject.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index b263305fee7366..03e96b46f72fbb 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -272,6 +272,15 @@ PyList_GetItemRef(PyObject *op, Py_ssize_t i) return Py_NewRef(PyList_GET_ITEM(op, i)); } +static inline PyObject* +_PyList_GetItemRef(PyListObject *op, Py_ssize_t i) +{ + if (!valid_index(i, Py_SIZE(op))) { + return NULL; + } + return Py_NewRef(PyList_GET_ITEM(op, i)); +} + int PyList_SetItem(PyObject *op, Py_ssize_t i, PyObject *newitem) @@ -480,10 +489,9 @@ list_contains(PyObject *aa, PyObject *el) { for (Py_ssize_t i = 0; ; i++) { - PyObject *item = PyList_GetItemRef(aa, i); - if (item == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { + PyObject *item = _PyList_GetItemRef((PyListObject *)aa, i); + if (item == NULL) { // out-of-bounds - PyErr_Clear(); return 0; } int cmp = PyObject_RichCompareBool(item, el, Py_EQ); @@ -2740,9 +2748,9 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, stop = 0; } for (i = start; i < stop && i < Py_SIZE(self); i++) { - PyObject *obj = PyList_GetItemRef((PyObject *)self, i); - if (obj == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { - PyErr_Clear(); + PyObject *obj = _PyList_GetItemRef(self, i); + if (obj == NULL) { + // out-of-bounds break; } int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); @@ -2771,9 +2779,9 @@ list_count(PyListObject *self, PyObject *value) { Py_ssize_t count = 0; for (Py_ssize_t i = 0; ; i++) { - PyObject *obj = PyList_GetItemRef((PyObject *)self, i); - if (obj == NULL && PyErr_ExceptionMatches(PyExc_IndexError)) { - PyErr_Clear(); + PyObject *obj = _PyList_GetItemRef(self, i); + if (obj == NULL) { + // out-of-bounds break; } if (obj == value) { From 2e44fab9e0819844ca1181c3c7afa355d7d4bdfa Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 6 Feb 2024 08:27:25 +0900 Subject: [PATCH 3/4] Address code review --- Objects/listobject.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 03e96b46f72fbb..3838be99b56ec1 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -273,7 +273,7 @@ PyList_GetItemRef(PyObject *op, Py_ssize_t i) } static inline PyObject* -_PyList_GetItemRef(PyListObject *op, Py_ssize_t i) +list_get_item_ref(PyListObject *op, Py_ssize_t i) { if (!valid_index(i, Py_SIZE(op))) { return NULL; @@ -489,7 +489,7 @@ list_contains(PyObject *aa, PyObject *el) { for (Py_ssize_t i = 0; ; i++) { - PyObject *item = _PyList_GetItemRef((PyListObject *)aa, i); + PyObject *item = list_get_item_ref((PyListObject *)aa, i); if (item == NULL) { // out-of-bounds return 0; @@ -2748,7 +2748,7 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, stop = 0; } for (i = start; i < stop && i < Py_SIZE(self); i++) { - PyObject *obj = _PyList_GetItemRef(self, i); + PyObject *obj = list_get_item_ref(self, i); if (obj == NULL) { // out-of-bounds break; @@ -2779,7 +2779,7 @@ list_count(PyListObject *self, PyObject *value) { Py_ssize_t count = 0; for (Py_ssize_t i = 0; ; i++) { - PyObject *obj = _PyList_GetItemRef(self, i); + PyObject *obj = list_get_item_ref(self, i); if (obj == NULL) { // out-of-bounds break; From 853b31425b0c657a5c76d5cbab40a82b4744bddc Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 7 Feb 2024 00:59:46 +0900 Subject: [PATCH 4/4] Address code review --- Objects/listobject.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 3838be99b56ec1..307b8f1bd76cac 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -2735,8 +2735,6 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, Py_ssize_t stop) /*[clinic end generated code: output=ec51b88787e4e481 input=40ec5826303a0eb1]*/ { - Py_ssize_t i; - if (start < 0) { start += Py_SIZE(self); if (start < 0) @@ -2747,7 +2745,7 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, if (stop < 0) stop = 0; } - for (i = start; i < stop && i < Py_SIZE(self); i++) { + for (Py_ssize_t i = start; i < stop; i++) { PyObject *obj = list_get_item_ref(self, i); if (obj == NULL) { // out-of-bounds