Skip to content

gh-88760: fix (Async)ExitStack handling of __context__ #27089

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.

Sign up for GitHub

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

Merged
njsmith merged 4 commits into from
Oct 4, 2021
Merged

gh-88760: fix (Async)ExitStack handling of __context__ #27089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b94522
bpo-44594: fix (Async)ExitStack handling of __context__
belm0 Jul 12, 2021
db2f55b
unit tests
belm0 Jul 12, 2021
d5f7cbe
📜🤖 Added by blurb_it.
blurb-it[bot] Jul 12, 2021
c70f7e2
fix rst warning
belm0 Jul 12, 2021
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
8 changes: 4 additions & 4 deletions Lib/contextlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,10 @@ def _fix_exception_context(new_exc, old_exc):
# Context may not be correct, so find the end of the chain
while 1:
exc_context = new_exc.__context__
if exc_context is old_exc:
if exc_context is None or exc_context is old_exc:
# Context is already set correctly (see issue 20317)
return
if exc_context is None or exc_context is frame_exc:
if exc_context is frame_exc:
Comment on lines -534 to +537
Copy link
Copy Markdown
Contributor Author

@belm0 belm0 Jul 12, 2021

Choose a reason for hiding this comment

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

I believe that an exception raised with __context__ of None will already have __context__ populated before ExitStack receives it. Therefore this existing handling of None was not necessary and not handling the intended case.

When __context__ is None here, it means that it was explicitly set this way at the raise site (via a finally block). So we should not override it to old_exc. This is in line with the behavior of actual nested with statements.

break
new_exc = exc_context
# Change the end of the chain to point to the exception
Expand Down Expand Up @@ -671,10 +671,10 @@ def _fix_exception_context(new_exc, old_exc):
# Context may not be correct, so find the end of the chain
while 1:
exc_context = new_exc.__context__
if exc_context is old_exc:
if exc_context is None or exc_context is old_exc:
# Context is already set correctly (see issue 20317)
return
if exc_context is None or exc_context is frame_exc:
if exc_context is frame_exc:
break
new_exc = exc_context
# Change the end of the chain to point to the exception
Expand Down
34 changes: 34 additions & 0 deletions Lib/test/test_contextlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,40 @@ def suppress_exc(*exc_details):
self.assertIsInstance(inner_exc, ValueError)
self.assertIsInstance(inner_exc.__context__, ZeroDivisionError)

def test_exit_exception_explicit_none_context(self):
# Ensure ExitStack chaining matches actual nested `with` statements
# regarding explicit __context__ = None.

class MyException(Exception):
pass

@contextmanager
def my_cm():
try:
yield
except BaseException:
exc = MyException()
try:
raise exc
finally:
exc.__context__ = None

@contextmanager
def my_cm_with_exit_stack():
with self.exit_stack() as stack:
stack.enter_context(my_cm())
yield stack

for cm in (my_cm, my_cm_with_exit_stack):
with self.subTest():
try:
with cm():
raise IndexError()
except MyException as exc:
self.assertIsNone(exc.__context__)
else:
self.fail("Expected IndexError, but no exception was raised")

def test_exit_exception_non_suppressing(self):
# http://bugs.python.org/issue19092
def raise_exc(exc):
Expand Down
35 changes: 35 additions & 0 deletions Lib/test/test_contextlib_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,41 @@ async def suppress_exc(*exc_details):
self.assertIsInstance(inner_exc, ValueError)
self.assertIsInstance(inner_exc.__context__, ZeroDivisionError)

@_async_test
async def test_async_exit_exception_explicit_none_context(self):
# Ensure AsyncExitStack chaining matches actual nested `with` statements
# regarding explicit __context__ = None.

class MyException(Exception):
pass

@asynccontextmanager
async def my_cm():
try:
yield
except BaseException:
exc = MyException()
try:
raise exc
finally:
exc.__context__ = None

@asynccontextmanager
async def my_cm_with_exit_stack():
async with self.exit_stack() as stack:
await stack.enter_async_context(my_cm())
yield stack

for cm in (my_cm, my_cm_with_exit_stack):
with self.subTest():
try:
async with cm():
raise IndexError()
except MyException as exc:
self.assertIsNone(exc.__context__)
else:
self.fail("Expected IndexError, but no exception was raised")

@_async_test
async def test_instance_bypass_async(self):
class Example(object): pass
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2021-07-12-10-32-48.bpo-44594.eEa5zi.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix an edge case of :class:`ExitStack` and :class:`AsyncExitStack` exception
chaining. They will now match ``with`` block behavior when ``__context__`` is
explicitly set to ``None`` when the exception is in flight.