Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-112087: Make __sizeof__ and listiter_{len, next} to be threadsafe#114843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
7186522e8be089777c9d5671a1bc61362aaa93faf644f202279989dbf4526f9a13beedbec617a6e170ccc8a8b81File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| For an empty reverse iterator for list will be reduced to :func:`reversed`. | ||
| Patch by Donghee Na |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -20,6 +20,14 @@ class list "PyListObject *" "&PyList_Type" | ||
| _Py_DECLARE_STR(list_err, "list index out of range"); | ||
| #ifdef Py_GIL_DISABLED | ||
| # define LOAD_SSIZE(value) _Py_atomic_load_ssize_relaxed(&value) | ||
| # define STORE_SSIZE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) | ||
| #else | ||
| # define LOAD_SSIZE(value) value | ||
| # define STORE_SSIZE(value, new_value) value = new_value | ||
| #endif | ||
| #ifdef WITH_FREELISTS | ||
| static struct _Py_list_state * | ||
| get_list_state(void) | ||
| @@ -2981,7 +2989,8 @@ list___sizeof___impl(PyListObject *self) | ||
| /*[clinic end generated code: output=3417541f95f9a53e input=b8030a5d5ce8a187]*/ | ||
| { | ||
| size_t res = _PyObject_SIZE(Py_TYPE(self)); | ||
| res += (size_t)self->allocated * sizeof(void*); | ||
| Py_ssize_t allocated = LOAD_SSIZE(self->allocated); | ||
| res += (size_t)allocated * sizeof(void*); | ||
| return PyLong_FromSize_t(res); | ||
| } | ||
| @@ -3383,33 +3392,34 @@ static PyObject * | ||
| listiter_next(PyObject *self) | ||
| { | ||
| _PyListIterObject *it = (_PyListIterObject *)self; | ||
| PyListObject *seq; | ||
| PyObject *item; | ||
| assert(it != NULL); | ||
| seq = it->it_seq; | ||
| if (seq == NULL) | ||
| Py_ssize_t index = LOAD_SSIZE(it->it_index); | ||
| if (index < 0) { | ||
| return NULL; | ||
| assert(PyList_Check(seq)); | ||
| if (it->it_index < PyList_GET_SIZE(seq)) { | ||
| item = PyList_GET_ITEM(seq, it->it_index); | ||
| ++it->it_index; | ||
| return Py_NewRef(item); | ||
| } | ||
| it->it_seq = NULL; | ||
| Py_DECREF(seq); | ||
| return NULL; | ||
| PyObject *item = list_get_item_ref(it->it_seq, index); | ||
| if (item == NULL) { | ||
| // out-of-bounds | ||
| STORE_SSIZE(it->it_index, -1); | ||
| #ifndef Py_GIL_DISABLED | ||
| PyListObject *seq = it->it_seq; | ||
| it->it_seq = NULL; | ||
| Py_DECREF(seq); | ||
| #endif | ||
| return NULL; | ||
| } | ||
| STORE_SSIZE(it->it_index, index + 1); | ||
| return item; | ||
| } | ||
| static PyObject * | ||
| listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) | ||
| { | ||
| assert(self != NULL); | ||
| _PyListIterObject *it = (_PyListIterObject *)self; | ||
| Py_ssize_t len; | ||
| if (it->it_seq) { | ||
| len = PyList_GET_SIZE(it->it_seq) - it->it_index; | ||
| Py_ssize_t index = LOAD_SSIZE(it->it_index); | ||
| if (index >= 0) { | ||
| Py_ssize_t len = PyList_GET_SIZE(it->it_seq) - index; | ||
| if (len >= 0) | ||
| return PyLong_FromSsize_t(len); | ||
| } | ||
| @@ -3430,8 +3440,8 @@ listiter_setstate(PyObject *self, PyObject *state) | ||
| if (index == -1 && PyErr_Occurred()) | ||
| return NULL; | ||
| if (it->it_seq != NULL) { | ||
| if (index < 0) | ||
| index = 0; | ||
| if (index < -1) | ||
| index = -1; | ||
| else if (index > PyList_GET_SIZE(it->it_seq)) | ||
| index = PyList_GET_SIZE(it->it_seq); /* iterator exhausted */ | ||
| it->it_index = index; | ||
| @@ -3536,34 +3546,33 @@ static PyObject * | ||
| listreviter_next(PyObject *self) | ||
| { | ||
| listreviterobject *it = (listreviterobject *)self; | ||
| PyObject *item; | ||
| Py_ssize_t index; | ||
| PyListObject *seq; | ||
| assert(it != NULL); | ||
| seq = it->it_seq; | ||
| if (seq == NULL) { | ||
| return NULL; | ||
| } | ||
| PyListObject *seq = it->it_seq; | ||
| assert(PyList_Check(seq)); | ||
| index = it->it_index; | ||
| if (index>=0 && index < PyList_GET_SIZE(seq)) { | ||
| item = PyList_GET_ITEM(seq, index); | ||
| it->it_index--; | ||
| return Py_NewRef(item); | ||
| Py_ssize_t index = LOAD_SSIZE(it->it_index); | ||
| if (index < 0) { | ||
| return NULL; | ||
| } | ||
| PyObject *item = list_get_item_ref(seq, index); | ||
| if (item != NULL) { | ||
| STORE_SSIZE(it->it_index, index - 1); | ||
| return item; | ||
| } | ||
| it->it_index = -1; | ||
| STORE_SSIZE(it->it_index, -1); | ||
colesbury marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| #ifndef Py_GIL_DISABLED | ||
| it->it_seq = NULL; | ||
| Py_DECREF(seq); | ||
| #endif | ||
| return NULL; | ||
| } | ||
| static PyObject * | ||
| listreviter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) | ||
| { | ||
| listreviterobject *it = (listreviterobject *)self; | ||
| Py_ssize_t len = it->it_index + 1; | ||
| Py_ssize_t index = LOAD_SSIZE(it->it_index); | ||
| Py_ssize_t len = index + 1; | ||
| if (it->it_seq == NULL || PyList_GET_SIZE(it->it_seq) < len) | ||
| len = 0; | ||
| return PyLong_FromSsize_t(len); | ||
| @@ -3598,36 +3607,29 @@ static PyObject * | ||
| listiter_reduce_general(void *_it, int forward) | ||
| { | ||
| PyObject *list; | ||
| PyObject *iter; | ||
| /* _PyEval_GetBuiltin can invoke arbitrary code, | ||
| * call must be before access of iterator pointers. | ||
| * see issue #101765 */ | ||
| /* the objects are not the same, index is of different types! */ | ||
| if (forward) { | ||
| PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); | ||
| if (!iter) { | ||
| return NULL; | ||
| } | ||
| iter = _PyEval_GetBuiltin(&_Py_ID(iter)); | ||
| _PyListIterObject *it = (_PyListIterObject *)_it; | ||
| if (it->it_seq) { | ||
| if (it->it_index >= 0) { | ||
| return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); | ||
| } | ||
| Py_DECREF(iter); | ||
| } else { | ||
| PyObject *reversed = _PyEval_GetBuiltin(&_Py_ID(reversed)); | ||
| if (!reversed) { | ||
| return NULL; | ||
| } | ||
| iter = _PyEval_GetBuiltin(&_Py_ID(reversed)); | ||
| listreviterobject *it = (listreviterobject *)_it; | ||
| if (it->it_seq) { | ||
| return Py_BuildValue("N(O)n", reversed, it->it_seq, it->it_index); | ||
| if (it->it_index >= 0) { | ||
| return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); | ||
| } | ||
| Py_DECREF(reversed); | ||
| } | ||
| /* empty iterator, create an empty list */ | ||
| list = PyList_New(0); | ||
| if (list == NULL) | ||
| return NULL; | ||
| return Py_BuildValue("N(N)", _PyEval_GetBuiltin(&_Py_ID(iter)), list); | ||
| return Py_BuildValue("N(N)", iter, list); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the change causing the behavior difference in the test. Previously, an empty iterator would always reduce to the
| ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test was added from #101769, not sure that it will be okay to change the implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is OK, see my other comment.