From 9b65a0609991468aa1472c77305dc73035cf0c3f Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 09:45:47 -0700 Subject: [PATCH 1/4] Fix the use of recursion in itertools.chain.from_iterable. Using recursion is unnecessary, and can easily cause stack overflows, especially when building in low optimization modes or with Py_DEBUG enabled. --- Lib/test/test_itertools.py | 8 ++++++ Modules/itertoolsmodule.c | 52 ++++++++++++++++++++------------------ 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/Lib/test/test_itertools.py b/Lib/test/test_itertools.py index ea1f57caade12b9..b78290447b5141d 100644 --- a/Lib/test/test_itertools.py +++ b/Lib/test/test_itertools.py @@ -1976,6 +1976,14 @@ def gen2(x): self.assertRaises(AssertionError, list, cycle(gen1())) self.assertEqual(hist, [0,1]) + def test_deep_recursion(self): + # Make sure itertools.chain doesn't run into recursion limits when + # dealing with long chains of empty iterables. Even with a high + # number this would probably only fail in Py_DEBUG mode. + it = chain.from_iterable(() for unused in range(10000000)) + with self.assertRaises(StopIteration): + next(it) + class SubclassWithKwargsTest(unittest.TestCase): def test_keywords_in_subclass(self): # count is not subclassable... diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index f867252481007cd..dbbfdec4a246c44 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -1864,33 +1864,37 @@ chain_next(chainobject *lz) { PyObject *item; - if (lz->source == NULL) - return NULL; /* already stopped */ - - if (lz->active == NULL) { - PyObject *iterable = PyIter_Next(lz->source); - if (iterable == NULL) { - Py_CLEAR(lz->source); - return NULL; /* no more input sources */ - } - lz->active = PyObject_GetIter(iterable); - Py_DECREF(iterable); + /* lz->source is the iterator of iterables. If it's NULL, we've already + * consumed them all. lz->active is the current iterator. If it's NULL, + * we should grab a new one from lz->source. */ + while (lz->source != NULL) { if (lz->active == NULL) { - Py_CLEAR(lz->source); - return NULL; /* input not iterable */ + PyObject *iterable = PyIter_Next(lz->source); + if (iterable == NULL) { + Py_CLEAR(lz->source); + return NULL; /* no more input sources */ + } + lz->active = PyObject_GetIter(iterable); + Py_DECREF(iterable); + if (lz->active == NULL) { + Py_CLEAR(lz->source); + return NULL; /* input not iterable */ + } } + item = (*Py_TYPE(lz->active)->tp_iternext)(lz->active); + if (item != NULL) + return item; + if (PyErr_Occurred()) { + if (PyErr_ExceptionMatches(PyExc_StopIteration)) + PyErr_Clear(); + else + return NULL; /* input raised an exception */ + } + /* lz->active is consumed, try with the next iterable. */ + Py_CLEAR(lz->active); } - item = (*Py_TYPE(lz->active)->tp_iternext)(lz->active); - if (item != NULL) - return item; - if (PyErr_Occurred()) { - if (PyErr_ExceptionMatches(PyExc_StopIteration)) - PyErr_Clear(); - else - return NULL; /* input raised an exception */ - } - Py_CLEAR(lz->active); - return chain_next(lz); /* recurse and use next active */ + /* Everything had been consumed already. */ + return NULL; } static PyObject * From d0add8737277b23f611e9c6d4a8afd501dbc2b29 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 10:27:06 -0700 Subject: [PATCH 2/4] Add NEWS entry for itertools recursion fix. --- Misc/NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index c46b6652d1a941e..490e7c196920af6 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -291,6 +291,9 @@ Extension Modules Library ------- +- bpo-29942: Fix a crash in itertools.chain.from_iterable when encountering + long runs of empty iterables. + - bpo-28699: Fixed a bug in pools in multiprocessing.pool that raising an exception at the very first of an iterable may swallow the exception or make the program hang. Patch by Davin Potts and Xiang Zhang. From f9d880b55e72dc80f22b3a7d6b32f6a23c44c363 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 10:59:11 -0700 Subject: [PATCH 3/4] Fix indentation depth. --- Lib/test/test_itertools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_itertools.py b/Lib/test/test_itertools.py index b78290447b5141d..f91acb04dd573a8 100644 --- a/Lib/test/test_itertools.py +++ b/Lib/test/test_itertools.py @@ -1982,7 +1982,7 @@ def test_deep_recursion(self): # number this would probably only fail in Py_DEBUG mode. it = chain.from_iterable(() for unused in range(10000000)) with self.assertRaises(StopIteration): - next(it) + next(it) class SubclassWithKwargsTest(unittest.TestCase): def test_keywords_in_subclass(self): From fd90aeadedea16ab572ff42c174d6991da67c009 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 29 Mar 2017 10:59:49 -0700 Subject: [PATCH 4/4] Make the new test's name reflect the actual problem. --- Lib/test/test_itertools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_itertools.py b/Lib/test/test_itertools.py index f91acb04dd573a8..c431f0dc6e1d7f4 100644 --- a/Lib/test/test_itertools.py +++ b/Lib/test/test_itertools.py @@ -1976,7 +1976,7 @@ def gen2(x): self.assertRaises(AssertionError, list, cycle(gen1())) self.assertEqual(hist, [0,1]) - def test_deep_recursion(self): + def test_long_chain_of_empty_iterables(self): # Make sure itertools.chain doesn't run into recursion limits when # dealing with long chains of empty iterables. Even with a high # number this would probably only fail in Py_DEBUG mode.