Skip to content

Commit

Permalink
Merge pull request #2006 from MSeifert04/fix-1965
Browse files Browse the repository at this point in the history
Fix memory leak with pytest.raises by using weakref
  • Loading branch information
nicoddemus committed Nov 9, 2016
2 parents 0b94c43 + 1130b9f commit fc304b8
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
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

0 comments on commit fc304b8

Please sign in to comment.