assertRaises increases reference counter

[VjacheslavFyodorov]

BPO	23890
Nosy	@ncoghlan, @vstinner, @rbtcollins, @ezio-melotti, @bitdancer, @voidspace, @ghost-script
PRs	bpo-23890: Fix ref cycles in TestCase.assertRaises() #193 [3.6] bpo-23890: Fix ref cycles in TestCase.assertRaises() #858 [3.5] bpo-23890: Fix ref cycles in TestCase.assertRaises() (#193) #2228
Files	test_assertRaises.py: unit test demo for assertRaises

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-06-15.22:52:29.492>
created_at = <Date 2015-04-08.18:37:05.507>
labels = ['3.7', 'type-bug', 'library']
title = 'assertRaises increases reference counter'
updated_at = <Date 2017-06-15.22:52:29.491>
user = 'https://bugs.python.org/VjacheslavFyodorov'

bugs.python.org fields:

activity = <Date 2017-06-15.22:52:29.491>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-06-15.22:52:29.492>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2015-04-08.18:37:05.507>
creator = 'Vjacheslav.Fyodorov'
dependencies = []
files = ['38866']
hgrepos = []
issue_num = 23890
keywords = []
message_count = 12.0
messages = ['240280', '240293', '240309', '261720', '288076', '288108', '288449', '290664', '290666', '296132', '296135', '296136']
nosy_count = 9.0
nosy_names = ['ncoghlan', 'vstinner', 'rbcollins', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'docs@python', 'Vjacheslav.Fyodorov', 'subho']
pr_nums = ['193', '858', '2228']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23890'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[VjacheslavFyodorov]

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.

[bitdancer]

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.

[VjacheslavFyodorov]

It seems, as a minimum it must be noticed in docs.

[rbtcollins]

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.

[ncoghlan]

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)

[ncoghlan]

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__)

[ncoghlan]

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.

[vstinner]

New changeset bbd3cf8 by Victor Stinner in branch 'master':
Fix ref cycles in TestCase.assertRaises() (#193)
bbd3cf8

[vstinner]

It seems like Python 2.7 is not affected.

[vstinner]

New changeset 50dbf57 by Victor Stinner in branch '3.6':
bpo-23890: Fix ref cycle in TestCase.assertRaises (#858)
50dbf57

[vstinner]

New changeset 3dc573c by Victor Stinner in branch '3.5':
Fix ref cycles in TestCase.assertRaises() (#193) (bpo-2228)
3dc573c

[vstinner]

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.