Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spurious error when asyncio.run() task raises SystemExit #149

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions newsfragments/149.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trio-asyncio no longer raises a spurious "Event loop stopped before Future
completed!" exception if a function passed to :func:`asyncio.run` calls
:func:`sys.exit`.
10 changes: 10 additions & 0 deletions tests/test_trio_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,13 @@ def noop():
await trio_asyncio.aio_as_trio(loop.run_in_executor)(executor, noop)

assert not scope.cancelled_caught


def test_system_exit():
async def main():
raise SystemExit(42)

with pytest.raises(SystemExit) as scope:
asyncio.run(main())

assert scope.value.code == 42
8 changes: 8 additions & 0 deletions trio_asyncio/_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ def is_done(_):
nonlocal result

result = outcome.capture(future.result)
if isinstance(result, outcome.Error) and isinstance(
result.error, (SystemExit, KeyboardInterrupt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at the exception hierarchy, do you need to include GeneratorExit here too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No; this is trying to match logic in asyncio where specifically these two exceptions will come out of the event loop if raised by a task.

asyncio/tasks.py:

class Task(...):
    ...
    def __step_run_and_handle_result(self, exc):
        coro = self._coro
        try:
            if exc is None:
                # We use the `send` method directly, because coroutines
                # don't have `__iter__` and `__next__` methods.
                result = coro.send(None)
            else:
                result = coro.throw(exc)
        ...
        except (KeyboardInterrupt, SystemExit) as exc:
            super().set_exception(exc)
            raise
        except BaseException as exc:
            super().set_exception(exc)
        else:
            ...

):
# These exceptions propagate out of the event loop;
# don't stop the event loop again, or else it will
# interfere with cleanup actions like
# run_until_complete(shutdown_asyncgens)
return
self.stop()

future.add_done_callback(is_done)
Expand Down
Loading