When a context manager's __exit__() method returns an object whose
conversion to bool raises an exception, 'with' loses that exception. For
example:

>>> class CM(object):
...   def __init__(self, exit_result):
...     self.exit_result = exit_result
...   def __enter__(self):
...     return 3
...   def __exit__(self, a, b, c):
...     return self.exit_result()
... 
>>> class TrueAsBool(object):
...   def __nonzero__(self): return True
... 
>>> class FalseAsBool(object):
...   def __nonzero__(self): return False
... 
>>> class FailAsBool(object):
...   def __nonzero__(self):
...     raise RuntimeError("Should see this but won't")
... 
>>> with CM(TrueAsBool):
...   raise AssertionError("Should NOT see this")
... 
>>> with CM(FalseAsBool):
...   raise AssertionError("Should see this")
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
AssertionError: Should see this
>>> with CM(FailAsBool):
...   raise AssertionError("Should see RuntimeException (oops)")
... 
>>> 

The problem is that WITH_CLEANUP only checks if PyObject_IsTrue(x)
returns non-zero, but that function returns <0 when the bool conversion
raises an exception.

Good catch. Here is a patch, with test.

I think your patch leaks a reference to 'x'. Move the Py_DECREF(x) to
before "if (err < 0)"?

And then really nitpicky: your test has 3 blank lines after it, and
should have 2.

Otherwise looks great. Thanks for picking this up!

Running the test with -R:: indeed shows the reference leak.
2nd patch with suggested corrections.

Looks good. Thanks!

I assume this should be merged into the 2.6.x and 3.0.x branches?

The patch doesn't apply cleanly to the 2.5 branch, because that doesn't
have r61290. If somebody wants to backport it, go ahead (if you can do
so by Thursday).

Does it need to be backported to 2.5? IMO it is a corner case.

Does it need to be backported to 2.5? IMO it is a corner case.

Not necessarily. If nobody volunteers, it won't be backported,
which is fine with me. The only potential release blocker for
a bug fix release can be a regression, which this isn't.

It's a corner case that would confuse anyone who ran into it, so I'm
happy to backport it. I'll probably just re-do your patch on the 2.5
branch since WITH_CLEANUP has a different structure there.

Fixed with r67684 (2.5), r67688 (trunk), r67689 (py3k), r67690 (3.0),
r67691 (2.6).