From 57bc2dc2ead058c71b8a394e3bc9ecee81bb015d Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 24 Jun 2024 22:02:10 +0200 Subject: [PATCH 01/23] Make reversed iterator thread-safe --- Objects/enumobject.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 556666779d8522..dab74ec0b769b0 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -429,20 +429,21 @@ static PyObject * reversed_next(reversedobject *ro) { PyObject *item; - Py_ssize_t index = ro->index; + Py_ssize_t index = _Py_atomic_add_ssize(&(ro->index), -1); if (index >= 0) { item = PySequence_GetItem(ro->seq, index); if (item != NULL) { - ro->index--; return item; } if (PyErr_ExceptionMatches(PyExc_IndexError) || PyErr_ExceptionMatches(PyExc_StopIteration)) PyErr_Clear(); } - ro->index = -1; + FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, -1); +#ifndef Py_GIL_DISABLED Py_CLEAR(ro->seq); +#endif return NULL; } From 7127a856b5f88e1773e89ee09079f9a4c9091f14 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 24 Jun 2024 22:04:50 +0200 Subject: [PATCH 02/23] Add test for free-threading --- Lib/test/test_reversed.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Lib/test/test_reversed.py diff --git a/Lib/test/test_reversed.py b/Lib/test/test_reversed.py new file mode 100644 index 00000000000000..de37c532784768 --- /dev/null +++ b/Lib/test/test_reversed.py @@ -0,0 +1,34 @@ +import unittest +from threading import Thread +from test.support import threading_helper + + +@threading_helper.reap_threads +@threading_helper.requires_working_threading() +def test_reversed_threading(self): + # Test that when reading out with multiple treads no identical elemenents are returned + def work(r, output): + while True: + try: + value = next(r) + except StopIteration: + break + else: + output.append(value) + + number_of_threads = 10 + x = tuple(range(4_000)) + r = reversed(x) + worker_threads = [] + output = [] + for ii in range(number_of_threads): + worker_threads.append(Thread(target=work, args=[r, output])) + _ = [t.start() for t in worker_threads] + _ = [t.join() for t in worker_threads] + + if sorted(output) != x: + raise ValueError('reversed returned value from sequence twice') + + +if __name__ == "__main__": + unittest.main() From 687a976a905b494f354c2d8493b62874dabc5964 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:09:00 +0000 Subject: [PATCH 03/23] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst new file mode 100644 index 00000000000000..76e018eeefbcfa --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst @@ -0,0 +1 @@ +Make :func:`reversed` thread-safe. From a74a33cc2541ad552d5ad6e3fa0f3bd4cea8ff2c Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 25 Jun 2024 21:01:11 +0200 Subject: [PATCH 04/23] Apply suggestions from code review Co-authored-by: Sam Gross --- Objects/enumobject.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Objects/enumobject.c b/Objects/enumobject.c index dab74ec0b769b0..50061e3dda699f 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -429,11 +429,12 @@ static PyObject * reversed_next(reversedobject *ro) { PyObject *item; - Py_ssize_t index = _Py_atomic_add_ssize(&(ro->index), -1); + Py_ssize_t index = _Py_atomic_load_ssize_relaxed(&ro->index); if (index >= 0) { item = PySequence_GetItem(ro->seq, index); if (item != NULL) { + _Py_atomic_store_ssize_relaxed(&ro->index, index - 1); return item; } if (PyErr_ExceptionMatches(PyExc_IndexError) || From de1b3c6aa629ed6abd6347a2b772304c57b53ff1 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 25 Jun 2024 21:21:25 +0200 Subject: [PATCH 05/23] make reversed_len thread safe --- Lib/test/test_reversed.py | 8 +++----- Objects/enumobject.c | 10 ++++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_reversed.py b/Lib/test/test_reversed.py index de37c532784768..04a64addb6b3cd 100644 --- a/Lib/test/test_reversed.py +++ b/Lib/test/test_reversed.py @@ -6,7 +6,9 @@ @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_reversed_threading(self): - # Test that when reading out with multiple treads no identical elemenents are returned + # Test reading out the iterator with multiple threads cannot corrupt + # the reversed iterator. + # The reversed iterator is not guaranteed to be thread safe def work(r, output): while True: try: @@ -26,9 +28,5 @@ def work(r, output): _ = [t.start() for t in worker_threads] _ = [t.join() for t in worker_threads] - if sorted(output) != x: - raise ValueError('reversed returned value from sequence twice') - - if __name__ == "__main__": unittest.main() diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 50061e3dda699f..679f282f9cf2e3 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -429,12 +429,12 @@ static PyObject * reversed_next(reversedobject *ro) { PyObject *item; - Py_ssize_t index = _Py_atomic_load_ssize_relaxed(&ro->index); + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index); if (index >= 0) { item = PySequence_GetItem(ro->seq, index); if (item != NULL) { - _Py_atomic_store_ssize_relaxed(&ro->index, index - 1); + FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index - 1); return item; } if (PyErr_ExceptionMatches(PyExc_IndexError) || @@ -452,13 +452,15 @@ static PyObject * reversed_len(reversedobject *ro, PyObject *Py_UNUSED(ignored)) { Py_ssize_t position, seqsize; + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index); - if (ro->seq == NULL) + if (index == -1) return PyLong_FromLong(0); + assert(ro->seq != 0); seqsize = PySequence_Size(ro->seq); if (seqsize == -1) return NULL; - position = ro->index + 1; + position = index + 1; return PyLong_FromSsize_t((seqsize < position) ? 0 : position); } From 3bef3fde85c1dbb905b1c6c0e3b39484c8f93cb1 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 25 Jun 2024 21:48:52 +0200 Subject: [PATCH 06/23] update news entry, make reversed_reduce safe --- .../2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst | 3 ++- Objects/enumobject.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst index 76e018eeefbcfa..c1887bd78b5b76 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst @@ -1 +1,2 @@ -Make :func:`reversed` thread-safe. +Adapt :func:`reversed` for usage in the free-theading build. +The :func:`reversed` is not guaranteed to be thread-safe. diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 679f282f9cf2e3..92640832817ccc 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -469,7 +469,8 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * reversed_reduce(reversedobject *ro, PyObject *Py_UNUSED(ignored)) { - if (ro->seq) + Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index); + if (index != -1) return Py_BuildValue("O(O)n", Py_TYPE(ro), ro->seq, ro->index); else return Py_BuildValue("O(())", Py_TYPE(ro)); From 220b41464b6012df47e374adacef139d0c852016 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 26 Jun 2024 11:40:26 +0200 Subject: [PATCH 07/23] update reversed_setstate for free-threading --- Objects/enumobject.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 92640832817ccc..8c460c07aa7314 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -482,7 +482,11 @@ reversed_setstate(reversedobject *ro, PyObject *state) Py_ssize_t index = PyLong_AsSsize_t(state); if (index == -1 && PyErr_Occurred()) return NULL; - if (ro->seq != 0) { + Py_ssize_t ro_index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index); + // if the iterator is exhausted we do not set the state + // this is for backwards compatibility reasons. in practice this situation + // will not occur, see gh-120971 + if (ro_index != -1) { Py_ssize_t n = PySequence_Size(ro->seq); if (n < 0) return NULL; @@ -490,7 +494,7 @@ reversed_setstate(reversedobject *ro, PyObject *state) index = -1; else if (index > n-1) index = n-1; - ro->index = index; + FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index); } Py_RETURN_NONE; } From 28aa548509371f2c16387e0a4fccaf60d6106784 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 26 Jun 2024 11:54:10 +0200 Subject: [PATCH 08/23] Update Objects/enumobject.c Co-authored-by: Sam Gross --- Objects/enumobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/enumobject.c b/Objects/enumobject.c index 8c460c07aa7314..b982fc84335c00 100644 --- a/Objects/enumobject.c +++ b/Objects/enumobject.c @@ -456,7 +456,7 @@ reversed_len(reversedobject *ro, PyObject *Py_UNUSED(ignored)) if (index == -1) return PyLong_FromLong(0); - assert(ro->seq != 0); + assert(ro->seq != NULL); seqsize = PySequence_Size(ro->seq); if (seqsize == -1) return NULL; From e052ca46cd15b6aca8f1bbf97788d0998663ee24 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 26 Jun 2024 12:10:00 +0200 Subject: [PATCH 09/23] simplify test --- Lib/test/test_reversed.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_reversed.py b/Lib/test/test_reversed.py index 04a64addb6b3cd..4b9190f1f631d8 100644 --- a/Lib/test/test_reversed.py +++ b/Lib/test/test_reversed.py @@ -9,22 +9,22 @@ def test_reversed_threading(self): # Test reading out the iterator with multiple threads cannot corrupt # the reversed iterator. # The reversed iterator is not guaranteed to be thread safe + + size = 4_000 def work(r, output): while True: try: - value = next(r) + l = r.__length_hint__() + next(r) except StopIteration: break - else: - output.append(value) - + assert 0 <= l <= size number_of_threads = 10 - x = tuple(range(4_000)) + x = tuple(range(size)) r = reversed(x) worker_threads = [] - output = [] for ii in range(number_of_threads): - worker_threads.append(Thread(target=work, args=[r, output])) + worker_threads.append(Thread(target=work, args=[r])) _ = [t.start() for t in worker_threads] _ = [t.join() for t in worker_threads] From 8a7876abd87e3934b476fd685efc8ce26ec45300 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 8 Jul 2024 21:28:35 +0200 Subject: [PATCH 10/23] Update Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst Co-authored-by: Sam Gross --- .../2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst index c1887bd78b5b76..31d1dfd54d7933 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst @@ -1,2 +1,4 @@ -Adapt :func:`reversed` for usage in the free-theading build. -The :func:`reversed` is not guaranteed to be thread-safe. +Adapt :func:`reversed` for use in the free-theading build. +The :func:`reversed` is still not thread-safe in the sense that concurrent +iterations may see the same object, but they will not corrupt the interpreter +state. From 081ba4082458607dfde8a64e113cfc0710f3f269 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 8 Jul 2024 21:33:42 +0200 Subject: [PATCH 11/23] review comments: use for loop instead of comprehension --- Lib/test/test_reversed.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_reversed.py b/Lib/test/test_reversed.py index 4b9190f1f631d8..30a1de64f1d062 100644 --- a/Lib/test/test_reversed.py +++ b/Lib/test/test_reversed.py @@ -7,11 +7,13 @@ @threading_helper.requires_working_threading() def test_reversed_threading(self): # Test reading out the iterator with multiple threads cannot corrupt - # the reversed iterator. + # the reversed iterator state. # The reversed iterator is not guaranteed to be thread safe + number_of_threads = 10 size = 4_000 - def work(r, output): + + def work(r): while True: try: l = r.__length_hint__() @@ -19,14 +21,15 @@ def work(r, output): except StopIteration: break assert 0 <= l <= size - number_of_threads = 10 x = tuple(range(size)) r = reversed(x) worker_threads = [] for ii in range(number_of_threads): worker_threads.append(Thread(target=work, args=[r])) - _ = [t.start() for t in worker_threads] - _ = [t.join() for t in worker_threads] + for t in worker_threads: + t.start() + for t in worker_threads: + t.join() if __name__ == "__main__": unittest.main() From b2f2dd39544fae20ba9dfcf431ee1cd3382768a8 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 8 Jul 2024 22:28:37 +0200 Subject: [PATCH 12/23] move test; skip free_after_iterating test on ft build --- Lib/test/test_free_threading/test_reversed.py | 36 +++++++++++++++++++ Lib/test/test_reversed.py | 35 ------------------ Lib/test/test_str.py | 3 +- 3 files changed, 38 insertions(+), 36 deletions(-) create mode 100644 Lib/test/test_free_threading/test_reversed.py delete mode 100644 Lib/test/test_reversed.py diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py new file mode 100644 index 00000000000000..f5a1458df508b4 --- /dev/null +++ b/Lib/test/test_free_threading/test_reversed.py @@ -0,0 +1,36 @@ +import unittest +from threading import Thread +from test.support import threading_helper + + +@threading_helper.reap_threads +@threading_helper.requires_working_threading() +class TestReversed(unittest.TestCase): + def test_reversed_threading(self): + # Test reading out the iterator with multiple threads cannot corrupt + # the reversed iterator state. + # The reversed iterator is not guaranteed to be thread safe + + number_of_threads = 10 + size = 4_000 + + def work(r): + while True: + try: + l = r.__length_hint__() + next(r) + except StopIteration: + break + assert 0 <= l <= size + x = tuple(range(size)) + r = reversed(x) + worker_threads = [] + for ii in range(number_of_threads): + worker_threads.append(Thread(target=work, args=[r])) + for t in worker_threads: + t.start() + for t in worker_threads: + t.join() + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_reversed.py b/Lib/test/test_reversed.py deleted file mode 100644 index 30a1de64f1d062..00000000000000 --- a/Lib/test/test_reversed.py +++ /dev/null @@ -1,35 +0,0 @@ -import unittest -from threading import Thread -from test.support import threading_helper - - -@threading_helper.reap_threads -@threading_helper.requires_working_threading() -def test_reversed_threading(self): - # Test reading out the iterator with multiple threads cannot corrupt - # the reversed iterator state. - # The reversed iterator is not guaranteed to be thread safe - - number_of_threads = 10 - size = 4_000 - - def work(r): - while True: - try: - l = r.__length_hint__() - next(r) - except StopIteration: - break - assert 0 <= l <= size - x = tuple(range(size)) - r = reversed(x) - worker_threads = [] - for ii in range(number_of_threads): - worker_threads.append(Thread(target=work, args=[r])) - for t in worker_threads: - t.start() - for t in worker_threads: - t.join() - -if __name__ == "__main__": - unittest.main() diff --git a/Lib/test/test_str.py b/Lib/test/test_str.py index 7bdd2881904548..ff1a90efcfcfdf 100644 --- a/Lib/test/test_str.py +++ b/Lib/test/test_str.py @@ -2589,7 +2589,8 @@ def test_compare(self): def test_free_after_iterating(self): support.check_free_after_iterating(self, iter, str) - support.check_free_after_iterating(self, reversed, str) + if not sys._is_gil_enabled(): + support.check_free_after_iterating(self, reversed, str) def test_check_encoding_errors(self): # bpo-37388: str(bytes) and str.decode() must check encoding and errors From c7f6f928f86b1c7d49c0b8d5a61594e89285b9ca Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 23 Nov 2024 21:49:40 +0100 Subject: [PATCH 13/23] lint --- .../2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/{Core and Builtins => Core_and_Builtins}/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst (100%) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst similarity index 100% rename from Misc/NEWS.d/next/Core and Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst rename to Misc/NEWS.d/next/Core_and_Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst From a0c316baad718cefe3b5a6e144facbd67066d04f Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 14 Jan 2025 14:05:40 +0100 Subject: [PATCH 14/23] improve test --- Lib/test/test_free_threading/test_reversed.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index f5a1458df508b4..b13cd323f1df2d 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -14,14 +14,16 @@ def test_reversed_threading(self): number_of_threads = 10 size = 4_000 + workers_enabled = False def work(r): while True: - try: - l = r.__length_hint__() - next(r) - except StopIteration: - break - assert 0 <= l <= size + if workers_enabled: + try: + l = r.__length_hint__() + next(r) + except StopIteration: + break + assert 0 <= l <= size x = tuple(range(size)) r = reversed(x) worker_threads = [] @@ -29,6 +31,7 @@ def work(r): worker_threads.append(Thread(target=work, args=[r])) for t in worker_threads: t.start() + workers_enabled = True # make sure to start all threads simultaneously for t in worker_threads: t.join() From fc80d569f6ca24608ccadd9f378ed24ccc4a671a Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 15 Jan 2025 21:09:07 +0100 Subject: [PATCH 15/23] use Barrier in the test --- Lib/test/test_free_threading/test_reversed.py | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index b13cd323f1df2d..6bafa0acf1cea2 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -1,39 +1,42 @@ import unittest -from threading import Thread +from threading import Barrier, Thread from test.support import threading_helper -@threading_helper.reap_threads -@threading_helper.requires_working_threading() +#@threading_helper.reap_threads +#@threading_helper.requires_working_threading() class TestReversed(unittest.TestCase): def test_reversed_threading(self): # Test reading out the iterator with multiple threads cannot corrupt # the reversed iterator state. # The reversed iterator is not guaranteed to be thread safe - + number_of_iterations = 10 number_of_threads = 10 - size = 4_000 + size = 1_000 - workers_enabled = False + barrier = Barrier(number_of_threads) def work(r): + barrier.wait() while True: - if workers_enabled: - try: - l = r.__length_hint__() - next(r) - except StopIteration: - break - assert 0 <= l <= size + try: + l = r.__length_hint__() + next(r) + except StopIteration: + break + assert 0 <= l <= size x = tuple(range(size)) - r = reversed(x) - worker_threads = [] - for ii in range(number_of_threads): - worker_threads.append(Thread(target=work, args=[r])) - for t in worker_threads: - t.start() - workers_enabled = True # make sure to start all threads simultaneously - for t in worker_threads: - t.join() + + for _ in range(number_of_iterations): + r = reversed(x) + worker_threads = [] + for ii in range(number_of_threads): + worker_threads.append(Thread(target=work, args=[r])) + for t in worker_threads: + t.start() + for t in worker_threads: + t.join() + + barrier.reset() if __name__ == "__main__": unittest.main() From b1071dfee699c0cd6a16fc7f2709f35fd664307b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 10 Feb 2025 12:42:26 +0100 Subject: [PATCH 16/23] remove testing code --- Lib/test/test_free_threading/test_reversed.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 6bafa0acf1cea2..873c449a82e718 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -3,8 +3,8 @@ from test.support import threading_helper -#@threading_helper.reap_threads -#@threading_helper.requires_working_threading() +@threading_helper.reap_threads +@threading_helper.requires_working_threading() class TestReversed(unittest.TestCase): def test_reversed_threading(self): # Test reading out the iterator with multiple threads cannot corrupt From aa71804c5fdc1c375472bd7417fec39226f1cd2b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 11 Mar 2025 21:54:36 +0100 Subject: [PATCH 17/23] review comments --- Lib/test/test_free_threading/test_reversed.py | 9 +++------ Lib/test/test_str.py | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 873c449a82e718..48a6d6aef2d36d 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -3,8 +3,9 @@ from test.support import threading_helper +threading_helper.requires_working_threading(module=True) + @threading_helper.reap_threads -@threading_helper.requires_working_threading() class TestReversed(unittest.TestCase): def test_reversed_threading(self): # Test reading out the iterator with multiple threads cannot corrupt @@ -31,11 +32,7 @@ def work(r): worker_threads = [] for ii in range(number_of_threads): worker_threads.append(Thread(target=work, args=[r])) - for t in worker_threads: - t.start() - for t in worker_threads: - t.join() - + threading_helper.start_threads(worker_threads) barrier.reset() if __name__ == "__main__": diff --git a/Lib/test/test_str.py b/Lib/test/test_str.py index 709b951595b8d0..53985fc6b32d21 100644 --- a/Lib/test/test_str.py +++ b/Lib/test/test_str.py @@ -2596,7 +2596,7 @@ def test_compare(self): def test_free_after_iterating(self): support.check_free_after_iterating(self, iter, str) - if not sys._is_gil_enabled(): + if not support.Py_GIL_DISABLED: support.check_free_after_iterating(self, reversed, str) def test_check_encoding_errors(self): From 76931aa2dc76fd025c16eae9b0710504ef9db221 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 11 Mar 2025 21:58:44 +0100 Subject: [PATCH 18/23] rework --- Lib/test/test_free_threading/test_reversed.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 48a6d6aef2d36d..06a6ef24cf55ad 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -3,11 +3,8 @@ from test.support import threading_helper -threading_helper.requires_working_threading(module=True) - -@threading_helper.reap_threads class TestReversed(unittest.TestCase): - def test_reversed_threading(self): + def test_reversed(self): # Test reading out the iterator with multiple threads cannot corrupt # the reversed iterator state. # The reversed iterator is not guaranteed to be thread safe From 2c5e7ef7e3bf729bed4344569433235597792c07 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 11 Mar 2025 22:14:32 +0100 Subject: [PATCH 19/23] add thread helper code --- Lib/test/test_free_threading/test_reversed.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 06a6ef24cf55ad..639a8c9b4a6852 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -2,8 +2,11 @@ from threading import Barrier, Thread from test.support import threading_helper +threading_helper.requires_working_threading(module=True) class TestReversed(unittest.TestCase): + + @threading_helper.reap_threads def test_reversed(self): # Test reading out the iterator with multiple threads cannot corrupt # the reversed iterator state. From b36bd5059820ce100665eeb0f6b24b60261e149f Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 12 Mar 2025 11:15:13 +0530 Subject: [PATCH 20/23] Update Lib/test/test_free_threading/test_reversed.py --- Lib/test/test_free_threading/test_reversed.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 639a8c9b4a6852..38fcb7d64c67a4 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -8,9 +8,8 @@ class TestReversed(unittest.TestCase): @threading_helper.reap_threads def test_reversed(self): - # Test reading out the iterator with multiple threads cannot corrupt - # the reversed iterator state. - # The reversed iterator is not guaranteed to be thread safe + # Iterating over the iterator with multiple threads should not + # emit TSAN warnings number_of_iterations = 10 number_of_threads = 10 size = 1_000 From 01f31a7aa9837a41dab4f2e80acd94c18bc4f18c Mon Sep 17 00:00:00 2001 From: Kumar Aditya Date: Wed, 12 Mar 2025 05:50:19 +0000 Subject: [PATCH 21/23] use ctx manager --- Lib/test/test_free_threading/test_reversed.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_free_threading/test_reversed.py b/Lib/test/test_free_threading/test_reversed.py index 38fcb7d64c67a4..dd6016489c88ec 100644 --- a/Lib/test/test_free_threading/test_reversed.py +++ b/Lib/test/test_free_threading/test_reversed.py @@ -8,7 +8,7 @@ class TestReversed(unittest.TestCase): @threading_helper.reap_threads def test_reversed(self): - # Iterating over the iterator with multiple threads should not + # Iterating over the iterator with multiple threads should not # emit TSAN warnings number_of_iterations = 10 number_of_threads = 10 @@ -29,9 +29,10 @@ def work(r): for _ in range(number_of_iterations): r = reversed(x) worker_threads = [] - for ii in range(number_of_threads): + for _ in range(number_of_threads): worker_threads.append(Thread(target=work, args=[r])) - threading_helper.start_threads(worker_threads) + with threading_helper.start_threads(worker_threads): + pass barrier.reset() if __name__ == "__main__": From c4c1ea976c74a0b3c8910f722b434a2fff959e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 12 Mar 2025 11:48:46 +0100 Subject: [PATCH 22/23] Remove trailing comma in .github/workflows/build.yml --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 97ec5cbaa7475f..a26ba82387bc6c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -638,7 +638,7 @@ jobs: build-windows-msi, build-ubuntu-ssltests, test-hypothesis, - cifuzz, + cifuzz allowed-skips: >- ${{ !fromJSON(needs.build-context.outputs.run-docs) From eb60c8f90c5ddef16ebc4d15d318e92a8f8f4961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Wed, 12 Mar 2025 12:08:04 +0100 Subject: [PATCH 23/23] Revert "Remove trailing comma in .github/workflows/build.yml" This reverts commit c4c1ea976c74a0b3c8910f722b434a2fff959e90. --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a26ba82387bc6c..97ec5cbaa7475f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -638,7 +638,7 @@ jobs: build-windows-msi, build-ubuntu-ssltests, test-hypothesis, - cifuzz + cifuzz, allowed-skips: >- ${{ !fromJSON(needs.build-context.outputs.run-docs)