return in finally swallows exceptions

[iritkatriel]

Things to check first

• I have searched the existing issues and didn't find my bug already reported there

• I have checked that my bug is still present in the latest release

AnyIO version

mater branch

Python version

NA

What happened?

In

anyio/src/anyio/from_thread.py

Line 119 in 3a62738

return result

there is a return statement in a finally block, which would swallow any in-flight exception.

This means that if an unhandled exception (including a BaseException such as KeyboardInterrupt) is raised from the try body, it will not propagate on as expected.

If the intention is to suppress all exceptions, I would propose to make this clear by using except BaseException.

See also https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions.

How can we reproduce the bug?

NA

[agronholm]

Note to self: this can be found in anyio.from_thread._BlockingAsyncContextManager.run_async_cm().

[jakkdl]

this now also gives a SyntaxWarning for downstream users testing on py314

[agronholm]

This is in contributed code and I've looked at it a couple times but didn't know what to make of it – that is, why it's there. Feel free to send a PR if you know how to deal with it.

[jakkdl]

EDIT: ~all of this is wrong, I made some incorrect assumptions when testing

It appears that the exception suppression is intended, otherwise this test fails

anyio/tests/test_from_thread.py

Lines 494 to 502 in 9c876b1

	def test_async_context_manager_error_ignore(
	self, anyio_backend_name: str, anyio_backend_options: dict[str, Any]
	) -> None:
	with start_blocking_portal(anyio_backend_name, anyio_backend_options) as portal:
	with portal.wrap_async_context_manager(
	TestBlockingPortal.AsyncCM(True)
	) as cm:
	assert cm == "test"
	raise Exception("should be ignored")

which was added in #191

But the test only checks for Exception, so even with exception suppression being intended there should probably be logic that excludes BaseException (or all BaseExceptions except get_cancelled_exc_class()?)

[jakkdl]

nvm what I said above, I did my testing in a bad way and confused different error suppression things.

What #382 addressed was _async_cm.__aexit__ never being called when the _exit_event got cancelled, and __aexit__ should clearly be cancelled no matter the exception raised.
If I change the code to stop suppressing exceptions no tests are impacted, any raised cancellations get handled by the TaskGroup in the BlockingPortal. I'm not sure how to emulate a KeyboardInterrupt being raised in self._exit_event.wait() in a test, but it seems Bad™️ to suppress.

tl;dr, I propose doing the easy fix of moving return result to after the finally.

        try:
            # Wait for the sync context manager to exit.
            # This next statement can raise `get_cancelled_exc_class()` if
            # something went wrong in a task group in this async context
            # manager.
            await self._exit_event.wait()
        finally:
            # In case of cancellation, it could be that we end up here before
            # `_BlockingAsyncContextManager.__exit__` is called, and an
            # `_exit_exc_info` has been set.
            result = await self._async_cm.__aexit__(*self._exit_exc_info)
        return result