Skip to content

Commit

Permalink
Test that Trio works around some CPython .throw bugs present in >= 3.9
Browse files Browse the repository at this point in the history
The current tests that ensure Trio works around CPython .throw bugs are
all for .throw bugs that were fixed in 3.9.0 or earlier. However .throw
is still buggy in every CPython release supported by Trio today.
  • Loading branch information
gschaffner committed Aug 30, 2023
1 parent a15d0f9 commit 6784759
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 13 deletions.
4 changes: 3 additions & 1 deletion src/trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2546,9 +2546,11 @@ def unrolled_run(
try:
# We used to unwrap the Outcome object here and send/throw
# its contents in directly, but it turns out that .throw()
# is buggy, at least before CPython 3.9:
# is buggy on CPython (all versions at time of writing):
# https://bugs.python.org/issue29587
# https://bugs.python.org/issue29590
# https://bugs.python.org/issue40694
# https://github.com/python/cpython/issues/108668
# So now we send in the Outcome object and unwrap it on the
# other side.
msg = task.context.run(next_send_fn, next_send)
Expand Down
103 changes: 91 additions & 12 deletions src/trio/_core/_tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -1074,12 +1074,19 @@ async def child2() -> None:
]


# Before CPython 3.9, using .throw() to raise an exception inside a
# coroutine/generator causes the original exc_info state to be lost, so things
# like re-raising and exception chaining are broken.
# On all CPython versions (at time of writing), using .throw() to raise an
# exception inside a coroutine/generator can cause the original `exc_info` state
# to be lost, so things like re-raising and exception chaining are broken unless
# Trio implements its workaround. At time of writing, CPython main (3.13-dev)
# and every CPython release (excluding releases for old Python versions not
# supported by Trio) is affected (albeit in differing ways).
#
# https://bugs.python.org/issue29587
async def test_exc_info_after_yield_error() -> None:
# If the `ValueError()` gets sent in via `throw` and is suppressed, then CPython
# loses track of the original `exc_info`:
# https://bugs.python.org/issue25612 (Example 1)
# https://bugs.python.org/issue29587 (Example 2)
# This is fixed in CPython >= 3.7.
async def test_exc_info_after_throw_suppressed() -> None:
child_task: _core.Task | None = None

async def child() -> None:
Expand All @@ -1088,21 +1095,28 @@ async def child() -> None:

try:
raise KeyError
except Exception:
with suppress(Exception):
except KeyError:
with suppress(ValueError):
await sleep_forever()
raise

with pytest.raises(KeyError):
with pytest.raises(KeyError) as excinfo:
async with _core.open_nursery() as nursery:
nursery.start_soon(child)
await wait_all_tasks_blocked()
_core.reschedule(not_none(child_task), outcome.Error(ValueError()))

assert excinfo.value.__context__ is None


# Similar to previous test -- if the ValueError() gets sent in via 'throw',
# then Python's normal implicit chaining stuff is broken.
async def test_exception_chaining_after_yield_error() -> None:
# Similar to previous test -- if the `ValueError()` gets sent in via 'throw' and
# propagates out, then CPython doesn't set its `__context__` so normal implicit
# exception chaining is broken:
# https://bugs.python.org/issue25612 (Example 2)
# https://bugs.python.org/issue25683
# https://bugs.python.org/issue29587 (Example 1)
# This is fixed in CPython >= 3.9.
async def test_exception_chaining_after_throw() -> None:
child_task: _core.Task | None = None

async def child() -> None:
Expand All @@ -1111,7 +1125,7 @@ async def child() -> None:

try:
raise KeyError
except Exception:
except KeyError:
await sleep_forever()

with pytest.raises(ValueError) as excinfo:
Expand All @@ -1123,6 +1137,71 @@ async def child() -> None:
assert isinstance(excinfo.value.__context__, KeyError)


# Similar to previous tests -- if the `ValueError()` gets sent into an inner
# `await` via 'throw' and is suppressed there, then CPython loses track of
# `exc_info` in the inner coroutine:
# https://github.com/python/cpython/issues/108668
# This is unfixed in CPython at time of writing.
async def test_exc_info_after_throw_to_inner_suppressed() -> None:
child_task: _core.Task | None = None

async def child() -> None:
nonlocal child_task
child_task = _core.current_task()

try:
raise KeyError
except KeyError as exc:
await inner(exc)
raise

async def inner(exc: BaseException) -> None:
with suppress(ValueError):
await sleep_forever()
assert not_none(sys.exc_info()[1]) is exc

with pytest.raises(KeyError) as excinfo:
async with _core.open_nursery() as nursery:
nursery.start_soon(child)
await wait_all_tasks_blocked()
_core.reschedule(not_none(child_task), outcome.Error(ValueError()))

assert excinfo.value.__context__ is None


# Similar to previous tests -- if the `ValueError()` gets sent into an inner
# `await` via `throw` and propagates out, then CPython incorrectly sets its
# `__context__` so normal implicit exception chaining is broken:
# https://bugs.python.org/issue40694
# This is unfixed in CPython at time of writing.
async def test_exception_chaining_after_throw_to_inner() -> None:
child_task: _core.Task | None = None

async def child() -> None:
nonlocal child_task
child_task = _core.current_task()

try:
raise KeyError
except KeyError:
await inner()

async def inner() -> None:
try:
raise IndexError
except IndexError:
await sleep_forever()

with pytest.raises(ValueError) as excinfo:
async with _core.open_nursery() as nursery:
nursery.start_soon(child)
await wait_all_tasks_blocked()
_core.reschedule(not_none(child_task), outcome.Error(ValueError()))

assert isinstance(excinfo.value.__context__, IndexError)
assert isinstance(excinfo.value.__context__.__context__, KeyError)


async def test_nursery_exception_chaining_doesnt_make_context_loops() -> None:
async def crasher() -> NoReturn:
raise KeyError
Expand Down

0 comments on commit 6784759

Please sign in to comment.