msg240280 - (view) |
Author: Vjacheslav Fyodorov (Vjacheslav.Fyodorov) |
Date: 2015-04-08 18:37 |
Sometimes unittest's assertRaises increases reference counter of callable. This can break tests in tricky cases. Not seen in 2.X version. Demo file attached. |
|
|
msg240293 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2015-04-09 00:18 |
This is presumably a result of the exception object being saved on the context manager object and there being a circular reference chain that doesn't immediately get GCed. This is just how python works. I don't know if it would be sensible/useful to use the new lightweight traceback stuff in assertRaises. If so it would probably have to be optional for backward compatibility reasons. |
|
|
msg240309 - (view) |
Author: Vjacheslav Fyodorov (Vjacheslav.Fyodorov) |
Date: 2015-04-09 07:09 |
It seems, as a minimum it must be noticed in docs. |
|
|
msg261720 - (view) |
Author: Robert Collins (rbcollins) *  |
Date: 2016-03-14 03:10 |
I don't think we make any guarantees for or against references, but - we attempt to free references already (see how we call clear_frames), so this is a bug, pure and simple. |
|
|
msg288076 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-02-18 12:46 |
As Robert noted, this is a straight up error, where the clear_frames() call and deleting the exception traceback from the saved object aren't enough to drop all references to `callable_obj`. The trick is that the cleanup code isn't accounting for __cause__ and __context__: it's only clearing and dropping the traceback for the topmost exception in the chain. In Vjacheslav's example, there are *two* tracebacks to worry about: both the top level one (which is getting cleaned up) and the inner one from exc.__context__ which isn't being touched. We could likely use a "traceback.clear_all_frames()" helper that walks an exception tree clearing *all* the traceback frames, both on the original exception, and on all the __cause__ and __context__ attributes. (We can't just clear __cause__ and __context__, since those can be accessed and tested when using the context manager form and the `exception` attribute) |
|
|
msg288108 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-02-19 05:55 |
I've been looking into this further, and a reproducer that's independent of assertRaises() is to just bind the function to a local variable name in an outer function: def outer(): callable_obj = f try: callable_obj() except Exception as exc: return exc That should make it easier to test a basic recursive "clear_all_frames" operation like: def clear_all_frames(exc): cause = exc.__cause__ if cause is not None: clear_all_frames(cause) context = exc.__context__ if context is not None: clear_all_frames(context) traceback.clear_frames(exc.__traceback__) |
|
|
msg288449 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2017-02-23 15:20 |
Victor's PR takes a more pragmatic approach to address this problem: rather than adding the ability to clear the frames for an arbitrary exception tree, it just uses a try/finally in a couple of key unittest function definitions to clear the offending circular references by setting them to None. |
|
|
msg290664 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-03-27 22:56 |
New changeset bbd3cf8f1ef1e91a8d6dac6411e18b4b9084abf5 by Victor Stinner in branch 'master': Fix ref cycles in TestCase.assertRaises() (#193) https://github.com/python/cpython/commit/bbd3cf8f1ef1e91a8d6dac6411e18b4b9084abf5 |
|
|
msg290666 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-03-27 23:04 |
It seems like Python 2.7 is not affected. |
|
|
msg296132 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-06-15 22:18 |
New changeset 50dbf577e10f806056d60ac956db0748d2cc8257 by Victor Stinner in branch '3.6': bpo-23890: Fix ref cycle in TestCase.assertRaises (#858) https://github.com/python/cpython/commit/50dbf577e10f806056d60ac956db0748d2cc8257 |
|
|
msg296135 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-06-15 22:51 |
New changeset 3dc573c8d19dc42ed786ca3237afdad183c41ca0 by Victor Stinner in branch '3.5': Fix ref cycles in TestCase.assertRaises() (#193) (#2228) https://github.com/python/cpython/commit/3dc573c8d19dc42ed786ca3237afdad183c41ca0 |
|
|
msg296136 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-06-15 22:52 |
Thank you Vjacheslav Fyodorov for your bug report! The bug should now be fixed in 3.5, 3.6 and master (future 3.7) branches. Python 2.7 is not affected by the bug. |
|
|