From f8b06aeb9d05d1d948ce4483c6355906999ac3c3 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 14:22:57 -0500 Subject: [PATCH 01/40] Lock the iterable for list.__init__() --- Objects/listobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 930aefde325a7c..da365ecba0201e 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -1362,9 +1362,9 @@ _list_extend(PyListObject *self, PyObject *iterable) Py_END_CRITICAL_SECTION2(); } else { - Py_BEGIN_CRITICAL_SECTION(self); + Py_BEGIN_CRITICAL_SECTION2(self, iterable); res = list_extend_iter_lock_held(self, iterable); - Py_END_CRITICAL_SECTION(); + Py_END_CRITICAL_SECTION2(); } return res; } From cd6f5732baf4c7c05ed23b9e59fcd84285e8acb0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 15:08:52 -0500 Subject: [PATCH 02/40] Add blurb --- .../2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst new file mode 100644 index 00000000000000..a72d853f428de9 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst @@ -0,0 +1 @@ +Fix possible data race on iterables in :meth:`list.__init__` on the free-threaded build. From 49fb9e57c85d5fc969ebdd5f9385cbcf5248771f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 15:58:35 -0500 Subject: [PATCH 03/40] Add a test. --- Lib/test/test_list.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index ad7accf2099f43..72ed44403603bc 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -1,5 +1,7 @@ import sys from test import list_tests +from test import support +from test.support import threading_helper from test.support import cpython_only import pickle import unittest @@ -309,5 +311,35 @@ def test_tier2_invalidates_iterator(self): a.append(4) self.assertEqual(list(it), []) + @support.requires_resource("cpu") + def test_list_init_thread_safety(self): + # GH-126369: list.__init__() didn't lock iterables + from threading import Thread + + def my_generator(): + for i in range(100000): + yield i + + def gen_to_list(gen): + list(gen) + + target = my_generator() + # Note: it's intentional to throw out the exception here. Generators + # aren't thread safe, so who knows what will happen. Right now, it just spams + # a lot of ValueError's, but that might change if we decide to make generators + # thread safe in the future. We're just making sure it doesn't crash. + with threading_helper.catch_threading_exception(): + ts = [Thread(target=gen_to_list, args=(my_generator(),)) for _ in range(10)] + for t in ts: + t.start() + + for t in ts: + t.join() + + # Make sure it exhausted the generator + with self.assertRaises(StopIteration): + next(target) + + if __name__ == "__main__": unittest.main() From ab087ce08e8708689bf8e0f0ff0a910e750b8936 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 16:06:11 -0500 Subject: [PATCH 04/40] Fix the test. --- Lib/test/test_list.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 72ed44403603bc..ed54e82f75ec0c 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -314,31 +314,41 @@ def test_tier2_invalidates_iterator(self): @support.requires_resource("cpu") def test_list_init_thread_safety(self): # GH-126369: list.__init__() didn't lock iterables - from threading import Thread + from threading import Thread, Lock + import contextlib def my_generator(): for i in range(100000): yield i + + lock = Lock() + amount = 0 + thread_count = 10 + def gen_to_list(gen): - list(gen) - - target = my_generator() - # Note: it's intentional to throw out the exception here. Generators - # aren't thread safe, so who knows what will happen. Right now, it just spams - # a lot of ValueError's, but that might change if we decide to make generators - # thread safe in the future. We're just making sure it doesn't crash. - with threading_helper.catch_threading_exception(): - ts = [Thread(target=gen_to_list, args=(my_generator(),)) for _ in range(10)] + # Note: it's intentional to throw out the exception here. Generators + # aren't thread safe, so who knows what will happen. Right now, it just spams + # a lot of ValueError's, but that might change if we decide to make generators + # thread safe in the future. We're just making sure it doesn't crash. + with contextlib.suppress(ValueError): + list(gen) + + with lock: + nonlocal amount + amount += 1 + + with threading_helper.catch_threading_exception() as cm: + ts = [Thread(target=gen_to_list, args=(my_generator(),)) for _ in range(thread_count)] for t in ts: t.start() for t in ts: t.join() - # Make sure it exhausted the generator - with self.assertRaises(StopIteration): - next(target) + self.assertIsNone(cm.exc_value) + + self.assertEqual(amount, thread_count) if __name__ == "__main__": From d332da1675e1e448c06fa7b79e9aea75bff20c54 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 16:10:15 -0500 Subject: [PATCH 05/40] Fix the test (again). --- Lib/test/test_list.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index ed54e82f75ec0c..e293f9028389d2 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -318,7 +318,7 @@ def test_list_init_thread_safety(self): import contextlib def my_generator(): - for i in range(100000): + for i in range(1000000): yield i @@ -338,8 +338,9 @@ def gen_to_list(gen): nonlocal amount amount += 1 + generator = my_generator() with threading_helper.catch_threading_exception() as cm: - ts = [Thread(target=gen_to_list, args=(my_generator(),)) for _ in range(thread_count)] + ts = [Thread(target=gen_to_list, args=(generator,)) for _ in range(thread_count)] for t in ts: t.start() From 61fa342f75192d5f7f1ade8a59d2cf9f23c89ff4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 3 Nov 2024 16:31:48 -0500 Subject: [PATCH 06/40] Fix doctest. --- .../2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst index a72d853f428de9..a29684fbaab824 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst @@ -1 +1 @@ -Fix possible data race on iterables in :meth:`list.__init__` on the free-threaded build. +Fix possible data race on iterables in :meth:`!list.__init__` on the free-threaded build. From 6b6b8e64e6dd2630acaf7da9d96008fb3ef5fb2c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 4 Nov 2024 13:24:30 -0500 Subject: [PATCH 07/40] Make generators thread safe. --- Objects/genobject.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 19c2c4e3331a89..d52cbee497b51f 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -175,7 +175,7 @@ gen_dealloc(PyObject *self) } static PySendResult -gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, +gen_send_ex2_lock_held(PyGenObject *gen, PyObject *arg, PyObject **presult, int exc, int closing) { PyThreadState *tstate = _PyThreadState_GET(); @@ -273,6 +273,17 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, return result ? PYGEN_RETURN : PYGEN_ERROR; } +static inline PySendResult +gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult, + int exc, int closing) +{ + PySendResult result; + Py_BEGIN_CRITICAL_SECTION(gen); + result = gen_send_ex2_lock_held(gen, arg, presult, exc, closing); + Py_END_CRITICAL_SECTION(); + return result; +} + static PySendResult PyGen_am_send(PyObject *self, PyObject *arg, PyObject **result) { From 1b728428ca11f310bea6a17b226cf34755d29705 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 4 Nov 2024 13:26:45 -0500 Subject: [PATCH 08/40] Move test to test_generators. --- Lib/test/test_generators.py | 42 +++++++++++++++++++++++++++++++++++++ Lib/test/test_list.py | 41 ------------------------------------ 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index bf2cb1160723b0..379582acda6ec5 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -10,6 +10,7 @@ import types from test import support +from test.support import threading_helper try: import _testcapi @@ -268,6 +269,47 @@ def loop(): #This should not raise loop() + @support.requires_resource("cpu") + def test_list_init_thread_safety(self): + # GH-126369: generators were not thread safe + from threading import Thread, Lock + import contextlib + + def my_generator(): + for i in range(1000000): + yield i + + + lock = Lock() + amount = 0 + thread_count = 10 + + def gen_to_list(gen): + # Note: it's intentional to throw out the exception here. Generators + # aren't thread safe, so who knows what will happen. Right now, it just spams + # a lot of ValueError's, but that might change if we decide to make generators + # thread safe in the future. We're just making sure it doesn't crash. + with contextlib.suppress(ValueError): + list(gen) + + with lock: + nonlocal amount + amount += 1 + + generator = my_generator() + with threading_helper.catch_threading_exception() as cm: + ts = [Thread(target=gen_to_list, args=(generator,)) for _ in range(thread_count)] + for t in ts: + t.start() + + for t in ts: + t.join() + + self.assertIsNone(cm.exc_value) + + self.assertEqual(amount, thread_count) + + class ModifyUnderlyingIterableTest(unittest.TestCase): iterables = [ diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index e293f9028389d2..0bddcf8479c1cb 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -1,7 +1,6 @@ import sys from test import list_tests from test import support -from test.support import threading_helper from test.support import cpython_only import pickle import unittest @@ -311,46 +310,6 @@ def test_tier2_invalidates_iterator(self): a.append(4) self.assertEqual(list(it), []) - @support.requires_resource("cpu") - def test_list_init_thread_safety(self): - # GH-126369: list.__init__() didn't lock iterables - from threading import Thread, Lock - import contextlib - - def my_generator(): - for i in range(1000000): - yield i - - - lock = Lock() - amount = 0 - thread_count = 10 - - def gen_to_list(gen): - # Note: it's intentional to throw out the exception here. Generators - # aren't thread safe, so who knows what will happen. Right now, it just spams - # a lot of ValueError's, but that might change if we decide to make generators - # thread safe in the future. We're just making sure it doesn't crash. - with contextlib.suppress(ValueError): - list(gen) - - with lock: - nonlocal amount - amount += 1 - - generator = my_generator() - with threading_helper.catch_threading_exception() as cm: - ts = [Thread(target=gen_to_list, args=(generator,)) for _ in range(thread_count)] - for t in ts: - t.start() - - for t in ts: - t.join() - - self.assertIsNone(cm.exc_value) - - self.assertEqual(amount, thread_count) - if __name__ == "__main__": unittest.main() From 684ccaff09177641bf4faec5f94f03bf0e1638f0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 4 Nov 2024 13:28:45 -0500 Subject: [PATCH 09/40] Update the blurb --- .../2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst index a29684fbaab824..f7c3be98529d18 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-03-15-07-15.gh-issue-126366.nd_LOu.rst @@ -1 +1 @@ -Fix possible data race on iterables in :meth:`!list.__init__` on the free-threaded build. +Fix possible data races when using :term:`generator iterator` objects concurrently. From 9edb2659246c4800836f222781acb6e575623cb0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 4 Nov 2024 13:29:38 -0500 Subject: [PATCH 10/40] Remove changes to list tests. --- Lib/test/test_list.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 0bddcf8479c1cb..ad7accf2099f43 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -1,6 +1,5 @@ import sys from test import list_tests -from test import support from test.support import cpython_only import pickle import unittest @@ -310,6 +309,5 @@ def test_tier2_invalidates_iterator(self): a.append(4) self.assertEqual(list(it), []) - if __name__ == "__main__": unittest.main() From 3a1fe3ad5ab81474744fbbe7ec8da2eba49960bb Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 4 Nov 2024 13:30:28 -0500 Subject: [PATCH 11/40] Fix the test name. --- Lib/test/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 379582acda6ec5..8dccbb879baab8 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -270,7 +270,7 @@ def loop(): loop() @support.requires_resource("cpu") - def test_list_init_thread_safety(self): + def test_generator_thread_safety(self): # GH-126369: generators were not thread safe from threading import Thread, Lock import contextlib From 8e84dd4ca9f27770af908383e578d89c2e8fcb3e Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 6 Nov 2024 13:41:59 +0000 Subject: [PATCH 12/40] Add a lock in the _SEND instruction --- Python/bytecodes.c | 2 ++ Python/generated_cases.c.h | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index fa98af12c69aef..2ecf5a1fed99db 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1105,6 +1105,7 @@ dummy_func( ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) { PyGenObject *gen = (PyGenObject *)receiver_o; + Py_BEGIN_CRITICAL_SECTION(gen); _PyInterpreterFrame *gen_frame = &gen->gi_iframe; STACK_SHRINK(1); _PyFrame_StackPush(gen_frame, v); @@ -1115,6 +1116,7 @@ dummy_func( frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); assert(gen_frame->previous == NULL); gen_frame->previous = frame; + Py_END_CRITICAL_SECTION(); DISPATCH_INLINED(gen_frame); } if (PyStackRef_Is(v, PyStackRef_None) && PyIter_Check(receiver_o)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 632cbc7790a4d8..a537bc45a000e5 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -7029,6 +7029,9 @@ ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) { PyGenObject *gen = (PyGenObject *)receiver_o; + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); _PyInterpreterFrame *gen_frame = &gen->gi_iframe; STACK_SHRINK(1); _PyFrame_StackPush(gen_frame, v); @@ -7039,6 +7042,9 @@ frame->return_offset = (uint16_t)( 2 + oparg); assert(gen_frame->previous == NULL); gen_frame->previous = frame; + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); DISPATCH_INLINED(gen_frame); } if (PyStackRef_Is(v, PyStackRef_None) && PyIter_Check(receiver_o)) { From 517b7f881812c90d817a909168faf97bec1f7c8c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 6 Nov 2024 13:44:37 +0000 Subject: [PATCH 13/40] Add a test in test_free_threading (by Ken) --- .../test_free_threading/test_generators.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 Lib/test/test_free_threading/test_generators.py diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py new file mode 100644 index 00000000000000..999cb189be15b4 --- /dev/null +++ b/Lib/test/test_free_threading/test_generators.py @@ -0,0 +1,23 @@ +import unittest +import concurrent.futures + +from unittest import TestCase + +from test.support import threading_helper + +@threading_helper.requires_working_threading() +class TestGen(TestCase): + def test_generators_basic(self): + def gen(): + for _ in range(5000): + yield + + + it = gen() + with concurrent.futures.ThreadPoolExecutor() as executor: + for _ in range(5000): + executor.submit(lambda: next(it)) + + +if __name__ == "__main__": + unittest.main() From 6276ae5476bb3c33a172d8a5c0d82808d9bcdee2 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 6 Nov 2024 14:06:59 +0000 Subject: [PATCH 14/40] Fix failing build. --- Python/bytecodes.c | 3 ++- Python/generated_cases.c.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 2ecf5a1fed99db..b262140f06d996 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1104,9 +1104,10 @@ dummy_func( (Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) && ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) { + _PyInterpreterFrame *gen_frame; PyGenObject *gen = (PyGenObject *)receiver_o; Py_BEGIN_CRITICAL_SECTION(gen); - _PyInterpreterFrame *gen_frame = &gen->gi_iframe; + gen_frame = &gen->gi_iframe; STACK_SHRINK(1); _PyFrame_StackPush(gen_frame, v); gen->gi_frame_state = FRAME_EXECUTING; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index a537bc45a000e5..df4c61a365383c 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -7028,11 +7028,12 @@ (Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) && ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) { + _PyInterpreterFrame *gen_frame; PyGenObject *gen = (PyGenObject *)receiver_o; _PyFrame_SetStackPointer(frame, stack_pointer); Py_BEGIN_CRITICAL_SECTION(gen); stack_pointer = _PyFrame_GetStackPointer(frame); - _PyInterpreterFrame *gen_frame = &gen->gi_iframe; + gen_frame = &gen->gi_iframe; STACK_SHRINK(1); _PyFrame_StackPush(gen_frame, v); gen->gi_frame_state = FRAME_EXECUTING; From 570d80e0514a1ef9a53016f3da11fd430183fe03 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 29 Nov 2024 20:21:51 -0500 Subject: [PATCH 15/40] Revert "Lock the iterable for list.__init__()" This reverts commit f8b06aeb9d05d1d948ce4483c6355906999ac3c3. --- Objects/listobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index da365ecba0201e..930aefde325a7c 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -1362,9 +1362,9 @@ _list_extend(PyListObject *self, PyObject *iterable) Py_END_CRITICAL_SECTION2(); } else { - Py_BEGIN_CRITICAL_SECTION2(self, iterable); + Py_BEGIN_CRITICAL_SECTION(self); res = list_extend_iter_lock_held(self, iterable); - Py_END_CRITICAL_SECTION2(); + Py_END_CRITICAL_SECTION(); } return res; } From 690f45bf4fb828f1f7a29448a63b6016e971b633 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 20 Dec 2024 13:25:24 +0000 Subject: [PATCH 16/40] Some small changes to the test. --- Lib/test/test_free_threading/test_generators.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 999cb189be15b4..01c3ce12ab8493 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -1,5 +1,4 @@ import unittest -import concurrent.futures from unittest import TestCase @@ -7,16 +6,23 @@ @threading_helper.requires_working_threading() class TestGen(TestCase): - def test_generators_basic(self): + def test_generator_send(self): + import threading + def gen(): for _ in range(5000): yield it = gen() - with concurrent.futures.ThreadPoolExecutor() as executor: - for _ in range(5000): - executor.submit(lambda: next(it)) + with threading_helper.catch_threading_exception() as cm: + # next(it) is equivalent to it.send(None) + with threading_helper.start_threads( + (threading.Thread(target=lambda: next(it)) for _ in range(10)) + ): + pass + + self.assertIsNone(cm.exc_value) if __name__ == "__main__": From 200ba8b1cbbbab365315ce69a30ab5e897113775 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 20 Dec 2024 08:31:17 -0500 Subject: [PATCH 17/40] Update Lib/test/test_free_threading/test_generators.py Co-authored-by: Victor Stinner --- Lib/test/test_free_threading/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 01c3ce12ab8493..24da6e48e15b27 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -10,7 +10,7 @@ def test_generator_send(self): import threading def gen(): - for _ in range(5000): + for _ in range(5_000): yield From 51f2778b017353fbb7d598154a2b84b554a2ee30 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Fri, 20 Dec 2024 13:31:58 +0000 Subject: [PATCH 18/40] Lower iteration count. --- Lib/test/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index dc049a258b1a5a..fe5cb2fe946e99 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -276,7 +276,7 @@ def test_generator_thread_safety(self): import contextlib def my_generator(): - for i in range(1000000): + for i in range(1000): yield i From 6447a825936926e7233ad92292daa7e8ed4afa4a Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 10:55:41 -0500 Subject: [PATCH 19/40] Lock _PyGen_yf --- Objects/genobject.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index cdbd1923355d11..1a8c89bf5136f3 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -366,8 +366,8 @@ is_resume(_Py_CODEUNIT *instr) ); } -PyObject * -_PyGen_yf(PyGenObject *gen) +static PyObject * +_PyGen_yf_lock_held(PyGenObject *gen) { if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) { _PyInterpreterFrame *frame = &gen->gi_iframe; @@ -379,8 +379,18 @@ _PyGen_yf(PyGenObject *gen) return NULL; } +PyObject * +_PyGen_yf(PyGenObject *gen) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(gen); + res = _PyGen_yf_lock_held(gen); + Py_END_CRITICAL_SECTION(); + return res; +} + static PyObject * -gen_close(PyObject *self, PyObject *args) +gen_close_lock_held(PyObject *self, PyObject *args) { PyGenObject *gen = _PyGen_CAST(self); @@ -446,7 +456,6 @@ gen_close(PyObject *self, PyObject *args) return NULL; } - PyDoc_STRVAR(throw_doc, "throw(value)\n\ throw(type[,value[,tb]])\n\ From efe9f870e26f4da05420517fdb0427f562fb686c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 10:56:05 -0500 Subject: [PATCH 20/40] Remove name change. --- Objects/genobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 1a8c89bf5136f3..718bbc8433bac5 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -390,7 +390,7 @@ _PyGen_yf(PyGenObject *gen) } static PyObject * -gen_close_lock_held(PyObject *self, PyObject *args) +gen_close(PyObject *self, PyObject *args) { PyGenObject *gen = _PyGen_CAST(self); From 55ea54d4253c5daacd4fc2b8a1aaf46e850f7e22 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:08:26 -0500 Subject: [PATCH 21/40] Use a critical section for close() --- Objects/clinic/genobject.c.h | 21 +++++++++++++++++++++ Objects/genobject.c | 34 ++++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 Objects/clinic/genobject.c.h diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h new file mode 100644 index 00000000000000..39fdaa15179e14 --- /dev/null +++ b/Objects/clinic/genobject.c.h @@ -0,0 +1,21 @@ +/*[clinic input] +preserve +[clinic start generated code]*/ + +PyDoc_STRVAR(gen_close__doc__, +"close($self, /)\n" +"--\n" +"\n"); + +#define GEN_CLOSE_METHODDEF \ + {"close", (PyCFunction)gen_close, METH_NOARGS, gen_close__doc__}, + +static PyObject * +gen_close_impl(PyGenObject *self); + +static PyObject * +gen_close(PyGenObject *self, PyObject *Py_UNUSED(ignored)) +{ + return gen_close_impl(self); +} +/*[clinic end generated code: output=e7c9681104424906 input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index 718bbc8433bac5..c84e7a6ae8574e 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -17,8 +17,14 @@ #include "pystats.h" +/*[clinic input] +class generator "PyGenObject *" "&PyGen_Type" +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=a6c98fdc710c6976]*/ + +#include "clinic/genobject.c.h" + // Forward declarations -static PyObject* gen_close(PyObject *, PyObject *); static PyObject* async_gen_asend_new(PyAsyncGenObject *, PyObject *); static PyObject* async_gen_athrow_new(PyAsyncGenObject *, PyObject *); @@ -119,7 +125,7 @@ _PyGen_Finalize(PyObject *self) _PyErr_WarnUnawaitedCoroutine((PyObject *)gen); } else { - PyObject *res = gen_close((PyObject*)gen, NULL); + PyObject *res = gen_close(gen, NULL); if (res == NULL) { if (PyErr_Occurred()) { PyErr_WriteUnraisable(self); @@ -335,7 +341,7 @@ gen_close_iter(PyObject *yf) PyObject *retval = NULL; if (PyGen_CheckExact(yf) || PyCoro_CheckExact(yf)) { - retval = gen_close((PyObject *)yf, NULL); + retval = gen_close((PyGenObject *)yf, NULL); if (retval == NULL) return -1; } @@ -389,8 +395,16 @@ _PyGen_yf(PyGenObject *gen) return res; } +/*[clinic input] +@critical_section +generator.close as gen_close + +raise GeneratorExit inside generator. +[clinic start generated code]*/ + static PyObject * -gen_close(PyObject *self, PyObject *args) +gen_close_impl(PyGenObject *self) +/*[clinic end generated code: output=2d7adf450173059c input=6c40e85559b6f098]*/ { PyGenObject *gen = _PyGen_CAST(self); @@ -624,7 +638,11 @@ gen_throw(PyGenObject *gen, PyObject *const *args, Py_ssize_t nargs) else if (nargs == 2) { val = args[1]; } - return _gen_throw(gen, 1, typ, val, tb); + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(gen); + res = _gen_throw(gen, 1, typ, val, tb); + Py_END_CRITICAL_SECTION(); + return res; } @@ -856,7 +874,7 @@ PyDoc_STRVAR(sizeof__doc__, static PyMethodDef gen_methods[] = { {"send", gen_send, METH_O, send_doc}, {"throw", _PyCFunction_CAST(gen_throw), METH_FASTCALL, throw_doc}, - {"close", gen_close, METH_NOARGS, close_doc}, + GEN_CLOSE_METHODDEF {"__sizeof__", (PyCFunction)gen_sizeof, METH_NOARGS, sizeof__doc__}, {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, {NULL, NULL} /* Sentinel */ @@ -1217,7 +1235,7 @@ PyDoc_STRVAR(coro_close_doc, static PyMethodDef coro_methods[] = { {"send", gen_send, METH_O, coro_send_doc}, {"throw",_PyCFunction_CAST(gen_throw), METH_FASTCALL, coro_throw_doc}, - {"close", gen_close, METH_NOARGS, coro_close_doc}, + GEN_CLOSE_METHODDEF {"__sizeof__", (PyCFunction)gen_sizeof, METH_NOARGS, sizeof__doc__}, {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, {NULL, NULL} /* Sentinel */ @@ -1316,7 +1334,7 @@ static PyObject * coro_wrapper_close(PyObject *self, PyObject *args) { PyCoroWrapper *cw = _PyCoroWrapper_CAST(self); - return gen_close((PyObject *)cw->cw_coroutine, args); + return gen_close((PyGenObject *)cw->cw_coroutine, args); } static int From 17b7acf37b793235930e226eae5423bc62b9aebd Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:20:50 -0500 Subject: [PATCH 22/40] Fix races with gi_frame_state --- Objects/clinic/genobject.c.h | 90 ++++++++++++++++++++++++++++++++++-- Objects/genobject.c | 35 ++++++++++---- 2 files changed, 113 insertions(+), 12 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index 39fdaa15179e14..a420a22b05a9bd 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -2,10 +2,13 @@ preserve [clinic start generated code]*/ +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() + PyDoc_STRVAR(gen_close__doc__, "close($self, /)\n" "--\n" -"\n"); +"\n" +"raise GeneratorExit inside generator."); #define GEN_CLOSE_METHODDEF \ {"close", (PyCFunction)gen_close, METH_NOARGS, gen_close__doc__}, @@ -16,6 +19,87 @@ gen_close_impl(PyGenObject *self); static PyObject * gen_close(PyGenObject *self, PyObject *Py_UNUSED(ignored)) { - return gen_close_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = gen_close_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(gen_getrunning_DOCSTR) +# define gen_getrunning_DOCSTR NULL +#endif +#if defined(GEN_GETRUNNING_GETSETDEF) +# undef GEN_GETRUNNING_GETSETDEF +# define GEN_GETRUNNING_GETSETDEF {"gi_running", (getter)gen_getrunning_get, (setter)gen_getrunning_set, gen_getrunning_DOCSTR}, +#else +# define GEN_GETRUNNING_GETSETDEF {"gi_running", (getter)gen_getrunning_get, NULL, gen_getrunning_DOCSTR}, +#endif + +static PyObject * +gen_getrunning_get_impl(PyGenObject *self); + +static PyObject * +gen_getrunning_get(PyGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = gen_getrunning_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(gen_getsuspended_DOCSTR) +# define gen_getsuspended_DOCSTR NULL +#endif +#if defined(GEN_GETSUSPENDED_GETSETDEF) +# undef GEN_GETSUSPENDED_GETSETDEF +# define GEN_GETSUSPENDED_GETSETDEF {"gi_suspended", (getter)gen_getsuspended_get, (setter)gen_getsuspended_set, gen_getsuspended_DOCSTR}, +#else +# define GEN_GETSUSPENDED_GETSETDEF {"gi_suspended", (getter)gen_getsuspended_get, NULL, gen_getsuspended_DOCSTR}, +#endif + +static PyObject * +gen_getsuspended_get_impl(PyGenObject *self); + +static PyObject * +gen_getsuspended_get(PyGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = gen_getsuspended_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(gen_getframe_DOCSTR) +# define gen_getframe_DOCSTR NULL +#endif +#if defined(GEN_GETFRAME_GETSETDEF) +# undef GEN_GETFRAME_GETSETDEF +# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, (setter)gen_getframe_set, gen_getframe_DOCSTR}, +#else +# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, NULL, gen_getframe_DOCSTR}, +#endif + +static PyObject * +gen_getframe_get_impl(PyGenObject *self); + +static PyObject * +gen_getframe_get(PyGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = gen_getframe_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } -/*[clinic end generated code: output=e7c9681104424906 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=71e69e51fda7e6a9 input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index c84e7a6ae8574e..b706c1ba865b1d 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -404,7 +404,7 @@ raise GeneratorExit inside generator. static PyObject * gen_close_impl(PyGenObject *self) -/*[clinic end generated code: output=2d7adf450173059c input=6c40e85559b6f098]*/ +/*[clinic end generated code: output=2d7adf450173059c input=417e6161842080c9]*/ { PyGenObject *gen = _PyGen_CAST(self); @@ -787,27 +787,40 @@ gen_getyieldfrom(PyObject *gen, void *Py_UNUSED(ignored)) return yf; } +/*[clinic input] +@getter +@critical_section +generator.gi_running as gen_getrunning +[clinic start generated code]*/ static PyObject * -gen_getrunning(PyObject *self, void *Py_UNUSED(ignored)) +gen_getrunning_get_impl(PyGenObject *self) +/*[clinic end generated code: output=a7233b957ce88a6f input=d3d995cf1581b21b]*/ { - PyGenObject *gen = _PyGen_CAST(self); - if (gen->gi_frame_state == FRAME_EXECUTING) { + if (self->gi_frame_state == FRAME_EXECUTING) { Py_RETURN_TRUE; } Py_RETURN_FALSE; } +/*[clinic input] +@getter +@critical_section +generator.gi_suspended as gen_getsuspended +[clinic start generated code]*/ + static PyObject * -gen_getsuspended(PyObject *self, void *Py_UNUSED(ignored)) +gen_getsuspended_get_impl(PyGenObject *self) +/*[clinic end generated code: output=a0345f9be186eda3 input=880e9fb8436726cb]*/ { PyGenObject *gen = _PyGen_CAST(self); return PyBool_FromLong(FRAME_STATE_SUSPENDED(gen->gi_frame_state)); } static PyObject * -_gen_getframe(PyGenObject *gen, const char *const name) +_gen_getframe(PyGenObject *self, const char *name) { + PyGenObject *gen = _PyGen_CAST(self); if (PySys_Audit("object.__getattr__", "Os", gen, name) < 0) { return NULL; } @@ -821,7 +834,11 @@ static PyObject * gen_getframe(PyObject *self, void *Py_UNUSED(ignored)) { PyGenObject *gen = _PyGen_CAST(self); - return _gen_getframe(gen, "gi_frame"); + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(gen); + res = _gen_getframe(gen, "gi_frame"); + Py_END_CRITICAL_SECTION(); + return res; } static PyObject * @@ -847,9 +864,9 @@ static PyGetSetDef gen_getsetlist[] = { PyDoc_STR("qualified name of the generator")}, {"gi_yieldfrom", gen_getyieldfrom, NULL, PyDoc_STR("object being iterated by yield from, or None")}, - {"gi_running", gen_getrunning, NULL, NULL}, + GEN_GETRUNNING_GETSETDEF + GEN_GETSUSPENDED_GETSETDEF {"gi_frame", gen_getframe, NULL, NULL}, - {"gi_suspended", gen_getsuspended, NULL, NULL}, {"gi_code", gen_getcode, NULL, NULL}, {NULL} /* Sentinel */ }; From ccc65c65eeb40adeebb9b20dc7b6ff39dbd1519c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:21:40 -0500 Subject: [PATCH 23/40] Rerun clinic. --- Objects/clinic/genobject.c.h | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index a420a22b05a9bd..908d18de621165 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -77,29 +77,4 @@ gen_getsuspended_get(PyGenObject *self, void *Py_UNUSED(context)) return return_value; } - -#if !defined(gen_getframe_DOCSTR) -# define gen_getframe_DOCSTR NULL -#endif -#if defined(GEN_GETFRAME_GETSETDEF) -# undef GEN_GETFRAME_GETSETDEF -# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, (setter)gen_getframe_set, gen_getframe_DOCSTR}, -#else -# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, NULL, gen_getframe_DOCSTR}, -#endif - -static PyObject * -gen_getframe_get_impl(PyGenObject *self); - -static PyObject * -gen_getframe_get(PyGenObject *self, void *Py_UNUSED(context)) -{ - PyObject *return_value = NULL; - - Py_BEGIN_CRITICAL_SECTION(self); - return_value = gen_getframe_get_impl(self); - Py_END_CRITICAL_SECTION(); - - return return_value; -} -/*[clinic end generated code: output=71e69e51fda7e6a9 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ff4665c12c43063f input=a9049054013a1b77]*/ From 192b6a3f2e82f080c61214f3fc45b1a3b43b1a52 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:28:49 -0500 Subject: [PATCH 24/40] Put a critical section around bytecodes that access gi_frame_state --- Include/internal/pycore_opcode_metadata.h | 6 +-- Include/internal/pycore_uop_metadata.h | 6 +-- Python/bytecodes.c | 32 +++++++----- Python/executor_cases.c.h | 24 +++++++-- Python/generated_cases.c.h | 64 +++++++++++++++++------ 5 files changed, 95 insertions(+), 37 deletions(-) diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 5fb236836dccd9..cf6f4d29698a98 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -2036,7 +2036,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [FORMAT_SIMPLE] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [FORMAT_WITH_SPEC] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [FOR_ITER] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, - [FOR_ITER_GEN] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG }, + [FOR_ITER_GEN] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG }, [FOR_ITER_LIST] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EXIT_FLAG }, [FOR_ITER_RANGE] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EXIT_FLAG | HAS_ERROR_FLAG }, [FOR_ITER_TUPLE] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EXIT_FLAG }, @@ -2134,7 +2134,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [RETURN_GENERATOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [RETURN_VALUE] = { true, INSTR_FMT_IX, 0 }, [SEND] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [SEND_GEN] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG }, + [SEND_GEN] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG }, [SETUP_ANNOTATIONS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [SET_ADD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [SET_FUNCTION_ATTRIBUTE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, @@ -2170,7 +2170,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [UNPACK_SEQUENCE_TUPLE] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG }, [UNPACK_SEQUENCE_TWO_TUPLE] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG }, [WITH_EXCEPT_START] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [YIELD_VALUE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, + [YIELD_VALUE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ESCAPES_FLAG }, [JUMP] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_EVAL_BREAK_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [JUMP_IF_FALSE] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [JUMP_IF_TRUE] = { true, -1, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index e71194b116e020..7babe4b6ec81ef 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -102,8 +102,8 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_GET_AITER] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_GET_ANEXT] = HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_GET_AWAITABLE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, - [_SEND_GEN_FRAME] = HAS_ARG_FLAG | HAS_DEOPT_FLAG, - [_YIELD_VALUE] = HAS_ARG_FLAG, + [_SEND_GEN_FRAME] = HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG, + [_YIELD_VALUE] = HAS_ARG_FLAG | HAS_ESCAPES_FLAG, [_POP_EXCEPT] = HAS_ESCAPES_FLAG, [_LOAD_COMMON_CONSTANT] = HAS_ARG_FLAG, [_LOAD_BUILD_CLASS] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, @@ -199,7 +199,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_ITER_CHECK_RANGE] = HAS_EXIT_FLAG, [_GUARD_NOT_EXHAUSTED_RANGE] = HAS_EXIT_FLAG, [_ITER_NEXT_RANGE] = HAS_ERROR_FLAG, - [_FOR_ITER_GEN_FRAME] = HAS_ARG_FLAG | HAS_DEOPT_FLAG, + [_FOR_ITER_GEN_FRAME] = HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ESCAPES_FLAG, [_LOAD_SPECIAL] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_WITH_EXCEPT_START] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_PUSH_EXC_INFO] = 0, diff --git a/Python/bytecodes.c b/Python/bytecodes.c index fb79e79dd5b98e..69bd5c31a6a5a7 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1135,22 +1135,24 @@ dummy_func( PyObject *retval_o; assert(frame != &entry_frame); if ((tstate->interp->eval_frame == NULL) && - (Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) && - ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) + (Py_TYPE(receiver_o) == &PyGen_Type || + Py_TYPE(receiver_o) == &PyCoro_Type)) { _PyInterpreterFrame *gen_frame; PyGenObject *gen = (PyGenObject *)receiver_o; Py_BEGIN_CRITICAL_SECTION(gen); - gen_frame = &gen->gi_iframe; - STACK_SHRINK(1); - _PyFrame_StackPush(gen_frame, v); - gen->gi_frame_state = FRAME_EXECUTING; - gen->gi_exc_state.previous_item = tstate->exc_info; - tstate->exc_info = &gen->gi_exc_state; - assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); - assert(gen_frame->previous == NULL); - gen_frame->previous = frame; + if (gen->gi_frame_state < FRAME_EXECUTING) { + gen_frame = &gen->gi_iframe; + STACK_SHRINK(1); + _PyFrame_StackPush(gen_frame, v); + gen->gi_frame_state = FRAME_EXECUTING; + gen->gi_exc_state.previous_item = tstate->exc_info; + tstate->exc_info = &gen->gi_exc_state; + assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); + assert(gen_frame->previous == NULL); + gen_frame->previous = frame; + } Py_END_CRITICAL_SECTION(); DISPATCH_INLINED(gen_frame); } @@ -1185,6 +1187,7 @@ dummy_func( op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame: _PyInterpreterFrame *)) { PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver); + Py_BEGIN_CRITICAL_SECTION(gen); DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type); DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING); STAT_INC(SEND, hit); @@ -1197,6 +1200,7 @@ dummy_func( assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX); frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); gen_frame->previous = frame; + Py_END_CRITICAL_SECTION(); } macro(SEND_GEN) = @@ -1214,6 +1218,7 @@ dummy_func( #endif frame->instr_ptr++; PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); + Py_BEGIN_CRITICAL_SECTION(gen); assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1); assert(oparg == 0 || oparg == 1); gen->gi_frame_state = FRAME_SUSPENDED + oparg; @@ -1239,6 +1244,7 @@ dummy_func( RELOAD_STACK(); LOAD_IP(1 + INLINE_CACHE_ENTRIES_SEND); value = temp; + Py_END_CRITICAL_SECTION(); LLTRACE_RESUME_FRAME(); } @@ -3136,6 +3142,7 @@ dummy_func( op(_FOR_ITER_GEN_FRAME, (iter -- iter, gen_frame: _PyInterpreterFrame*)) { PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter); + Py_BEGIN_CRITICAL_SECTION(gen); DEOPT_IF(Py_TYPE(gen) != &PyGen_Type); DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING); STAT_INC(FOR_ITER, hit); @@ -3145,6 +3152,7 @@ dummy_func( gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; + Py_END_CRITICAL_SECTION(); // oparg is the return offset from the next instruction. frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 6e752c57cd70f3..8e15c9c6b9db14 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1470,6 +1470,9 @@ v = stack_pointer[-1]; receiver = stack_pointer[-2]; PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); if (Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -1488,6 +1491,9 @@ frame->return_offset = (uint16_t)( 2 + oparg); gen_frame->previous = frame; stack_pointer[-1].bits = (uintptr_t)gen_frame; + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); break; } @@ -1504,6 +1510,9 @@ #endif frame->instr_ptr++; PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1); assert(oparg == 0 || oparg == 1); gen->gi_frame_state = FRAME_SUSPENDED + oparg; @@ -1530,10 +1539,13 @@ stack_pointer = _PyFrame_GetStackPointer(frame); LOAD_IP(1 + INLINE_CACHE_ENTRIES_SEND); value = temp; - LLTRACE_RESUME_FRAME(); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); + LLTRACE_RESUME_FRAME(); break; } @@ -3740,6 +3752,9 @@ oparg = CURRENT_OPARG(); iter = stack_pointer[-1]; PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); if (Py_TYPE(gen) != &PyGen_Type) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -3755,11 +3770,14 @@ gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; - // oparg is the return offset from the next instruction. - frame->return_offset = (uint16_t)( 2 + oparg); stack_pointer[0].bits = (uintptr_t)gen_frame; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); + // oparg is the return offset from the next instruction. + frame->return_offset = (uint16_t)( 2 + oparg); break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index e8a8b5239877f3..2aec9797be00bf 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3978,6 +3978,9 @@ { iter = stack_pointer[-1]; PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); DEOPT_IF(Py_TYPE(gen) != &PyGen_Type, FOR_ITER); DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, FOR_ITER); STAT_INC(FOR_ITER, hit); @@ -3987,6 +3990,12 @@ gen->gi_exc_state.previous_item = tstate->exc_info; tstate->exc_info = &gen->gi_exc_state; gen_frame->previous = frame; + stack_pointer[0].bits = (uintptr_t)gen_frame; + stack_pointer += 1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); // oparg is the return offset from the next instruction. frame->return_offset = (uint16_t)( 2 + oparg); } @@ -3997,6 +4006,8 @@ // Eventually this should be the only occurrence of this code. assert(tstate->interp->eval_frame == NULL); _PyInterpreterFrame *temp = new_frame; + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); assert(new_frame->previous == frame || new_frame->previous->previous == frame); CALL_STAT_INC(inlined_py_calls); @@ -4979,6 +4990,9 @@ #endif frame->instr_ptr++; PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1); assert(oparg == 0 || oparg == 1); gen->gi_frame_state = FRAME_SUSPENDED + oparg; @@ -5005,11 +5019,14 @@ stack_pointer = _PyFrame_GetStackPointer(frame); LOAD_IP(1 + INLINE_CACHE_ENTRIES_SEND); value = temp; + stack_pointer[0] = value; + stack_pointer += 1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); LLTRACE_RESUME_FRAME(); } - stack_pointer[0] = value; - stack_pointer += 1; - assert(WITHIN_STACK_BOUNDS()); DISPATCH(); } @@ -7095,24 +7112,26 @@ PyObject *retval_o; assert(frame != &entry_frame); if ((tstate->interp->eval_frame == NULL) && - (Py_TYPE(receiver_o) == &PyGen_Type || Py_TYPE(receiver_o) == &PyCoro_Type) && - ((PyGenObject *)receiver_o)->gi_frame_state < FRAME_EXECUTING) + (Py_TYPE(receiver_o) == &PyGen_Type || + Py_TYPE(receiver_o) == &PyCoro_Type)) { _PyInterpreterFrame *gen_frame; PyGenObject *gen = (PyGenObject *)receiver_o; _PyFrame_SetStackPointer(frame, stack_pointer); Py_BEGIN_CRITICAL_SECTION(gen); stack_pointer = _PyFrame_GetStackPointer(frame); - gen_frame = &gen->gi_iframe; - STACK_SHRINK(1); - _PyFrame_StackPush(gen_frame, v); - gen->gi_frame_state = FRAME_EXECUTING; - gen->gi_exc_state.previous_item = tstate->exc_info; - tstate->exc_info = &gen->gi_exc_state; - assert( 2 + oparg <= UINT16_MAX); - frame->return_offset = (uint16_t)( 2 + oparg); - assert(gen_frame->previous == NULL); - gen_frame->previous = frame; + if (gen->gi_frame_state < FRAME_EXECUTING) { + gen_frame = &gen->gi_iframe; + STACK_SHRINK(1); + _PyFrame_StackPush(gen_frame, v); + gen->gi_frame_state = FRAME_EXECUTING; + gen->gi_exc_state.previous_item = tstate->exc_info; + tstate->exc_info = &gen->gi_exc_state; + assert( 2 + oparg <= UINT16_MAX); + frame->return_offset = (uint16_t)( 2 + oparg); + assert(gen_frame->previous == NULL); + gen_frame->previous = frame; + } _PyFrame_SetStackPointer(frame, stack_pointer); Py_END_CRITICAL_SECTION(); stack_pointer = _PyFrame_GetStackPointer(frame); @@ -7177,6 +7196,9 @@ v = stack_pointer[-1]; receiver = stack_pointer[-2]; PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type, SEND); DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, SEND); STAT_INC(SEND, hit); @@ -7188,6 +7210,10 @@ assert( 2 + oparg <= UINT16_MAX); frame->return_offset = (uint16_t)( 2 + oparg); gen_frame->previous = frame; + stack_pointer[-1].bits = (uintptr_t)gen_frame; + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); } // _PUSH_FRAME { @@ -8215,6 +8241,9 @@ #endif frame->instr_ptr++; PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_BEGIN_CRITICAL_SECTION(gen); + stack_pointer = _PyFrame_GetStackPointer(frame); assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1); assert(oparg == 0 || oparg == 1); gen->gi_frame_state = FRAME_SUSPENDED + oparg; @@ -8241,10 +8270,13 @@ stack_pointer = _PyFrame_GetStackPointer(frame); LOAD_IP(1 + INLINE_CACHE_ENTRIES_SEND); value = temp; - LLTRACE_RESUME_FRAME(); stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + Py_END_CRITICAL_SECTION(); + stack_pointer = _PyFrame_GetStackPointer(frame); + LLTRACE_RESUME_FRAME(); DISPATCH(); } #undef TIER_ONE From 2da46782c9b36d8dec8d08ff533e614615610884 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:40:42 -0500 Subject: [PATCH 25/40] Add more to the test. --- .../test_free_threading/test_generators.py | 44 ++++++++++++++----- Python/ceval.c | 2 + 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 24da6e48e15b27..7bb98bcd22187b 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -2,28 +2,48 @@ from unittest import TestCase +from test import support from test.support import threading_helper +from threading import Thread @threading_helper.requires_working_threading() +@support.requires_resource("cpu") class TestGen(TestCase): - def test_generator_send(self): - import threading - + def infinite_generator(self): def gen(): - for _ in range(5_000): + while True: yield + return gen() + + def stress_generator(self, *funcs): + threads = [] + gen = self.infinite_generator() + + for func in funcs: + for _ in range(10): + def with_iterations(gen): + for _ in range(100): + func(gen) - it = gen() - with threading_helper.catch_threading_exception() as cm: - # next(it) is equivalent to it.send(None) - with threading_helper.start_threads( - (threading.Thread(target=lambda: next(it)) for _ in range(10)) - ): - pass + threads.append(Thread(target=with_iterations, args=(gen,))) + + for thread in threads: + thread.start() + + for thread in threads: + thread.join() + + def test_generator_send(self): + self.stress_generator(lambda gen: next(gen)) - self.assertIsNone(cm.exc_value) + def test_generator_close(self): + self.stress_generator(lambda gen: gen.close()) + self.stress_generator(lambda gen: next(gen), lambda gen: gen.close()) + def test_generator_state(self): + self.stress_generator(lambda gen: next(gen), lambda gen: gen.gi_running) + self.stress_generator(lambda gen: gen.gi_isrunning, lambda gen: gen.close()) if __name__ == "__main__": unittest.main() diff --git a/Python/ceval.c b/Python/ceval.c index bfdf5687c287db..5b104e6ff58fda 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1741,6 +1741,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) { assert(frame->owner == FRAME_OWNED_BY_GENERATOR); PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); + Py_BEGIN_CRITICAL_SECTION(gen); gen->gi_frame_state = FRAME_CLEARED; assert(tstate->exc_info == &gen->gi_exc_state); tstate->exc_info = gen->gi_exc_state.previous_item; @@ -1751,6 +1752,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) _PyErr_ClearExcState(&gen->gi_exc_state); tstate->c_recursion_remaining++; frame->previous = NULL; + Py_END_CRITICAL_SECTION(); } void From b34cde6290b9b9ece7f818ab4982abb1574594c7 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:41:21 -0500 Subject: [PATCH 26/40] Add a comment. --- Lib/test/test_free_threading/test_generators.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 7bb98bcd22187b..6e71620fb6dec7 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -28,6 +28,8 @@ def with_iterations(gen): threads.append(Thread(target=with_iterations, args=(gen,))) + # Errors might come up, but that's fine. + # All we care about is that this doesn't crash. for thread in threads: thread.start() From 2d03a136250fd6d73b6304a1deffb693fdadd4b6 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 24 Dec 2024 11:43:24 -0500 Subject: [PATCH 27/40] Prevent logs from getting spammed. --- Lib/test/test_free_threading/test_generators.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 6e71620fb6dec7..cef645f8501ebb 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -28,13 +28,14 @@ def with_iterations(gen): threads.append(Thread(target=with_iterations, args=(gen,))) - # Errors might come up, but that's fine. - # All we care about is that this doesn't crash. - for thread in threads: - thread.start() - - for thread in threads: - thread.join() + with threading_helper.catch_threading_exception(): + # Errors might come up, but that's fine. + # All we care about is that this doesn't crash. + for thread in threads: + thread.start() + + for thread in threads: + thread.join() def test_generator_send(self): self.stress_generator(lambda gen: next(gen)) From 80c3547b477830bdaaa28ef25bb472b0b928d49e Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 11:42:20 -0500 Subject: [PATCH 28/40] Update Objects/genobject.c Co-authored-by: Victor Stinner --- Objects/genobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index b706c1ba865b1d..c2a637f6c7cfc1 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -397,7 +397,7 @@ _PyGen_yf(PyGenObject *gen) /*[clinic input] @critical_section -generator.close as gen_close +generator.close as gen_close_meth raise GeneratorExit inside generator. [clinic start generated code]*/ From ee931930ff0c389964f98dcaf7a260a110ca43ea Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 11:42:49 -0500 Subject: [PATCH 29/40] Rerun clinic. --- Objects/clinic/genobject.c.h | 14 +++++++------- Objects/genobject.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index 908d18de621165..2f2055e1072668 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -4,25 +4,25 @@ preserve #include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() -PyDoc_STRVAR(gen_close__doc__, +PyDoc_STRVAR(gen_close_meth__doc__, "close($self, /)\n" "--\n" "\n" "raise GeneratorExit inside generator."); -#define GEN_CLOSE_METHODDEF \ - {"close", (PyCFunction)gen_close, METH_NOARGS, gen_close__doc__}, +#define GEN_CLOSE_METH_METHODDEF \ + {"close", (PyCFunction)gen_close_meth, METH_NOARGS, gen_close_meth__doc__}, static PyObject * -gen_close_impl(PyGenObject *self); +gen_close_meth_impl(PyGenObject *self); static PyObject * -gen_close(PyGenObject *self, PyObject *Py_UNUSED(ignored)) +gen_close_meth(PyGenObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = gen_close_impl(self); + return_value = gen_close_meth_impl(self); Py_END_CRITICAL_SECTION(); return return_value; @@ -77,4 +77,4 @@ gen_getsuspended_get(PyGenObject *self, void *Py_UNUSED(context)) return return_value; } -/*[clinic end generated code: output=ff4665c12c43063f input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e949b15b48a76555 input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index c2a637f6c7cfc1..7c4ae7dbe75a18 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -403,8 +403,8 @@ raise GeneratorExit inside generator. [clinic start generated code]*/ static PyObject * -gen_close_impl(PyGenObject *self) -/*[clinic end generated code: output=2d7adf450173059c input=417e6161842080c9]*/ +gen_close_meth_impl(PyGenObject *self) +/*[clinic end generated code: output=b6f489da23415c60 input=f90b586ecfb6d2fa]*/ { PyGenObject *gen = _PyGen_CAST(self); From d6cc59a1c90ccb8521096b2fc89a0446d0dcb3ce Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 11:44:37 -0500 Subject: [PATCH 30/40] Rename _gen_throw to gen_throw_lock_held --- Objects/genobject.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 7c4ae7dbe75a18..cb405df3bc8cd7 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -480,7 +480,7 @@ the (type, val, tb) signature is deprecated, \n\ and may be removed in a future version of Python."); static PyObject * -_gen_throw(PyGenObject *gen, int close_on_genexit, +gen_throw_lock_held(PyGenObject *gen, int close_on_genexit, PyObject *typ, PyObject *val, PyObject *tb) { PyObject *yf = _PyGen_yf(gen); @@ -520,7 +520,7 @@ _gen_throw(PyGenObject *gen, int close_on_genexit, 'yield from' or awaiting on with 'await'. */ PyFrameState state = gen->gi_frame_state; gen->gi_frame_state = FRAME_EXECUTING; - ret = _gen_throw((PyGenObject *)yf, close_on_genexit, + ret = gen_throw_lock_held((PyGenObject *)yf, close_on_genexit, typ, val, tb); gen->gi_frame_state = state; tstate->current_frame = prev; @@ -640,7 +640,7 @@ gen_throw(PyGenObject *gen, PyObject *const *args, Py_ssize_t nargs) } PyObject *res; Py_BEGIN_CRITICAL_SECTION(gen); - res = _gen_throw(gen, 1, typ, val, tb); + res = gen_throw_lock_held(gen, 1, typ, val, tb); Py_END_CRITICAL_SECTION(); return res; } @@ -2193,12 +2193,12 @@ async_gen_athrow_send(PyObject *self, PyObject *arg) if (o->agt_args == NULL) { /* aclose() mode */ o->agt_gen->ag_closed = 1; - - retval = _gen_throw((PyGenObject *)gen, + Py_BEGIN_CRITICAL_SECTION(gen); + retval = gen_throw_lock_held((PyGenObject *)gen, 0, /* Do not close generator when PyExc_GeneratorExit is passed */ PyExc_GeneratorExit, NULL, NULL); - + Py_END_CRITICAL_SECTION(); if (retval && _PyAsyncGenWrappedValue_CheckExact(retval)) { Py_DECREF(retval); goto yield_close; @@ -2213,10 +2213,12 @@ async_gen_athrow_send(PyObject *self, PyObject *arg) return NULL; } - retval = _gen_throw((PyGenObject *)gen, + Py_BEGIN_CRITICAL_SECTION(gen); + retval = gen_throw_lock_held((PyGenObject *)gen, 0, /* Do not close generator when PyExc_GeneratorExit is passed */ typ, val, tb); + Py_END_CRITICAL_SECTION(); retval = async_gen_unwrap_value(o->agt_gen, retval); } if (retval == NULL) { From afbb669861894be2a67fd78d8c3c25879c6bb3b6 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 11:57:33 -0500 Subject: [PATCH 31/40] Add helper for gen_close --- Objects/genobject.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index cb405df3bc8cd7..cdd0c28ca4ea91 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -25,6 +25,7 @@ class generator "PyGenObject *" "&PyGen_Type" #include "clinic/genobject.c.h" // Forward declarations +static int gen_close(PyGenObject *gen); static PyObject* async_gen_asend_new(PyAsyncGenObject *, PyObject *); static PyObject* async_gen_athrow_new(PyAsyncGenObject *, PyObject *); @@ -125,15 +126,12 @@ _PyGen_Finalize(PyObject *self) _PyErr_WarnUnawaitedCoroutine((PyObject *)gen); } else { - PyObject *res = gen_close(gen, NULL); - if (res == NULL) { + int res = gen_close(gen); + if (res < 0) { if (PyErr_Occurred()) { PyErr_WriteUnraisable(self); } } - else { - Py_DECREF(res); - } } /* Restore the saved exception. */ @@ -339,10 +337,9 @@ static int gen_close_iter(PyObject *yf) { PyObject *retval = NULL; - if (PyGen_CheckExact(yf) || PyCoro_CheckExact(yf)) { - retval = gen_close((PyGenObject *)yf, NULL); - if (retval == NULL) + int retval = gen_close(_PyGen_CAST(yf)); + if (retval < 0) return -1; } else { @@ -470,6 +467,16 @@ gen_close_meth_impl(PyGenObject *self) return NULL; } +int gen_close(PyGenObject *gen) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(gen); + res = gen_close_meth_impl(gen); + Py_END_CRITICAL_SECTION(); + assert(res == Py_None || res == NULL); + return res == Py_None ? 0 : -1; +} + PyDoc_STRVAR(throw_doc, "throw(value)\n\ throw(type[,value[,tb]])\n\ @@ -891,7 +898,7 @@ PyDoc_STRVAR(sizeof__doc__, static PyMethodDef gen_methods[] = { {"send", gen_send, METH_O, send_doc}, {"throw", _PyCFunction_CAST(gen_throw), METH_FASTCALL, throw_doc}, - GEN_CLOSE_METHODDEF + GEN_CLOSE_METH_METHODDEF {"__sizeof__", (PyCFunction)gen_sizeof, METH_NOARGS, sizeof__doc__}, {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, {NULL, NULL} /* Sentinel */ @@ -1252,7 +1259,7 @@ PyDoc_STRVAR(coro_close_doc, static PyMethodDef coro_methods[] = { {"send", gen_send, METH_O, coro_send_doc}, {"throw",_PyCFunction_CAST(gen_throw), METH_FASTCALL, coro_throw_doc}, - GEN_CLOSE_METHODDEF + GEN_CLOSE_METH_METHODDEF {"__sizeof__", (PyCFunction)gen_sizeof, METH_NOARGS, sizeof__doc__}, {"__class_getitem__", Py_GenericAlias, METH_O|METH_CLASS, PyDoc_STR("See PEP 585")}, {NULL, NULL} /* Sentinel */ @@ -1351,7 +1358,7 @@ static PyObject * coro_wrapper_close(PyObject *self, PyObject *args) { PyCoroWrapper *cw = _PyCoroWrapper_CAST(self); - return gen_close((PyGenObject *)cw->cw_coroutine, args); + return gen_close(_PyGen_CAST(cw->cw_coroutine)) == 0 ? Py_None : NULL; } static int From 6e45597da13401021823e2c0e981120feeeac5b8 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 12:01:08 -0500 Subject: [PATCH 32/40] Rename to gen_getframe_lock_held --- Objects/genobject.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index cdd0c28ca4ea91..0e3cc363fa8e6e 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -825,7 +825,7 @@ gen_getsuspended_get_impl(PyGenObject *self) } static PyObject * -_gen_getframe(PyGenObject *self, const char *name) +gen_getframe_lock_held(PyGenObject *self, const char *name) { PyGenObject *gen = _PyGen_CAST(self); if (PySys_Audit("object.__getattr__", "Os", gen, name) < 0) { @@ -843,7 +843,7 @@ gen_getframe(PyObject *self, void *Py_UNUSED(ignored)) PyGenObject *gen = _PyGen_CAST(self); PyObject *res; Py_BEGIN_CRITICAL_SECTION(gen); - res = _gen_getframe(gen, "gi_frame"); + res = gen_getframe_lock_held(gen, "gi_frame"); Py_END_CRITICAL_SECTION(); return res; } @@ -1210,7 +1210,11 @@ cr_getrunning(PyObject *self, void *Py_UNUSED(ignored)) static PyObject * cr_getframe(PyObject *coro, void *Py_UNUSED(ignored)) { - return _gen_getframe(_PyGen_CAST(coro), "cr_frame"); + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(coro); + res = gen_getframe_lock_held(_PyGen_CAST(coro), "cr_frame"); + Py_END_CRITICAL_SECTION(); + return res; } static PyObject * @@ -1632,7 +1636,11 @@ async_gen_athrow(PyAsyncGenObject *o, PyObject *args) static PyObject * ag_getframe(PyObject *ag, void *Py_UNUSED(ignored)) { - return _gen_getframe((PyGenObject *)ag, "ag_frame"); + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(ag); + res = gen_getframe_lock_held((PyGenObject *)ag, "ag_frame"); + Py_END_CRITICAL_SECTION(); + return res; } static PyObject * From 99105e766ceaf1ebcd5d3880c5c032f69177d98f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 12:10:06 -0500 Subject: [PATCH 33/40] Use argument clinic for some more things. --- Objects/clinic/genobject.c.h | 52 +++++++++++++++++++++++++++++++++++- Objects/genobject.c | 47 +++++++++++++++++++------------- 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index 2f2055e1072668..d59ddce0b171e5 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -77,4 +77,54 @@ gen_getsuspended_get(PyGenObject *self, void *Py_UNUSED(context)) return return_value; } -/*[clinic end generated code: output=e949b15b48a76555 input=a9049054013a1b77]*/ + +#if !defined(generator_gi_frame_DOCSTR) +# define generator_gi_frame_DOCSTR NULL +#endif +#if defined(GENERATOR_GI_FRAME_GETSETDEF) +# undef GENERATOR_GI_FRAME_GETSETDEF +# define GENERATOR_GI_FRAME_GETSETDEF {"gi_frame", (getter)generator_gi_frame_get, (setter)generator_gi_frame_set, generator_gi_frame_DOCSTR}, +#else +# define GENERATOR_GI_FRAME_GETSETDEF {"gi_frame", (getter)generator_gi_frame_get, NULL, generator_gi_frame_DOCSTR}, +#endif + +static PyObject * +generator_gi_frame_get_impl(PyGenObject *self); + +static PyObject * +generator_gi_frame_get(PyGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = generator_gi_frame_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(ag_getsuspended_DOCSTR) +# define ag_getsuspended_DOCSTR NULL +#endif +#if defined(AG_GETSUSPENDED_GETSETDEF) +# undef AG_GETSUSPENDED_GETSETDEF +# define AG_GETSUSPENDED_GETSETDEF {"ag_suspended", (getter)ag_getsuspended_get, (setter)ag_getsuspended_set, ag_getsuspended_DOCSTR}, +#else +# define AG_GETSUSPENDED_GETSETDEF {"ag_suspended", (getter)ag_getsuspended_get, NULL, ag_getsuspended_DOCSTR}, +#endif + +static PyObject * +ag_getsuspended_get_impl(PyAsyncGenObject *self); + +static PyObject * +ag_getsuspended_get(PyAsyncGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = ag_getsuspended_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} +/*[clinic end generated code: output=c08221831ce66479 input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index 0e3cc363fa8e6e..2054ede804629b 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -19,8 +19,9 @@ /*[clinic input] class generator "PyGenObject *" "&PyGen_Type" +class async_generator "PyAsyncGenObject *" "&PyAsyncGen_Type" [clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=a6c98fdc710c6976]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=403b2ee985491847]*/ #include "clinic/genobject.c.h" @@ -810,6 +811,12 @@ gen_getrunning_get_impl(PyGenObject *self) Py_RETURN_FALSE; } +static PyObject * +gen_getsuspended_lock_held(PyGenObject *self) +{ + return PyBool_FromLong(FRAME_STATE_SUSPENDED(self->gi_frame_state)); +} + /*[clinic input] @getter @critical_section @@ -820,8 +827,7 @@ static PyObject * gen_getsuspended_get_impl(PyGenObject *self) /*[clinic end generated code: output=a0345f9be186eda3 input=880e9fb8436726cb]*/ { - PyGenObject *gen = _PyGen_CAST(self); - return PyBool_FromLong(FRAME_STATE_SUSPENDED(gen->gi_frame_state)); + return PyBool_FromLong(FRAME_STATE_SUSPENDED(self->gi_frame_state)); } static PyObject * @@ -837,15 +843,17 @@ gen_getframe_lock_held(PyGenObject *self, const char *name) return _Py_XNewRef((PyObject *)_PyFrame_GetFrameObject(&gen->gi_iframe)); } +/*[clinic input] +@critical_section +@getter +generator.gi_frame +[clinic start generated code]*/ + static PyObject * -gen_getframe(PyObject *self, void *Py_UNUSED(ignored)) +generator_gi_frame_get_impl(PyGenObject *self) +/*[clinic end generated code: output=8e17169979687334 input=70e20e834b852c3a]*/ { - PyGenObject *gen = _PyGen_CAST(self); - PyObject *res; - Py_BEGIN_CRITICAL_SECTION(gen); - res = gen_getframe_lock_held(gen, "gi_frame"); - Py_END_CRITICAL_SECTION(); - return res; + return gen_getframe_lock_held(self, "gi_frame"); } static PyObject * @@ -873,7 +881,7 @@ static PyGetSetDef gen_getsetlist[] = { PyDoc_STR("object being iterated by yield from, or None")}, GEN_GETRUNNING_GETSETDEF GEN_GETSUSPENDED_GETSETDEF - {"gi_frame", gen_getframe, NULL, NULL}, + GENERATOR_GI_FRAME_GETSETDEF {"gi_code", gen_getcode, NULL, NULL}, {NULL} /* Sentinel */ }; @@ -1649,14 +1657,17 @@ ag_getcode(PyObject *gen, void *Py_UNUSED(ignored)) return _gen_getcode((PyGenObject*)gen, "ag_code"); } +/*[clinic input] +@critical_section +@getter +async_generator.ag_suspended as ag_getsuspended +[clinic start generated code]*/ + static PyObject * -ag_getsuspended(PyObject *self, void *Py_UNUSED(ignored)) +ag_getsuspended_get_impl(PyAsyncGenObject *self) +/*[clinic end generated code: output=f9c6d455edce4c50 input=e463d3db950251a3]*/ { - PyAsyncGenObject *ag = _PyAsyncGenObject_CAST(self); - if (FRAME_STATE_SUSPENDED(ag->ag_frame_state)) { - Py_RETURN_TRUE; - } - Py_RETURN_FALSE; + return gen_getsuspended_lock_held(_PyGen_CAST(self)); } static PyGetSetDef async_gen_getsetlist[] = { @@ -1668,7 +1679,7 @@ static PyGetSetDef async_gen_getsetlist[] = { PyDoc_STR("object being awaited on, or None")}, {"ag_frame", ag_getframe, NULL, NULL}, {"ag_code", ag_getcode, NULL, NULL}, - {"ag_suspended", ag_getsuspended, NULL, NULL}, + AG_GETSUSPENDED_GETSETDEF {NULL} /* Sentinel */ }; From 560df731779f84fdc7311f0e3c6280530a36d43d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 12:16:04 -0500 Subject: [PATCH 34/40] Fix some coroutine thread safety. --- Objects/clinic/genobject.c.h | 102 ++++++++++++++++++++++++++++++++++- Objects/genobject.c | 59 +++++++++++++------- 2 files changed, 141 insertions(+), 20 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index d59ddce0b171e5..3cc99f85209671 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -103,6 +103,106 @@ generator_gi_frame_get(PyGenObject *self, void *Py_UNUSED(context)) return return_value; } +#if !defined(cr_getsuspended_DOCSTR) +# define cr_getsuspended_DOCSTR NULL +#endif +#if defined(CR_GETSUSPENDED_GETSETDEF) +# undef CR_GETSUSPENDED_GETSETDEF +# define CR_GETSUSPENDED_GETSETDEF {"cr_suspended", (getter)cr_getsuspended_get, (setter)cr_getsuspended_set, cr_getsuspended_DOCSTR}, +#else +# define CR_GETSUSPENDED_GETSETDEF {"cr_suspended", (getter)cr_getsuspended_get, NULL, cr_getsuspended_DOCSTR}, +#endif + +static PyObject * +cr_getsuspended_get_impl(PyCoroObject *self); + +static PyObject * +cr_getsuspended_get(PyCoroObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = cr_getsuspended_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(cr_getrunning_DOCSTR) +# define cr_getrunning_DOCSTR NULL +#endif +#if defined(CR_GETRUNNING_GETSETDEF) +# undef CR_GETRUNNING_GETSETDEF +# define CR_GETRUNNING_GETSETDEF {"cr_running", (getter)cr_getrunning_get, (setter)cr_getrunning_set, cr_getrunning_DOCSTR}, +#else +# define CR_GETRUNNING_GETSETDEF {"cr_running", (getter)cr_getrunning_get, NULL, cr_getrunning_DOCSTR}, +#endif + +static PyObject * +cr_getrunning_get_impl(PyCoroObject *self); + +static PyObject * +cr_getrunning_get(PyCoroObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = cr_getrunning_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(cr_getframe_DOCSTR) +# define cr_getframe_DOCSTR NULL +#endif +#if defined(CR_GETFRAME_GETSETDEF) +# undef CR_GETFRAME_GETSETDEF +# define CR_GETFRAME_GETSETDEF {"cr_frame", (getter)cr_getframe_get, (setter)cr_getframe_set, cr_getframe_DOCSTR}, +#else +# define CR_GETFRAME_GETSETDEF {"cr_frame", (getter)cr_getframe_get, NULL, cr_getframe_DOCSTR}, +#endif + +static PyObject * +cr_getframe_get_impl(PyCoroObject *self); + +static PyObject * +cr_getframe_get(PyCoroObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = cr_getframe_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + +#if !defined(ag_getframe_DOCSTR) +# define ag_getframe_DOCSTR NULL +#endif +#if defined(AG_GETFRAME_GETSETDEF) +# undef AG_GETFRAME_GETSETDEF +# define AG_GETFRAME_GETSETDEF {"ag_frame", (getter)ag_getframe_get, (setter)ag_getframe_set, ag_getframe_DOCSTR}, +#else +# define AG_GETFRAME_GETSETDEF {"ag_frame", (getter)ag_getframe_get, NULL, ag_getframe_DOCSTR}, +#endif + +static PyObject * +ag_getframe_get_impl(PyAsyncGenObject *self); + +static PyObject * +ag_getframe_get(PyAsyncGenObject *self, void *Py_UNUSED(context)) +{ + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = ag_getframe_get_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; +} + #if !defined(ag_getsuspended_DOCSTR) # define ag_getsuspended_DOCSTR NULL #endif @@ -127,4 +227,4 @@ ag_getsuspended_get(PyAsyncGenObject *self, void *Py_UNUSED(context)) return return_value; } -/*[clinic end generated code: output=c08221831ce66479 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c19d74dfeefdb5db input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index 2054ede804629b..4c98f3da8953a3 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -20,8 +20,9 @@ /*[clinic input] class generator "PyGenObject *" "&PyGen_Type" class async_generator "PyAsyncGenObject *" "&PyAsyncGen_Type" +class coroutine "PyCoroObject *" "&PyCoro_Type" [clinic start generated code]*/ -/*[clinic end generated code: output=da39a3ee5e6b4b0d input=403b2ee985491847]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=593e3f7a2581251e]*/ #include "clinic/genobject.c.h" @@ -1195,8 +1196,15 @@ coro_get_cr_await(PyObject *coro, void *Py_UNUSED(ignored)) return yf; } +/*[clinic input] +@getter +@critical_section +coroutine.cr_suspended as cr_getsuspended +[clinic start generated code]*/ + static PyObject * -cr_getsuspended(PyObject *self, void *Py_UNUSED(ignored)) +cr_getsuspended_get_impl(PyCoroObject *self) +/*[clinic end generated code: output=fa37923084e2a87d input=2581134666285807]*/ { PyCoroObject *coro = _PyCoroObject_CAST(self); if (FRAME_STATE_SUSPENDED(coro->cr_frame_state)) { @@ -1205,8 +1213,15 @@ cr_getsuspended(PyObject *self, void *Py_UNUSED(ignored)) Py_RETURN_FALSE; } +/*[clinic input] +@getter +@critical_section +coroutine.cr_running as cr_getrunning +[clinic start generated code]*/ + static PyObject * -cr_getrunning(PyObject *self, void *Py_UNUSED(ignored)) +cr_getrunning_get_impl(PyCoroObject *self) +/*[clinic end generated code: output=153bd71b7b6e4842 input=9b43ec12b69a9f01]*/ { PyCoroObject *coro = _PyCoroObject_CAST(self); if (coro->cr_frame_state == FRAME_EXECUTING) { @@ -1215,14 +1230,17 @@ cr_getrunning(PyObject *self, void *Py_UNUSED(ignored)) Py_RETURN_FALSE; } +/*[clinic input] +@critical_section +@getter +coroutine.cr_frame as cr_getframe +[clinic start generated code]*/ + static PyObject * -cr_getframe(PyObject *coro, void *Py_UNUSED(ignored)) +cr_getframe_get_impl(PyCoroObject *self) +/*[clinic end generated code: output=300f3facb67ebc50 input=32fca6ffc44085b7]*/ { - PyObject *res; - Py_BEGIN_CRITICAL_SECTION(coro); - res = gen_getframe_lock_held(_PyGen_CAST(coro), "cr_frame"); - Py_END_CRITICAL_SECTION(); - return res; + return gen_getframe_lock_held(_PyGen_CAST(self), "cr_frame"); } static PyObject * @@ -1239,10 +1257,10 @@ static PyGetSetDef coro_getsetlist[] = { PyDoc_STR("qualified name of the coroutine")}, {"cr_await", coro_get_cr_await, NULL, PyDoc_STR("object being awaited on, or None")}, - {"cr_running", cr_getrunning, NULL, NULL}, - {"cr_frame", cr_getframe, NULL, NULL}, + CR_GETRUNNING_GETSETDEF + CR_GETFRAME_GETSETDEF {"cr_code", cr_getcode, NULL, NULL}, - {"cr_suspended", cr_getsuspended, NULL, NULL}, + CR_GETSUSPENDED_GETSETDEF {NULL} /* Sentinel */ }; @@ -1641,14 +1659,17 @@ async_gen_athrow(PyAsyncGenObject *o, PyObject *args) return async_gen_athrow_new(o, args); } +/*[clinic input] +@critical_section +@getter +async_generator.ag_frame as ag_getframe +[clinic start generated code]*/ + static PyObject * -ag_getframe(PyObject *ag, void *Py_UNUSED(ignored)) +ag_getframe_get_impl(PyAsyncGenObject *self) +/*[clinic end generated code: output=7a31c7181090a4fb input=b059ce210436a682]*/ { - PyObject *res; - Py_BEGIN_CRITICAL_SECTION(ag); - res = gen_getframe_lock_held((PyGenObject *)ag, "ag_frame"); - Py_END_CRITICAL_SECTION(); - return res; + return gen_getframe_lock_held(_PyGen_CAST(self), "ag_frame"); } static PyObject * @@ -1677,7 +1698,7 @@ static PyGetSetDef async_gen_getsetlist[] = { PyDoc_STR("qualified name of the async generator")}, {"ag_await", coro_get_cr_await, NULL, PyDoc_STR("object being awaited on, or None")}, - {"ag_frame", ag_getframe, NULL, NULL}, + AG_GETFRAME_GETSETDEF {"ag_code", ag_getcode, NULL, NULL}, AG_GETSUSPENDED_GETSETDEF {NULL} /* Sentinel */ From 7cd787f38d7dfa8d321aedb5a9e3e3f9bf7857b1 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 12:17:36 -0500 Subject: [PATCH 35/40] Fix inconsistency with naming. --- Objects/clinic/genobject.c.h | 20 ++++++++++---------- Objects/genobject.c | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Objects/clinic/genobject.c.h b/Objects/clinic/genobject.c.h index 3cc99f85209671..244822649320a2 100644 --- a/Objects/clinic/genobject.c.h +++ b/Objects/clinic/genobject.c.h @@ -78,26 +78,26 @@ gen_getsuspended_get(PyGenObject *self, void *Py_UNUSED(context)) return return_value; } -#if !defined(generator_gi_frame_DOCSTR) -# define generator_gi_frame_DOCSTR NULL +#if !defined(gen_getframe_DOCSTR) +# define gen_getframe_DOCSTR NULL #endif -#if defined(GENERATOR_GI_FRAME_GETSETDEF) -# undef GENERATOR_GI_FRAME_GETSETDEF -# define GENERATOR_GI_FRAME_GETSETDEF {"gi_frame", (getter)generator_gi_frame_get, (setter)generator_gi_frame_set, generator_gi_frame_DOCSTR}, +#if defined(GEN_GETFRAME_GETSETDEF) +# undef GEN_GETFRAME_GETSETDEF +# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, (setter)gen_getframe_set, gen_getframe_DOCSTR}, #else -# define GENERATOR_GI_FRAME_GETSETDEF {"gi_frame", (getter)generator_gi_frame_get, NULL, generator_gi_frame_DOCSTR}, +# define GEN_GETFRAME_GETSETDEF {"gi_frame", (getter)gen_getframe_get, NULL, gen_getframe_DOCSTR}, #endif static PyObject * -generator_gi_frame_get_impl(PyGenObject *self); +gen_getframe_get_impl(PyGenObject *self); static PyObject * -generator_gi_frame_get(PyGenObject *self, void *Py_UNUSED(context)) +gen_getframe_get(PyGenObject *self, void *Py_UNUSED(context)) { PyObject *return_value = NULL; Py_BEGIN_CRITICAL_SECTION(self); - return_value = generator_gi_frame_get_impl(self); + return_value = gen_getframe_get_impl(self); Py_END_CRITICAL_SECTION(); return return_value; @@ -227,4 +227,4 @@ ag_getsuspended_get(PyAsyncGenObject *self, void *Py_UNUSED(context)) return return_value; } -/*[clinic end generated code: output=c19d74dfeefdb5db input=a9049054013a1b77]*/ +/*[clinic end generated code: output=fb2d54f4bfdabe4f input=a9049054013a1b77]*/ diff --git a/Objects/genobject.c b/Objects/genobject.c index 4c98f3da8953a3..86acfaef44e097 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -847,12 +847,12 @@ gen_getframe_lock_held(PyGenObject *self, const char *name) /*[clinic input] @critical_section @getter -generator.gi_frame +generator.gi_frame as gen_getframe [clinic start generated code]*/ static PyObject * -generator_gi_frame_get_impl(PyGenObject *self) -/*[clinic end generated code: output=8e17169979687334 input=70e20e834b852c3a]*/ +gen_getframe_get_impl(PyGenObject *self) +/*[clinic end generated code: output=69a961dad790fd48 input=0d906f30ab99e1e4]*/ { return gen_getframe_lock_held(self, "gi_frame"); } @@ -882,7 +882,7 @@ static PyGetSetDef gen_getsetlist[] = { PyDoc_STR("object being iterated by yield from, or None")}, GEN_GETRUNNING_GETSETDEF GEN_GETSUSPENDED_GETSETDEF - GENERATOR_GI_FRAME_GETSETDEF + GEN_GETFRAME_GETSETDEF {"gi_code", gen_getcode, NULL, NULL}, {NULL} /* Sentinel */ }; From 77a33bb4b3dfab87fdff44665c2d7f3636241255 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 25 Dec 2024 12:23:47 -0500 Subject: [PATCH 36/40] Add tests for coroutines. --- .../test_free_threading/test_generators.py | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index cef645f8501ebb..8833b0023c9c3f 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -4,6 +4,7 @@ from test import support from test.support import threading_helper +from test.support import import_helper from threading import Thread @threading_helper.requires_working_threading() @@ -16,9 +17,17 @@ def gen(): return gen() - def stress_generator(self, *funcs): + def infinite_coroutine(self): + asyncio = import_helper.import_module("asyncio") + + async def coro(): + while True: + await asyncio.sleep(0) + + return coro() + + def _stress_genlike(self, gen, *funcs): threads = [] - gen = self.infinite_generator() for func in funcs: for _ in range(10): @@ -37,6 +46,12 @@ def with_iterations(gen): for thread in threads: thread.join() + def stress_generator(self, *funcs): + self._stress_genlike(self.infinite_generator(), *funcs) + + def stress_coroutine(self, *funcs): + self._stress_genlike(self.infinite_coroutine(), *funcs) + def test_generator_send(self): self.stress_generator(lambda gen: next(gen)) @@ -48,5 +63,23 @@ def test_generator_state(self): self.stress_generator(lambda gen: next(gen), lambda gen: gen.gi_running) self.stress_generator(lambda gen: gen.gi_isrunning, lambda gen: gen.close()) + def test_coroutine_send(self): + def try_send_non_none(coro): + try: + coro.send(42) + except ValueError: + # Can't send non-None to just started coroutine + coro.send(None) + + self.stress_coroutine(lambda coro: next(coro)) + self.stress_coroutine(try_send_non_none) + + def test_coroutine_attributes(self): + self.stress_coroutine( + lambda coro: next(coro), + lambda coro: coro.cr_frame, + lambda coro: coro.cr_suspended, + ) + if __name__ == "__main__": unittest.main() From 173ba3b660f114fe015450a3566bcc5a46d25bd2 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 28 Dec 2024 10:13:34 -0500 Subject: [PATCH 37/40] Add some locking assertions. --- Objects/genobject.c | 48 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 86acfaef44e097..2a03558c3c9ead 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -184,6 +184,7 @@ static PySendResult gen_send_ex2_lock_held(PyGenObject *gen, PyObject *arg, PyObject **presult, int exc, int closing) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); PyThreadState *tstate = _PyThreadState_GET(); _PyInterpreterFrame *frame = &gen->gi_iframe; @@ -374,6 +375,7 @@ is_resume(_Py_CODEUNIT *instr) static PyObject * _PyGen_yf_lock_held(PyGenObject *gen) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) { _PyInterpreterFrame *frame = &gen->gi_iframe; // GH-122390: These asserts are wrong in the presence of ENTER_EXECUTOR! @@ -406,6 +408,7 @@ gen_close_meth_impl(PyGenObject *self) /*[clinic end generated code: output=b6f489da23415c60 input=f90b586ecfb6d2fa]*/ { PyGenObject *gen = _PyGen_CAST(self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); if (gen->gi_frame_state == FRAME_CREATED) { gen->gi_frame_state = FRAME_COMPLETED; @@ -492,6 +495,7 @@ static PyObject * gen_throw_lock_held(PyGenObject *gen, int close_on_genexit, PyObject *typ, PyObject *val, PyObject *tb) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); PyObject *yf = _PyGen_yf(gen); if (yf) { @@ -529,8 +533,10 @@ gen_throw_lock_held(PyGenObject *gen, int close_on_genexit, 'yield from' or awaiting on with 'await'. */ PyFrameState state = gen->gi_frame_state; gen->gi_frame_state = FRAME_EXECUTING; + Py_BEGIN_CRITICAL_SECTION(yf); ret = gen_throw_lock_held((PyGenObject *)yf, close_on_genexit, typ, val, tb); + Py_END_CRITICAL_SECTION(); gen->gi_frame_state = state; tstate->current_frame = prev; frame->previous = NULL; @@ -815,6 +821,7 @@ gen_getrunning_get_impl(PyGenObject *self) static PyObject * gen_getsuspended_lock_held(PyGenObject *self) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); return PyBool_FromLong(FRAME_STATE_SUSPENDED(self->gi_frame_state)); } @@ -835,6 +842,7 @@ static PyObject * gen_getframe_lock_held(PyGenObject *self, const char *name) { PyGenObject *gen = _PyGen_CAST(self); + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); if (PySys_Audit("object.__getattr__", "Os", gen, name) < 0) { return NULL; } @@ -854,6 +862,7 @@ static PyObject * gen_getframe_get_impl(PyGenObject *self) /*[clinic end generated code: output=69a961dad790fd48 input=0d906f30ab99e1e4]*/ { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); return gen_getframe_lock_held(self, "gi_frame"); } @@ -1240,6 +1249,7 @@ static PyObject * cr_getframe_get_impl(PyCoroObject *self) /*[clinic end generated code: output=300f3facb67ebc50 input=32fca6ffc44085b7]*/ { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); return gen_getframe_lock_held(_PyGen_CAST(self), "cr_frame"); } @@ -1669,6 +1679,7 @@ static PyObject * ag_getframe_get_impl(PyAsyncGenObject *self) /*[clinic end generated code: output=7a31c7181090a4fb input=b059ce210436a682]*/ { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); return gen_getframe_lock_held(_PyGen_CAST(self), "ag_frame"); } @@ -1688,6 +1699,7 @@ static PyObject * ag_getsuspended_get_impl(PyAsyncGenObject *self) /*[clinic end generated code: output=f9c6d455edce4c50 input=e463d3db950251a3]*/ { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); return gen_getsuspended_lock_held(_PyGen_CAST(self)); } @@ -2188,8 +2200,9 @@ async_gen_athrow_traverse(PyObject *self, visitproc visit, void *arg) static PyObject * -async_gen_athrow_send(PyObject *self, PyObject *arg) +async_gen_athrow_send_lock_held(PyObject *self, PyObject *arg) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); PyAsyncGenAThrow *o = _PyAsyncGenAThrow_CAST(self); PyGenObject *gen = _PyGen_CAST(o->agt_gen); PyObject *retval; @@ -2321,10 +2334,20 @@ async_gen_athrow_send(PyObject *self, PyObject *arg) return NULL; } +static PyObject * +async_gen_athrow_send(PyObject *self, PyObject *arg) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(self); + res = async_gen_athrow_send_lock_held(self, arg); + Py_END_CRITICAL_SECTION(); + return res; +} static PyObject * -async_gen_athrow_throw(PyObject *self, PyObject *const *args, Py_ssize_t nargs) +async_gen_athrow_throw_lock_held(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); PyAsyncGenAThrow *o = _PyAsyncGenAThrow_CAST(self); if (o->agt_state == AWAITABLE_STATE_CLOSED) { @@ -2391,6 +2414,15 @@ async_gen_athrow_throw(PyObject *self, PyObject *const *args, Py_ssize_t nargs) } } +static PyObject * +async_gen_athrow_throw(PyObject *self, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(self); + res = async_gen_athrow_throw_lock_held(self, args, nargs); + Py_END_CRITICAL_SECTION(); + return res; +} static PyObject * async_gen_athrow_iternext(PyObject *agt) @@ -2400,8 +2432,9 @@ async_gen_athrow_iternext(PyObject *agt) static PyObject * -async_gen_athrow_close(PyObject *self, PyObject *args) +async_gen_athrow_close_lock_held(PyObject *self, PyObject *args) { + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self); PyAsyncGenAThrow *agt = _PyAsyncGenAThrow_CAST(self); if (agt->agt_state == AWAITABLE_STATE_CLOSED) { Py_RETURN_NONE; @@ -2424,6 +2457,15 @@ async_gen_athrow_close(PyObject *self, PyObject *args) } } +static PyObject * +async_gen_athrow_close(PyObject *self, PyObject *args) +{ + PyObject *res; + Py_BEGIN_CRITICAL_SECTION(self); + res = async_gen_athrow_close_lock_held(self, args); + Py_END_CRITICAL_SECTION(); + return res; +} static void async_gen_athrow_finalize(PyAsyncGenAThrow *o) From 770ce0912b3e350c2204a0a947979280f3606713 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 28 Dec 2024 11:39:06 -0500 Subject: [PATCH 38/40] Don't try and re-lock the generator to clear it. --- Python/ceval.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 5b104e6ff58fda..15801336a4844e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1741,7 +1741,12 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) { assert(frame->owner == FRAME_OWNED_BY_GENERATOR); PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); - Py_BEGIN_CRITICAL_SECTION(gen); + /* It's safe to assume that the generator is locked, because this + * was eventually called by something that locked it already. + * + * In fact, trying to lock the generator here ends up breaking + * the critical section API. */ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); gen->gi_frame_state = FRAME_CLEARED; assert(tstate->exc_info == &gen->gi_exc_state); tstate->exc_info = gen->gi_exc_state.previous_item; @@ -1752,7 +1757,6 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) _PyErr_ClearExcState(&gen->gi_exc_state); tstate->c_recursion_remaining++; frame->previous = NULL; - Py_END_CRITICAL_SECTION(); } void From 81e90bfe54755259bde65f940a8ae26bf6cf9e5a Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sat, 28 Dec 2024 11:49:32 -0500 Subject: [PATCH 39/40] Add tests for async generators. --- .../test_free_threading/test_generators.py | 45 +++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 8833b0023c9c3f..a76aef7e69b684 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -6,10 +6,11 @@ from test.support import threading_helper from test.support import import_helper from threading import Thread +import types @threading_helper.requires_working_threading() @support.requires_resource("cpu") -class TestGen(TestCase): +class GeneratorFreeThreadingTests(TestCase): def infinite_generator(self): def gen(): while True: @@ -17,16 +18,32 @@ def gen(): return gen() - def infinite_coroutine(self): + def infinite_coroutine(self, from_gen: bool = False): asyncio = import_helper.import_module("asyncio") - async def coro(): + if from_gen: + @types.coroutine + def coro(): + while True: + yield from asyncio.sleep(0) + else: + async def coro(): + while True: + await asyncio.sleep(0) + + return coro() + + def infinite_asyncgen(self): + asyncio = import_helper.import_module("asyncio") + + async def async_gen(): while True: await asyncio.sleep(0) + yield 42 - return coro() + return async_gen() - def _stress_genlike(self, gen, *funcs): + def _stress_object(self, gen, *funcs): threads = [] for func in funcs: @@ -47,10 +64,13 @@ def with_iterations(gen): thread.join() def stress_generator(self, *funcs): - self._stress_genlike(self.infinite_generator(), *funcs) + self._stress_object(self.infinite_generator(), *funcs) + + def stress_coroutine(self, *funcs, from_gen: bool = False): + self._stress_object(self.infinite_coroutine(from_gen=from_gen), *funcs) - def stress_coroutine(self, *funcs): - self._stress_genlike(self.infinite_coroutine(), *funcs) + def stress_asyncgen(self, *funcs): + self._stress_object(self.infinite_asyncgen(), *funcs) def test_generator_send(self): self.stress_generator(lambda gen: next(gen)) @@ -81,5 +101,14 @@ def test_coroutine_attributes(self): lambda coro: coro.cr_suspended, ) + def test_generator_coroutine(self): + self.stress_coroutine(lambda coro: next(coro), from_gen=True) + + def test_async_gen(self): + self.stress_asyncgen(lambda ag: next(ag), lambda ag: ag.send(None)) + self.stress_asyncgen(lambda ag: next(ag), lambda ag: ag.throw(RuntimeError)) + self.stress_asyncgen(lambda ag: next(ag), lambda ag: ag.close()) + self.stress_asyncgen(lambda ag: ag.throw(RuntimeError), lambda ag: ag.close()) + if __name__ == "__main__": unittest.main() From 415f0819ebf27ba742e8211d90f2f615951e32d3 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 30 Dec 2024 16:40:35 -0500 Subject: [PATCH 40/40] Revert "Don't try and re-lock the generator to clear it." This reverts commit 770ce0912b3e350c2204a0a947979280f3606713. --- Python/ceval.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 15801336a4844e..5b104e6ff58fda 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1741,12 +1741,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) { assert(frame->owner == FRAME_OWNED_BY_GENERATOR); PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame); - /* It's safe to assume that the generator is locked, because this - * was eventually called by something that locked it already. - * - * In fact, trying to lock the generator here ends up breaking - * the critical section API. */ - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(gen); + Py_BEGIN_CRITICAL_SECTION(gen); gen->gi_frame_state = FRAME_CLEARED; assert(tstate->exc_info == &gen->gi_exc_state); tstate->exc_info = gen->gi_exc_state.previous_item; @@ -1757,6 +1752,7 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame) _PyErr_ClearExcState(&gen->gi_exc_state); tstate->c_recursion_remaining++; frame->previous = NULL; + Py_END_CRITICAL_SECTION(); } void