gh-112087: Make __sizeof__ and listiter_{len, next} to be threadsafe (gh-114843)

diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
index 5b091fb2fd32dc68d094bcc988bcaf19855c8250..1d03ec0f5bd12be0bb10e425c6f7b6fa4124835a 100644 (file)
--- a/Lib/test/support/__init__.py
+++ b/Lib/test/support/__init__.py
@@ -1727,19 +1727,22 @@ def _check_tracemalloc():
 
 
 def check_free_after_iterating(test, iter, cls, args=()):
-    class A(cls):
-        def __del__(self):
-            nonlocal done
-            done = True
-            try:
-                next(it)
-            except StopIteration:
-                pass
-
     done = False
-    it = iter(A(*args))
-    # Issue 26494: Shouldn't crash
-    test.assertRaises(StopIteration, next, it)
+    def wrapper():
+        class A(cls):
+            def __del__(self):
+                nonlocal done
+                done = True
+                try:
+                    next(it)
+                except StopIteration:
+                    pass
+
+        it = iter(A(*args))
+        # Issue 26494: Shouldn't crash
+        test.assertRaises(StopIteration, next, it)
+
+    wrapper()
     # The sequence should be deallocated just after the end of iterating
     gc_collect()
     test.assertTrue(done)

diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py
index 30aedb0db3bb3dbf4e5cf9aba5fd1a05af179764..9606d5beab71cb0a4990c6acb42a17bf3d942b73 100644 (file)
--- a/Lib/test/test_iter.py
+++ b/Lib/test/test_iter.py
@@ -302,7 +302,7 @@ class TestCase(unittest.TestCase):
             # listiter_reduce_general
             self.assertEqual(
                 run("reversed", orig["reversed"](list(range(8)))),
-                (iter, ([],))
+                (reversed, ([],))
             )
 
             for case in types:

diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst
new file mode 100644 (file)
index 0000000..f92cdaf
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst
@@ -0,0 +1,2 @@
+For an empty reverse iterator for list will be reduced to :func:`reversed`.
+Patch by Donghee Na

diff --git a/Objects/listobject.c b/Objects/listobject.c
index 93409a82f8a48976c248db4322db3f4a2aef8e54..96182a42306d95766f46c21500109e759788b4d6 100644 (file)
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -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_freelist *
 get_list_freelist(void)
@@ -2971,7 +2979,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);
 }
 
@@ -3373,33 +3382,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);
     }
@@ -3420,8 +3430,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;
@@ -3526,26 +3536,24 @@ 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);
+#ifndef Py_GIL_DISABLED
     it->it_seq = NULL;
     Py_DECREF(seq);
+#endif
     return NULL;
 }
 
@@ -3553,7 +3561,8 @@ 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);
@@ -3588,6 +3597,7 @@ 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.
@@ -3595,29 +3605,21 @@ listiter_reduce_general(void *_it, int forward)
 
     /* 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);
 }

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 96b97ca4be6d939d1015d1ab602d368d9a2470ff..28ade64e056ad7041df7f3832733e14a060ed8ad 100644 (file)
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2606,11 +2606,14 @@ dummy_func(
             assert(Py_TYPE(iter) == &PyListIter_Type);
             STAT_INC(FOR_ITER, hit);
             PyListObject *seq = it->it_seq;
-            if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) {
+            if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) {
+                it->it_index = -1;
+                #ifndef Py_GIL_DISABLED
                 if (seq != NULL) {
                     it->it_seq = NULL;
                     Py_DECREF(seq);
                 }
+                #endif
                 Py_DECREF(iter);
                 STACK_SHRINK(1);
                 /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */
@@ -2624,8 +2627,7 @@ dummy_func(
             _PyListIterObject *it = (_PyListIterObject *)iter;
             assert(Py_TYPE(iter) == &PyListIter_Type);
             PyListObject *seq = it->it_seq;
-            DEOPT_IF(seq == NULL);
-            DEOPT_IF(it->it_index >= PyList_GET_SIZE(seq));
+            DEOPT_IF((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq));
         }
 
         op(_ITER_NEXT_LIST, (iter -- iter, next)) {

diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index 58d238320276f4bbb5c6b38a3bb1cc288a8523dd..7a0e0e43be019c59d41e0dd14bee2a6a61295dd0 100644 (file)
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -2201,8 +2201,7 @@
             _PyListIterObject *it = (_PyListIterObject *)iter;
             assert(Py_TYPE(iter) == &PyListIter_Type);
             PyListObject *seq = it->it_seq;
-            if (seq == NULL) goto deoptimize;
-            if (it->it_index >= PyList_GET_SIZE(seq)) goto deoptimize;
+            if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) goto deoptimize;
             break;
         }
 

diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index a49223e4db531865279be126c92bf1d18f870700..177bc327454f632dffb4a688d27c859d23016c79 100644 (file)
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -2541,11 +2541,14 @@
                 assert(Py_TYPE(iter) == &PyListIter_Type);
                 STAT_INC(FOR_ITER, hit);
                 PyListObject *seq = it->it_seq;
-                if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) {
+                if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) {
+                    it->it_index = -1;
+                    #ifndef Py_GIL_DISABLED
                     if (seq != NULL) {
                         it->it_seq = NULL;
                         Py_DECREF(seq);
                     }
+                    #endif
                     Py_DECREF(iter);
                     STACK_SHRINK(1);
                     /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */