gh-120608: Make reversed iterator work with free-threading

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()

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

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.

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,15 +497,19 @@ 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;
  if (index < -1)
  index = -1;
  else if (index > n-1)
  index = n-1;
- ro->index = index;
+FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index);
  }
  Py_RETURN_NONE;
 }