[New-bugs-announce] [issue19092] ExitStack.__exit__ incorrectly suppresses exceptions in __exit__ callbacks of inner context managers

[Hrvoje Nikšić]

New submission from Hrvoje Nikšić:

While using contextlib.ExitStack in our project, we noticed that its __exit__ method of contextlib.ExitStack suppresses the exception raised in any contextmanager's __exit__ except the outermost one. Here is a test case to reproduce the problem:

class Err:
  def __enter__(self): pass
  def __exit__(self, *exc): 1/0

class Ok:
  def __enter__(self): pass
  def __exit__(self, *exc): pass


import contextlib
s = contextlib.ExitStack()
s.push(Ok())
s.push(Err())
with s:
  pass

Since the inner context manager raises in __exit__ and neither context manager requests suppression, I would expect to see a ZeroDivisionError raised. What actually happens is that the exception is suppressed. This behavior caused us quite a few headaches before we figured out why we the exceptions raised in during __exit__ went silently undetected in production.

The problem is in ExitStack.__exit__, which explicitly propagates the exception only if it occurs in the outermost exit callback. The idea behind it appears to be to simply record the raised exception in order to allow exit callbacks of the outer context managers to see the it -- and get a chance to suppress it. The only exception that is directly re-raised is the one occurring in the outermost exit callback, because it certainly cannot be seen nor suppressed by anyone else.

But this reasoning is flawed because if an exception happens in an inner cm and none of the outer cm's chooses to suppress it, then there will be no one left to raise it. Simply returning True from ExitStack.__exit__ is of no help, as that only has an effect when an exception was passed into the function in the first place, and even then, the caller can only re-raise the earlier exception, not the exception that occurred in the exit callback. And if no exception was sent to ExitStack.__exit__, as is the case in the above code, then no exception will be re-raised at all, effectively suppressing it.

I believe the correct way to handle this is by keeping track of whether an exception actually occurred in one of the _exit_callbacks. As before, the exception is forwarded to next cm's exit callbacks, but if none of them suppresses it, then the exception is re-raised instead of returning from the function. I am attaching a patch to contextlib.py that implements this change.

The patch also makes sure that True is returned from ExitStack.__exit__ only if an exception was actually passed into it.

----------
components: Library (Lib)
files: exitstack.diff
keywords: patch
messages: 198411
nosy: hniksic
priority: normal
severity: normal
status: open
title: ExitStack.__exit__ incorrectly suppresses exceptions in __exit__ callbacks of inner context managers
type: behavior
versions: Python 3.4
Added file: http://bugs.python.org/file31872/exitstack.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue19092>
_______________________________________