https://github.com/python/cpython/commit/1fb7e2aeb7e4312b7f20f0d5f39ddd00d7762004
commit: 1fb7e2aeb7e4312b7f20f0d5f39ddd00d7762004
branch: main
author: Pieter Eendebak <pieter.eende...@gmail.com>
committer: ambv <luk...@langa.pl>
date: 2025-03-12T12:39:52+01:00
summary:
gh-120608: Make reversed iterator work with free-threading (#120971)
Co-authored-by: Sam Gross <colesb...@gmail.com>
Co-authored-by: Kumar Aditya <kumaradi...@python.org>
Co-authored-by: Łukasz Langa <luk...@langa.pl>
files:
A Lib/test/test_free_threading/test_reversed.py
A
Misc/NEWS.d/next/Core_and_Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst
M Lib/test/test_str.py
M Objects/enumobject.c
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..dd6016489c88ec
--- /dev/null
+++ b/Lib/test/test_free_threading/test_reversed.py
@@ -0,0 +1,39 @@
+import unittest
+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):
+ # Iterating over the iterator with multiple threads should not
+ # emit TSAN warnings
+ number_of_iterations = 10
+ number_of_threads = 10
+ size = 1_000
+
+ barrier = Barrier(number_of_threads)
+ def work(r):
+ barrier.wait()
+ while True:
+ try:
+ l = r.__length_hint__()
+ next(r)
+ except StopIteration:
+ break
+ assert 0 <= l <= size
+ x = tuple(range(size))
+
+ for _ in range(number_of_iterations):
+ r = reversed(x)
+ worker_threads = []
+ for _ in range(number_of_threads):
+ worker_threads.append(Thread(target=work, args=[r]))
+ with threading_helper.start_threads(worker_threads):
+ pass
+ barrier.reset()
+
+if __name__ == "__main__":
+ unittest.main()
diff --git a/Lib/test/test_str.py b/Lib/test/test_str.py
index 2694f5d45c7ebf..d6a7bd0da59910 100644
--- a/Lib/test/test_str.py
+++ b/Lib/test/test_str.py
@@ -2605,7 +2605,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 support.Py_GIL_DISABLED:
+ 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
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..31d1dfd54d7933
--- /dev/null
+++
b/Misc/NEWS.d/next/Core_and_Builtins/2024-06-24-20-08-55.gh-issue-120608.d75n8U.rst
@@ -0,0 +1,4 @@
+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.
diff --git a/Objects/enumobject.c b/Objects/enumobject.c
index eb8952675269a2..17510b29ee70b5 100644
--- a/Objects/enumobject.c
+++ b/Objects/enumobject.c
@@ -439,20 +439,22 @@ reversed_next(PyObject *op)
{
reversedobject *ro = _reversedobject_CAST(op);
PyObject *item;
- Py_ssize_t index = 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) {
- ro->index--;
+ FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index - 1);
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;
}
@@ -461,13 +463,15 @@ reversed_len(PyObject *op, PyObject *Py_UNUSED(ignored))
{
reversedobject *ro = _reversedobject_CAST(op);
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 != NULL);
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);
}
@@ -477,10 +481,13 @@ static PyObject *
reversed_reduce(PyObject *op, PyObject *Py_UNUSED(ignored))
{
reversedobject *ro = _reversedobject_CAST(op);
- 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
+ }
+ else {
return Py_BuildValue("O(())", Py_TYPE(ro));
+ }
}
static PyObject *
@@ -490,7 +497,11 @@ reversed_setstate(PyObject *op, 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;
@@ -498,7 +509,7 @@ reversed_setstate(PyObject *op, 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;
}
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
