From 9e5cb51e2f436a59a9551e62b42ed3753bd07602 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 13 Mar 2025 21:17:42 +0100 Subject: [PATCH 1/5] remove dead code related to firstpass --- Modules/itertoolsmodule.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 9c9a965ce74feb..0ceab8ef6b9b84 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -1126,7 +1126,6 @@ typedef struct { PyObject *it; PyObject *saved; Py_ssize_t index; - int firstpass; } cycleobject; #define cycleobject_CAST(op) ((cycleobject *)(op)) @@ -1168,7 +1167,6 @@ itertools_cycle_impl(PyTypeObject *type, PyObject *iterable) lz->it = it; lz->saved = saved; lz->index = 0; - lz->firstpass = 0; return (PyObject *)lz; } @@ -1204,8 +1202,6 @@ cycle_next(PyObject *op) if (lz->it != NULL) { item = PyIter_Next(lz->it); if (item != NULL) { - if (lz->firstpass) - return item; if (PyList_Append(lz->saved, item)) { Py_DECREF(item); return NULL; From 62bf9c2a8b7cf230cfc5c23d823267224488c7de Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 13 Mar 2025 21:36:43 +0100 Subject: [PATCH 2/5] make cycle_next thread safe --- ...itertools_batched.py => test_itertools.py} | 32 +++++++++++++++++-- Modules/itertoolsmodule.c | 19 +++++++---- 2 files changed, 42 insertions(+), 9 deletions(-) rename Lib/test/test_free_threading/{test_itertools_batched.py => test_itertools.py} (53%) diff --git a/Lib/test/test_free_threading/test_itertools_batched.py b/Lib/test/test_free_threading/test_itertools.py similarity index 53% rename from Lib/test/test_free_threading/test_itertools_batched.py rename to Lib/test/test_free_threading/test_itertools.py index a754b4f9ea9902..8360afbf78cadd 100644 --- a/Lib/test/test_free_threading/test_itertools_batched.py +++ b/Lib/test/test_free_threading/test_itertools.py @@ -1,15 +1,15 @@ import unittest from threading import Thread, Barrier -from itertools import batched +from itertools import batched, cycle from test.support import threading_helper threading_helper.requires_working_threading(module=True) -class EnumerateThreading(unittest.TestCase): +class ItertoolsThreading(unittest.TestCase): @threading_helper.reap_threads - def test_threading(self): + def test_batched(self): number_of_threads = 10 number_of_iterations = 20 barrier = Barrier(number_of_threads) @@ -34,5 +34,31 @@ def work(it): barrier.reset() + @threading_helper.reap_threads + def test_cycle(self): + number_of_threads = 6 + number_of_iterations = 10 + number_of_cycles = 400 + + barrier = Barrier(number_of_threads) + def work(it): + barrier.wait() + for _ in range(number_of_cycles): + _ = next(it) + + data = (1, 2, 3, 4) + for it in range(number_of_iterations): + cycle_iterator = cycle(data) + worker_threads = [] + for ii in range(number_of_threads): + worker_threads.append( + Thread(target=work, args=[cycle_iterator])) + + with threading_helper.start_threads(worker_threads): + pass + + barrier.reset() + + if __name__ == "__main__": unittest.main() diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 0ceab8ef6b9b84..3871e023427719 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -1166,7 +1166,7 @@ itertools_cycle_impl(PyTypeObject *type, PyObject *iterable) } lz->it = it; lz->saved = saved; - lz->index = 0; + lz->index = -1; return (PyObject *)lz; } @@ -1199,7 +1199,9 @@ cycle_next(PyObject *op) cycleobject *lz = cycleobject_CAST(op); PyObject *item; - if (lz->it != NULL) { + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(lz->index); + + if (index < 0) { item = PyIter_Next(lz->it); if (item != NULL) { if (PyList_Append(lz->saved, item)) { @@ -1211,14 +1213,19 @@ cycle_next(PyObject *op) /* Note: StopIteration is already cleared by PyIter_Next() */ if (PyErr_Occurred()) return NULL; + index = 0; + FT_ATOMIC_STORE_SSIZE_RELAXED(lz->index, 0); +#ifndef Py_GIL_DISABLED Py_CLEAR(lz->it); +#endif } if (PyList_GET_SIZE(lz->saved) == 0) return NULL; - item = PyList_GET_ITEM(lz->saved, lz->index); - lz->index++; - if (lz->index >= PyList_GET_SIZE(lz->saved)) - lz->index = 0; + item = PyList_GET_ITEM(lz->saved, index); + index++; + if (index >= PyList_GET_SIZE(lz->saved)) { + FT_ATOMIC_STORE_SSIZE_RELAXED(lz->index, 0); + } return Py_NewRef(item); } From c7f6ba05abc8cda2a80489e103d74f2887bcfc4f Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 13 Mar 2025 20:49:04 +0000 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2025-03-13-20-48-58.gh-issue-123471.cM4w4f.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-03-13-20-48-58.gh-issue-123471.cM4w4f.rst diff --git a/Misc/NEWS.d/next/Library/2025-03-13-20-48-58.gh-issue-123471.cM4w4f.rst b/Misc/NEWS.d/next/Library/2025-03-13-20-48-58.gh-issue-123471.cM4w4f.rst new file mode 100644 index 00000000000000..cfc783900de70f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-03-13-20-48-58.gh-issue-123471.cM4w4f.rst @@ -0,0 +1 @@ +Make concurrent iterations over :class:`itertools.cycle` safe under free-threading. From dea1835aa472551c20c874d07bfc08c757646216 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 13 Mar 2025 22:00:35 +0100 Subject: [PATCH 4/5] fix index update --- Modules/itertoolsmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 3871e023427719..70188b698a96e8 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -1224,8 +1224,9 @@ cycle_next(PyObject *op) item = PyList_GET_ITEM(lz->saved, index); index++; if (index >= PyList_GET_SIZE(lz->saved)) { - FT_ATOMIC_STORE_SSIZE_RELAXED(lz->index, 0); + index = 0; } + FT_ATOMIC_STORE_SSIZE_RELAXED(lz->index, index); return Py_NewRef(item); } From ab442da8ecac1dc16812c1f1425d5dfce36bcf24 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 28 May 2025 22:04:41 +0200 Subject: [PATCH 5/5] use PyList_GetItemRef --- Modules/itertoolsmodule.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 7cfe5b468fd963..2003546ce84cef 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -1219,13 +1219,14 @@ cycle_next(PyObject *op) } if (PyList_GET_SIZE(lz->saved) == 0) return NULL; - item = PyList_GET_ITEM(lz->saved, index); + item = PyList_GetItemRef(lz->saved, index); + assert(item); index++; if (index >= PyList_GET_SIZE(lz->saved)) { index = 0; } FT_ATOMIC_STORE_SSIZE_RELAXED(lz->index, index); - return Py_NewRef(item); + return item; } static PyType_Slot cycle_slots[] = {