Issue #26494: Fixed crash on iterating exhausting iterators.

Affected classes are generic sequence iterators, iterators of str, bytes,
bytearray, list, tuple, set, frozenset, dict, OrderedDict, corresponding
views and os.scandir() iterator.

diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py
index 24162494dd6123001081c01b2aa5572a022896ef..72f4845a973e94b91986d675c09bf27633a8cbeb 100644 (file)
--- a/Lib/test/seq_tests.py
+++ b/Lib/test/seq_tests.py
@@ -5,6 +5,7 @@ Tests common to tuple, list and UserList.UserList
 import unittest
 import sys
 import pickle
+from test import support
 
 # Various iterables
 # This is used for checking the constructor (here and in test_deque.py)
@@ -408,3 +409,7 @@ class CommonTest(unittest.TestCase):
             lst2 = pickle.loads(pickle.dumps(lst, proto))
             self.assertEqual(lst2, lst)
             self.assertNotEqual(id(lst2), id(lst))
+
+    def test_free_after_iterating(self):
+        support.check_free_after_iterating(self, iter, self.type2test)
+        support.check_free_after_iterating(self, reversed, self.type2test)

diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
index b82f9cb996b742a1efd3f62e3cb9bcc0a5452e0a..e124fab610cb3697c78f9bbafe00a6a41c95768d 100644 (file)
--- a/Lib/test/support/__init__.py
+++ b/Lib/test/support/__init__.py
@@ -2366,3 +2366,22 @@ def run_in_subinterp(code):
                                      "memory allocations")
     import _testcapi
     return _testcapi.run_in_subinterp(code)
+
+
+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)
+    # The sequence should be deallocated just after the end of iterating
+    gc_collect()
+    test.assertTrue(done)

diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py
index 80798f24373309671e43339b88537e771dff1c1d..1bd3a1ed9c4041e676fb0eb2fd57a03bcac3dcb2 100644 (file)
--- a/Lib/test/test_bytes.py
+++ b/Lib/test/test_bytes.py
@@ -747,6 +747,10 @@ class BaseBytesTest:
         self.assertRaisesRegex(TypeError, r'\bendswith\b', b.endswith,
                                 x, None, None, None)
 
+    def test_free_after_iterating(self):
+        test.support.check_free_after_iterating(self, iter, self.type2test)
+        test.support.check_free_after_iterating(self, reversed, self.type2test)
+
 
 class BytesTest(BaseBytesTest, unittest.TestCase):
     type2test = bytes

diff --git a/Lib/test/test_deque.py b/Lib/test/test_deque.py
index ec2be83d4254968e13d25d69a2b10f829de04d24..7041d1725e63aac829049bc3fe77630e87be204a 100644 (file)
--- a/Lib/test/test_deque.py
+++ b/Lib/test/test_deque.py
@@ -905,6 +905,10 @@ class TestSequence(seq_tests.CommonTest):
         # For now, bypass tests that require slicing
         pass
 
+    def test_free_after_iterating(self):
+        # For now, bypass tests that require slicing
+        self.skipTest("Exhausted deque iterator doesn't free a deque")
+
 #==============================================================================
 
 libreftest = """

diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index 3b4241422596ea2fc49095f17b6b2d427ae4a53f..075cb5a020443a3b0088331d5569d8fa8d097df3 100644 (file)
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -952,6 +952,12 @@ class DictTest(unittest.TestCase):
         d = {X(): 0, 1: 1}
         self.assertRaises(RuntimeError, d.update, other)
 
+    def test_free_after_iterating(self):
+        support.check_free_after_iterating(self, iter, dict)
+        support.check_free_after_iterating(self, lambda d: iter(d.keys()), dict)
+        support.check_free_after_iterating(self, lambda d: iter(d.values()), dict)
+        support.check_free_after_iterating(self, lambda d: iter(d.items()), dict)
+
 from test import mapping_tests
 
 class GeneralMappingTests(mapping_tests.BasicTestMappingProtocol):

diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py
index 56e21f8aa70064d554c3593c4c8a950c992b5204..54ddbaa5fdf49aa9445b5202ae1d656229228b64 100644 (file)
--- a/Lib/test/test_iter.py
+++ b/Lib/test/test_iter.py
@@ -3,6 +3,7 @@
 import sys
 import unittest
 from test.support import run_unittest, TESTFN, unlink, cpython_only
+from test.support import check_free_after_iterating
 import pickle
 import collections.abc
 
@@ -980,6 +981,9 @@ class TestCase(unittest.TestCase):
         self.assertEqual(next(it), 0)
         self.assertEqual(next(it), 1)
 
+    def test_free_after_iterating(self):
+        check_free_after_iterating(self, iter, SequenceClass, (0,))
+
 
 def test_main():
     run_unittest(TestCase)

diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py
index 8ab0a9fbd16d2991c2a1555dc406341943e3e77a..901d4b2ad29478f9fb35c702cc777b3132b9f26b 100644 (file)
--- a/Lib/test/test_ordered_dict.py
+++ b/Lib/test/test_ordered_dict.py
@@ -598,6 +598,12 @@ class OrderedDictTests:
         gc.collect()
         self.assertIsNone(r())
 
+    def test_free_after_iterating(self):
+        support.check_free_after_iterating(self, iter, self.OrderedDict)
+        support.check_free_after_iterating(self, lambda d: iter(d.keys()), self.OrderedDict)
+        support.check_free_after_iterating(self, lambda d: iter(d.values()), self.OrderedDict)
+        support.check_free_after_iterating(self, lambda d: iter(d.items()), self.OrderedDict)
+
 
 class PurePythonOrderedDictTests(OrderedDictTests, unittest.TestCase):
 

diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py
index 54de508a832de4f6c7fb7be3df0b91c8470a92bc..0b99dfc639da5d46926eae212b79291367198788 100644 (file)
--- a/Lib/test/test_set.py
+++ b/Lib/test/test_set.py
@@ -362,6 +362,9 @@ class TestJointOps:
         gc.collect()
         self.assertTrue(ref() is None, "Cycle was not collected")
 
+    def test_free_after_iterating(self):
+        support.check_free_after_iterating(self, iter, self.thetype)
+
 class TestSet(TestJointOps, unittest.TestCase):
     thetype = set
     basetype = set

diff --git a/Lib/test/test_unicode.py b/Lib/test/test_unicode.py
index c30310e1ae827489ef73e5102ebac40c05b59792..c2811468bae13e54eef8e02ccf995ecb0bc38357 100644 (file)
--- a/Lib/test/test_unicode.py
+++ b/Lib/test/test_unicode.py
@@ -2729,6 +2729,10 @@ class UnicodeTest(string_tests.CommonTest,
                 # Check that the second call returns the same result
                 self.assertEqual(getargs_s_hash(s), chr(k).encode() * (i + 1))
 
+    def test_free_after_iterating(self):
+        support.check_free_after_iterating(self, iter, str)
+        support.check_free_after_iterating(self, reversed, str)
+
 
 class StringModuleTest(unittest.TestCase):
     def test_formatter_parser(self):

diff --git a/Misc/NEWS b/Misc/NEWS
index 59c0828d5fe53cb62080260091657e65c0031010..391bbf256e62a1e2543a42089d59cee7318765d8 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,11 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #26494: Fixed crash on iterating exhausting iterators.
+  Affected classes are generic sequence iterators, iterators of str, bytes,
+  bytearray, list, tuple, set, frozenset, dict, OrderedDict, corresponding
+  views and os.scandir() iterator.
+
 - Issue #26581: If coding cookie is specified multiple times on a line in
   Python source code file, only the first one is taken to account.
 

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 710bcde812934cfb4baded9e2dd272af0283cf80..c95668b3fda66b6ecc311edb23fde9a3b85baa47 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -11928,13 +11928,15 @@ typedef struct {
 static void
 ScandirIterator_close(ScandirIterator *iterator)
 {
-    if (iterator->handle == INVALID_HANDLE_VALUE)
+    HANDLE handle = iterator->handle;
+
+    if (handle == INVALID_HANDLE_VALUE)
         return;
 
+    iterator->handle = INVALID_HANDLE_VALUE;
     Py_BEGIN_ALLOW_THREADS
-    FindClose(iterator->handle);
+    FindClose(handle);
     Py_END_ALLOW_THREADS
-    iterator->handle = INVALID_HANDLE_VALUE;
 }
 
 static PyObject *
@@ -11984,13 +11986,15 @@ ScandirIterator_iternext(ScandirIterator *iterator)
 static void
 ScandirIterator_close(ScandirIterator *iterator)
 {
-    if (!iterator->dirp)
+    DIR *dirp = iterator->dirp;
+
+    if (!dirp)
         return;
 
+    iterator->dirp = NULL;
     Py_BEGIN_ALLOW_THREADS
-    closedir(iterator->dirp);
+    closedir(dirp);
     Py_END_ALLOW_THREADS
-    iterator->dirp = NULL;
     return;
 }
 

diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c
index c59ad2499adb189e2df76d81138e6d702d42af01..c723a9c9764cbd39d37bbfb9749170e7b3dea5c7 100644 (file)
--- a/Objects/bytearrayobject.c
+++ b/Objects/bytearrayobject.c
@@ -3186,8 +3186,8 @@ bytearrayiter_next(bytesiterobject *it)
         return item;
     }
 
-    Py_DECREF(seq);
     it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }
 

diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
index 51d0871c8b8376a248dd70c21bc81bcbe936846d..495c3ebc23696d508f8c7cff670317296d73529d 100644 (file)
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -3628,8 +3628,8 @@ striter_next(striterobject *it)
         return item;
     }
 
-    Py_DECREF(seq);
     it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }
 

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index e4dff98940d21e5a5682f18d5b9aa332e2a7c58b..d7745860e47a8f0d576d6b931f71c6d4361095c9 100644 (file)
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -2985,8 +2985,8 @@ static PyObject *dictiter_iternextkey(dictiterobject *di)
     return key;
 
 fail:
-    Py_DECREF(d);
     di->di_dict = NULL;
+    Py_DECREF(d);
     return NULL;
 }
 
@@ -3066,8 +3066,8 @@ static PyObject *dictiter_iternextvalue(dictiterobject *di)
     return value;
 
 fail:
-    Py_DECREF(d);
     di->di_dict = NULL;
+    Py_DECREF(d);
     return NULL;
 }
 
@@ -3161,8 +3161,8 @@ static PyObject *dictiter_iternextitem(dictiterobject *di)
     return result;
 
 fail:
-    Py_DECREF(d);
     di->di_dict = NULL;
+    Py_DECREF(d);
     return NULL;
 }
 

diff --git a/Objects/iterobject.c b/Objects/iterobject.c
index 2fb0c8832a970af717f83aa617a7d88f0634a4f2..ab29ff81a9595b26c446d5d75bb72314f5d33bb2 100644 (file)
--- a/Objects/iterobject.c
+++ b/Objects/iterobject.c
@@ -69,8 +69,8 @@ iter_iternext(PyObject *iterator)
         PyErr_ExceptionMatches(PyExc_StopIteration))
     {
         PyErr_Clear();
-        Py_DECREF(seq);
         it->it_seq = NULL;
+        Py_DECREF(seq);
     }
     return NULL;
 }

diff --git a/Objects/listobject.c b/Objects/listobject.c
index eee7c68e9e4562c8da0c63efc80b67fc70ef261f..d688179d6b4d12340f8b6155c3c123409ee0cb7d 100644 (file)
--- a/Objects/listobject.c
+++ b/Objects/listobject.c
@@ -2782,8 +2782,8 @@ listiter_next(listiterobject *it)
         return item;
     }
 
-    Py_DECREF(seq);
     it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }
 
@@ -2912,9 +2912,17 @@ static PyObject *
 listreviter_next(listreviterobject *it)
 {
     PyObject *item;
-    Py_ssize_t index = it->it_index;
-    PyListObject *seq = it->it_seq;
+    Py_ssize_t index;
+    PyListObject *seq;
+
+    assert(it != NULL);
+    seq = it->it_seq;
+    if (seq == NULL) {
+        return NULL;
+    }
+    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--;
@@ -2922,10 +2930,8 @@ listreviter_next(listreviterobject *it)
         return item;
     }
     it->it_index = -1;
-    if (seq != NULL) {
-        it->it_seq = NULL;
-        Py_DECREF(seq);
-    }
+    it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }
 

diff --git a/Objects/setobject.c b/Objects/setobject.c
index 582f280e347e12851e92276ab7875924a4ded642..4ef692db33203129a69a324ed285c89fc533cac8 100644 (file)
--- a/Objects/setobject.c
+++ b/Objects/setobject.c
@@ -839,8 +839,8 @@ static PyObject *setiter_iternext(setiterobject *si)
     return key;
 
 fail:
-    Py_DECREF(so);
     si->si_set = NULL;
+    Py_DECREF(so);
     return NULL;
 }
 

diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c
index 7efa1a6776a00ee3eb0062dd92632030c220bf45..7920fec2bd86e8854f110afe8818e85737a66fc6 100644 (file)
--- a/Objects/tupleobject.c
+++ b/Objects/tupleobject.c
@@ -964,8 +964,8 @@ tupleiter_next(tupleiterobject *it)
         return item;
     }
 
-    Py_DECREF(seq);
     it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }
 

diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c
index adc46156036125d0a887fbb63842261c4eafb2f0..230125b62ba2657883c78feed206b43b59dc9e29 100644 (file)
--- a/Objects/unicodeobject.c
+++ b/Objects/unicodeobject.c
@@ -15149,8 +15149,8 @@ unicodeiter_next(unicodeiterobject *it)
         return item;
     }
 
-    Py_DECREF(seq);
     it->it_seq = NULL;
+    Py_DECREF(seq);
     return NULL;
 }