Skip to content
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.

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

Fix memory leak with pytest.raises by using weakref #2006

Merged
merged 3 commits into from
Nov 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ mbyt
Michael Aquilina
Michael Birtwell
Michael Droettboom
Michael Seifert
Mike Lundy
Nicolas Delaby
Oleg Pidsadnyi
Expand Down
14 changes: 11 additions & 3 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
* When loading plugins, import errors which contain non-ascii messages are now properly handled in Python 2 (`#1998`_).
Thanks `@nicoddemus`_ for the PR.

* Fixed cyclic reference when ``pytest.raises`` is used in context-manager form (`#1965`_). Also as a
result of this fix, ``sys.exc_info()`` is left empty in both context-manager and function call usages.
Previously, ``sys.exc_info`` would contain the exception caught by the context manager,
even when the expected exception occurred.
Thanks `@MSeifert04`_ for the report and the PR.

* Fixed false-positives warnings from assertion rewrite hook for modules that were rewritten but
were later marked explicitly by ``pytest.register_assert_rewrite``
or implicitly as a plugin (`#2005`_).
Expand All @@ -36,12 +42,14 @@

.. _@adborden: https://github.com/adborden
.. _@cwitty: https://github.com/cwitty
.. _@okulynyak: https://github.com/okulynyak
.. _@matclab: https://github.com/matclab
.. _@gdyuldin: https://github.com/gdyuldin
.. _@d_b_w: https://github.com/d_b_w
.. _@gdyuldin: https://github.com/gdyuldin
.. _@matclab: https://github.com/matclab
.. _@MSeifert04: https://github.com/MSeifert04
.. _@okulynyak: https://github.com/okulynyak

.. _#442: https://github.com/pytest-dev/pytest/issues/442
.. _#1965: https://github.com/pytest-dev/pytest/issues/1965
.. _#1976: https://github.com/pytest-dev/pytest/issues/1976
.. _#1984: https://github.com/pytest-dev/pytest/issues/1984
.. _#1998: https://github.com/pytest-dev/pytest/issues/1998
Expand Down
5 changes: 3 additions & 2 deletions _pytest/_code/code.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import sys
from inspect import CO_VARARGS, CO_VARKEYWORDS
import re
from weakref import ref

import py
builtin_repr = repr
Expand Down Expand Up @@ -230,7 +231,7 @@ def ishidden(self):
return False

if py.builtin.callable(tbh):
return tbh(self._excinfo)
return tbh(None if self._excinfo is None else self._excinfo())
else:
return tbh

Expand Down Expand Up @@ -370,7 +371,7 @@ def __init__(self, tup=None, exprinfo=None):
#: the exception type name
self.typename = self.type.__name__
#: the exception traceback (_pytest._code.Traceback instance)
self.traceback = _pytest._code.Traceback(self.tb, excinfo=self)
self.traceback = _pytest._code.Traceback(self.tb, excinfo=ref(self))

def __repr__(self):
return "<ExceptionInfo %s tblen=%d>" % (self.typename, len(self.traceback))
Expand Down
6 changes: 5 additions & 1 deletion _pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,11 @@ def __exit__(self, *tp):
exc_type, value, traceback = tp
tp = exc_type, exc_type(value), traceback
self.excinfo.__init__(tp)
return issubclass(self.excinfo.type, self.expected_exception)
suppress_exception = issubclass(self.excinfo.type, self.expected_exception)
if sys.version_info[0] == 2 and suppress_exception:
sys.exc_clear()
return suppress_exception


# builtin pytest.approx helper

Expand Down
30 changes: 30 additions & 0 deletions testing/python/raises.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import pytest
import sys


class TestRaises:
def test_raises(self):
Expand Down Expand Up @@ -96,3 +98,31 @@ def test_custom_raise_message(self):
assert e.msg == message
else:
assert False, "Expected pytest.raises.Exception"

@pytest.mark.parametrize('method', ['function', 'with'])
def test_raises_cyclic_reference(self, method):
"""
Ensure pytest.raises does not leave a reference cycle (#1965).
"""
import gc

class T(object):
def __call__(self):
raise ValueError

t = T()
if method == 'function':
pytest.raises(ValueError, t)
else:
with pytest.raises(ValueError):
t()

# ensure both forms of pytest.raises don't leave exceptions in sys.exc_info()
assert sys.exc_info() == (None, None, None)

del t

# ensure the t instance is not stuck in a cyclic reference
for o in gc.get_objects():
assert type(o) is not T