From 29f821067190535eb9a3fe4ae50474752f996e37 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 00:40:41 -0700 Subject: [PATCH 1/7] Don't crash on C coroutine objects Fixes: gh-550, gh-1191 --- newsfragments/1191.bugfix.rst | 3 +++ newsfragments/550.bugfix.rst | 3 +++ trio/_core/_run.py | 7 ++++++- trio/_core/tests/test_run.py | 17 +++++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 newsfragments/1191.bugfix.rst create mode 100644 newsfragments/550.bugfix.rst diff --git a/newsfragments/1191.bugfix.rst b/newsfragments/1191.bugfix.rst new file mode 100644 index 0000000000..65095542f7 --- /dev/null +++ b/newsfragments/1191.bugfix.rst @@ -0,0 +1,3 @@ +Trio no longer crashes when an async function is implemented in C or +Cython and then passed directly to `trio.run` or +``nursery.start_soon``. diff --git a/newsfragments/550.bugfix.rst b/newsfragments/550.bugfix.rst new file mode 100644 index 0000000000..65095542f7 --- /dev/null +++ b/newsfragments/550.bugfix.rst @@ -0,0 +1,3 @@ +Trio no longer crashes when an async function is implemented in C or +Cython and then passed directly to `trio.run` or +``nursery.start_soon``. diff --git a/trio/_core/_run.py b/trio/_core/_run.py index d662d0eb52..af2338b208 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -1355,11 +1355,16 @@ def _return_value_looks_like_wrong_library(value): name=name, context=context, ) - self.tasks.add(task) + if not hasattr(coro, "cr_frame"): + # This async function is implemented in C or Cython + async def python_wrapper(coro): + return await coro + coro = python_wrapper(coro) coro.cr_frame.f_locals.setdefault( LOCALS_KEY_KI_PROTECTION_ENABLED, system_task ) + self.tasks.add(task) if nursery is not None: nursery._children.add(task) task._activate_cancel_status(nursery._cancel_status) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index f1ca40f4d4..f5650ff6c2 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -2389,3 +2389,20 @@ def abort_fn(_): task.coro.send(None) assert abort_fn_called + + +def test_async_function_implemented_in_C(): + # These used to crash because we'd try to mutate the coroutine object's + # cr_frame, but C functions don't have Python frames. + async def agen_fn(): + yield "hi" + + agen = agen_fn() + assert _core.run(agen.__anext__) == "hi" + + async def main(): + agen = agen_fn() + async with _core.open_nursery() as nursery: + nursery.start_soon(agen.__anext__) + + _core.run(main) From c17178ced2598aa765f323ca8c4016bff1c40783 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 00:42:52 -0700 Subject: [PATCH 2/7] rename variable to avoid confusing shadowing --- trio/_core/_run.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index af2338b208..7b04499742 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -1357,8 +1357,8 @@ def _return_value_looks_like_wrong_library(value): ) if not hasattr(coro, "cr_frame"): # This async function is implemented in C or Cython - async def python_wrapper(coro): - return await coro + async def python_wrapper(orig_coro): + return await orig_coro coro = python_wrapper(coro) coro.cr_frame.f_locals.setdefault( LOCALS_KEY_KI_PROTECTION_ENABLED, system_task From 851c11dec005ef749b582226b32697b0c49d0967 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 16:34:46 -0700 Subject: [PATCH 3/7] yapf --- trio/_core/_run.py | 1 + 1 file changed, 1 insertion(+) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index 7b04499742..e7b5cb0d4d 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -1359,6 +1359,7 @@ def _return_value_looks_like_wrong_library(value): # This async function is implemented in C or Cython async def python_wrapper(orig_coro): return await orig_coro + coro = python_wrapper(coro) coro.cr_frame.f_locals.setdefault( LOCALS_KEY_KI_PROTECTION_ENABLED, system_task From 271a96ae7e3f6a14ce7f2aabe1447262201fc24d Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 16:34:52 -0700 Subject: [PATCH 4/7] don't crash on py35 --- trio/_core/tests/test_run.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index f5650ff6c2..5728b5627e 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -2394,8 +2394,14 @@ def abort_fn(_): def test_async_function_implemented_in_C(): # These used to crash because we'd try to mutate the coroutine object's # cr_frame, but C functions don't have Python frames. - async def agen_fn(): - yield "hi" + + ns = {} + try: + exec("async def agen_fn():\n yield 'hi'", ns) + except SyntaxError: + pytest.skip("Requires Python 3.6+") + else: + agen_fn = ns["agen_fn"] agen = agen_fn() assert _core.run(agen.__anext__) == "hi" From c56adc5613c8a79588c02e7600414c4dfd92a858 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 18:00:47 -0700 Subject: [PATCH 5/7] Fix massive bug revealed by coverage --- trio/_core/_run.py | 15 ++++++++------- trio/_core/tests/test_run.py | 10 ++++++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/trio/_core/_run.py b/trio/_core/_run.py index e7b5cb0d4d..b3335ae27e 100644 --- a/trio/_core/_run.py +++ b/trio/_core/_run.py @@ -1348,13 +1348,6 @@ def _return_value_looks_like_wrong_library(value): else: context = copy_context() - task = Task( - coro=coro, - parent_nursery=nursery, - runner=self, - name=name, - context=context, - ) if not hasattr(coro, "cr_frame"): # This async function is implemented in C or Cython async def python_wrapper(orig_coro): @@ -1365,6 +1358,14 @@ async def python_wrapper(orig_coro): LOCALS_KEY_KI_PROTECTION_ENABLED, system_task ) + task = Task( + coro=coro, + parent_nursery=nursery, + runner=self, + name=name, + context=context, + ) + self.tasks.add(task) if nursery is not None: nursery._children.add(task) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 5728b5627e..c865b1e1d2 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -8,6 +8,7 @@ import warnings from contextlib import contextmanager, ExitStack from math import inf +from textwrap import dedent import attr import outcome @@ -2395,9 +2396,14 @@ def test_async_function_implemented_in_C(): # These used to crash because we'd try to mutate the coroutine object's # cr_frame, but C functions don't have Python frames. - ns = {} + ns = {"_core": _core} try: - exec("async def agen_fn():\n yield 'hi'", ns) + exec(dedent(""" + async def agen_fn(): + assert not _core.currently_ki_protected() + yield 'hi' + """), + ns) except SyntaxError: pytest.skip("Requires Python 3.6+") else: From 8da091d35301bc5eeb56eab23737f9c7df675e13 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 18:02:09 -0700 Subject: [PATCH 6/7] yapf --- trio/_core/tests/test_run.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index c865b1e1d2..8c72a03ff5 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -2398,12 +2398,15 @@ def test_async_function_implemented_in_C(): ns = {"_core": _core} try: - exec(dedent(""" + exec( + dedent( + """ async def agen_fn(): assert not _core.currently_ki_protected() yield 'hi' - """), - ns) + """ + ), ns + ) except SyntaxError: pytest.skip("Requires Python 3.6+") else: From c8f51fe0a58984514e2d22ddabfd053212da48f9 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 17 Aug 2019 21:00:07 -0700 Subject: [PATCH 7/7] Check that nursery.start_soon actually runs the C async function --- trio/_core/tests/test_run.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/trio/_core/tests/test_run.py b/trio/_core/tests/test_run.py index 8c72a03ff5..09aea60eb9 100644 --- a/trio/_core/tests/test_run.py +++ b/trio/_core/tests/test_run.py @@ -2401,23 +2401,29 @@ def test_async_function_implemented_in_C(): exec( dedent( """ - async def agen_fn(): - assert not _core.currently_ki_protected() - yield 'hi' - """ - ), ns + async def agen_fn(record): + assert not _core.currently_ki_protected() + record.append("the generator ran") + yield + """ + ), + ns, ) except SyntaxError: pytest.skip("Requires Python 3.6+") else: agen_fn = ns["agen_fn"] - agen = agen_fn() - assert _core.run(agen.__anext__) == "hi" + run_record = [] + agen = agen_fn(run_record) + _core.run(agen.__anext__) + assert run_record == ["the generator ran"] async def main(): - agen = agen_fn() + start_soon_record = [] + agen = agen_fn(start_soon_record) async with _core.open_nursery() as nursery: nursery.start_soon(agen.__anext__) + assert start_soon_record == ["the generator ran"] _core.run(main)