8000 gh-120608: Make reversed iterator work with free-threading (#120971) · python/cpython@1fb7e2a · GitHub
[go: up one dir, main page]
Skip to content

Commit 1fb7e2a

Browse files
eendebakptcolesburykumaraditya303ambv
authored
gh-120608: Make reversed iterator work with free-threading (#120971)
Co-authored-by: Sam Gross <colesbury@gmail.com> Co-authored-by: Kumar Aditya <kumaraditya@python.org> Co-authored-by: Łukasz Langa <lukasz@langa.pl>
1 parent 4dcbe06 commit 1fb7e2a

File tree

4 files changed

+65
-10
lines changed

4 files changed

+65
-10
lines changed

‎Lib/test/test_free_threading/test_reversed.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import unittest
2+
from threading import Barrier, Thread
3+
from test.support import threading_helper
4+
5+
threading_helper.requires_working_threading(module=True)
6+
7+
class TestReversed(unittest.TestCase):
8+
9+
@threading_helper.reap_threads
10+
def test_reversed(self):
11+
# Iterating over the iterator with multiple threads should not
12+
# emit TSAN warnings
13+
number_of_iterations = 10
14+
number_of_threads = 10
15+
size = 1_000
16+
17+
barrier = Barrier(number_of_threads)
18+
def work(r):
19+
barrier.wait()
20+
while True:
21+
try:
22+
l = r.__length_hint__()
23+
next(r)
24+
except StopIteration:
25+
break
26+
assert 0 <= l <= size
27+
x = tuple(range(size))
28+
29+
for _ in range(number_of_iterations):
30+
r = reversed(x)
31+
worker_threads = []
32+
for _ in range(number_of_threads):
33+
worker_threads.append(Thread(target=work, args=[r]))
34+
with threading_helper.start_threads(worker_threads):
35+
pass
36+
barrier.reset()
37+
38+
if __name__ == "__main__":
39+
unittest.main()

‎Lib/test/test_str.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2605,7 +2605,8 @@ def test_compare(self):
26052605

26062606
def test_free_after_iterating(self):
26072607
support.check_free_after_iterating(self, iter, str)
2608-
support.check_free_after_iterating(self, reversed, str)
2608+
if not support.Py_GIL_DISABLED:
2609+
support.check_free_after_iterating(self, reversed, str)
26092610

26102611
def test_check_encoding_errors(self):
26112612
# 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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Adapt :func:`reversed` for use in the free-theading build.
2+
The :func:`reversed` is still not thread-safe in the sense that concurrent
3+
iterations may see the same object, but they will not corrupt the interpreter
4+
state.

‎Objects/enumobject.c

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -439,20 +439,22 @@ reversed_next(PyObject *op)
439439
{
440440
reversedobject *ro = _reversedobject_CAST(op);
441441
PyObject *item;
442-
Py_ssize_t index = ro->index;
442+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index);
443443

444444
if (index >= 0) {
445445
item = PySequence_GetItem(ro->seq, index);
446446
if (item != NULL) {
447-
ro->index--;
447+
FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index - 1);
448448
return item;
449449
}
450450
if (PyErr_ExceptionMatches(PyExc_IndexError) ||
451451
PyErr_ExceptionMatches(PyExc_StopIteration))
452452
PyErr_Clear();
453453
}
454-
ro->index = -1;
454+
FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, -1);
455+
#ifndef Py_GIL_DISABLED
455456
Py_CLEAR(ro->seq);
457+
#endif
456458
return NULL;
457459
}
458460

@@ -461,13 +463,15 @@ reversed_len(PyObject *op, PyObject *Py_UNUSED(ignored))
461463
{
462464
reversedobject *ro = _reversedobject_CAST(op);
463465
Py_ssize_t position, seqsize;
466+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index);
464467

465-
if (ro->seq == NULL)
468+
if (index == -1)
466469
return PyLong_FromLong(0);
470+
assert(ro->seq != NULL);
467471
seqsize = PySequence_Size(ro->seq);
468472
if (seqsize == -1)
469473
return NULL;
470-
position = ro->index + 1;
474+
position = index + 1;
471475
return PyLong_FromSsize_t((seqsize < position) ? 0 : position);
472476
}
473477

@@ -477,10 +481,13 @@ static PyObject *
477481
reversed_reduce(PyObject *op, PyObject *Py_UNUSED(ignored))
478482
{
479483
reversedobject *ro = _reversedobject_CAST(op);
480-
if (ro->seq)
484+
Py_ssize_t index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index);
485+
if (index != -1) {
481486
return Py_BuildValue("O(O)n", Py_TYPE(ro), ro->seq, ro->index);
482-
else
487+
}
488+
else {
483489
return Py_BuildValue("O(())", Py_TYPE(ro));
490+
}
484491
}
485492

486493
static PyObject *
@@ -490,15 +497,19 @@ reversed_setstate(PyObject *op, PyObject *state)
490497
Py_ssize_t index = PyLong_AsSsize_t(state);
491498
if (index == -1 && PyErr_Occurred())
492499
return NULL;
493-
if (ro->seq != 0) {
500+
Py_ssize_t ro_index = FT_ATOMIC_LOAD_SSIZE_RELAXED(ro->index);
501+
// if the iterator is exhausted we do not set the state
502+
// this is for backwards compatibility reasons. in practice this situation
503+
// will not occur, see gh-120971
504+
if (ro_index != -1) {
494505
Py_ssize_t n = PySequence_Size(ro->seq);
495506
if (n < 0)
496507
return NULL;
497508
if (index < -1)
498509
index = -1;
499510
else if (index > n-1)
500511
index = n-1;
501-
ro->index = index;
512+
FT_ATOMIC_STORE_SSIZE_RELAXED(ro->index, index);
502513
}
503514
Py_RETURN_NONE;
504515
}

0 commit comments

Comments
 (0)
0