From d7cbe1695b607b8a5bd06dacb46af9268033252e Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 13 Oct 2024 23:21:30 +0200 Subject: [PATCH 1/3] Make pairwise_next safe for free-threading --- Modules/itertoolsmodule.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 1201fa094902d7..e74bc9ce600222 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -327,37 +327,43 @@ pairwise_traverse(pairwiseobject *po, visitproc visit, void *arg) static PyObject * pairwise_next(pairwiseobject *po) { - PyObject *it = po->it; - PyObject *old = po->old; - PyObject *new, *result; - + PyObject *it = Py_XNewRef(po->it); if (it == NULL) { return NULL; } + + PyObject *old = Py_XNewRef(po->old); + PyObject *new, *result; + if (old == NULL) { old = (*Py_TYPE(it)->tp_iternext)(it); - Py_XSETREF(po->old, old); if (old == NULL) { Py_CLEAR(po->it); + Py_DECREF(it); return NULL; } - it = po->it; - if (it == NULL) { + Py_XSETREF(po->old, Py_NewRef(old)); + if (po->it == NULL) { + // for re-entrant calls to pairwise next. the actual behavior is not important and this does not avoid any bugs (or does it?) + // the reason for having it is to make the behaviour equal to the python implementation behavior Py_CLEAR(po->old); + Py_DECREF(old); + Py_DECREF(it); return NULL; } } - Py_INCREF(old); + new = (*Py_TYPE(it)->tp_iternext)(it); if (new == NULL) { Py_CLEAR(po->it); Py_CLEAR(po->old); + Py_DECREF(it); Py_DECREF(old); return NULL; } result = po->result; - if (Py_REFCNT(result) == 1) { + if (_PyObject_IsUniquelyReferenced(result)) { Py_INCREF(result); PyObject *last_old = PyTuple_GET_ITEM(result, 0); PyObject *last_new = PyTuple_GET_ITEM(result, 1); @@ -380,7 +386,8 @@ pairwise_next(pairwiseobject *po) } Py_XSETREF(po->old, new); - Py_DECREF(old); + Py_DECREF(old); // instead of the decref here we could borrow the reference above + Py_DECREF(it); return result; } From c7ffc4f7b3c064ded698a5852445be289c75ecaa Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 14 Oct 2024 10:45:58 +0200 Subject: [PATCH 2/3] refactor --- .../test_free_threading/test_itertools.py | 42 +++++++++++++++++++ Modules/itertoolsmodule.c | 17 +++++++- 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 Lib/test/test_free_threading/test_itertools.py diff --git a/Lib/test/test_free_threading/test_itertools.py b/Lib/test/test_free_threading/test_itertools.py new file mode 100644 index 00000000000000..4eae063eb517ec --- /dev/null +++ b/Lib/test/test_free_threading/test_itertools.py @@ -0,0 +1,42 @@ +import unittest +from threading import Thread + +from test.support import threading_helper + +from itertools import zip_longest + +class PairwiseThreading(unittest.TestCase): + @staticmethod + def work(enum): + while True: + try: + next(enum) + except StopIteration: + break + + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_zip_longest(self): + number_of_threads = 8 + number_of_iterations = 40 + n = 200 + enum = zip_longest(range(n), range(2*n)) + for _ in range(number_of_iterations): + worker_threads = [] + for ii in range(number_of_threads): + worker_threads.append( + Thread( + target=self.work, + args=[ + enum, + ], + ) + ) + for t in worker_threads: + t.start() + for t in worker_threads: + t.join() + + +if __name__ == "__main__": + unittest.main() diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index e74bc9ce600222..6f1fd87e8f12e2 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -327,7 +327,11 @@ pairwise_traverse(pairwiseobject *po, visitproc visit, void *arg) static PyObject * pairwise_next(pairwiseobject *po) { +#ifdef Py_GIL_DISABLED PyObject *it = Py_XNewRef(po->it); +#else + PyObject *it = po->it; +#endif if (it == NULL) { return NULL; } @@ -339,16 +343,21 @@ pairwise_next(pairwiseobject *po) old = (*Py_TYPE(it)->tp_iternext)(it); if (old == NULL) { Py_CLEAR(po->it); +#ifdef Py_GIL_DISABLED Py_DECREF(it); +#endif return NULL; } Py_XSETREF(po->old, Py_NewRef(old)); if (po->it == NULL) { - // for re-entrant calls to pairwise next. the actual behavior is not important and this does not avoid any bugs (or does it?) - // the reason for having it is to make the behaviour equal to the python implementation behavior + // gh-109786: special case for re-entrant calls to pairwise next. the actual behavior is not + // important and this does not avoid any bugs (or does it?) + // the reason for having it is to make the behaviour equal to the python implementation Py_CLEAR(po->old); Py_DECREF(old); +#ifdef Py_GIL_DISABLED Py_DECREF(it); +#endif return NULL; } } @@ -357,7 +366,9 @@ pairwise_next(pairwiseobject *po) if (new == NULL) { Py_CLEAR(po->it); Py_CLEAR(po->old); +#ifdef Py_GIL_DISABLED Py_DECREF(it); +#endif Py_DECREF(old); return NULL; } @@ -387,7 +398,9 @@ pairwise_next(pairwiseobject *po) Py_XSETREF(po->old, new); Py_DECREF(old); // instead of the decref here we could borrow the reference above +#ifdef Py_GIL_DISABLED Py_DECREF(it); +#endif return result; } From fdc7b01403e34cb61d44134e76a0a6a07ce02be9 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 08:57:18 +0000 Subject: [PATCH 3/3] =?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 --- .../2024-10-14-08-57-14.gh-issue-123471.p0UQBR.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-08-57-14.gh-issue-123471.p0UQBR.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-08-57-14.gh-issue-123471.p0UQBR.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-08-57-14.gh-issue-123471.p0UQBR.rst new file mode 100644 index 00000000000000..a6c66e04d47516 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-14-08-57-14.gh-issue-123471.p0UQBR.rst @@ -0,0 +1 @@ +Make concurrent iterations over the same :func:`itertools.pairwise` iterator safe under free-threading.