From 1e5b21cd6180009939130f6169c936427d0d4572 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 18 Oct 2016 00:56:44 +0200 Subject: [PATCH 1/3] Fix memory leak with pytest.raises by using weakref --- AUTHORS | 1 + CHANGELOG.rst | 3 +++ _pytest/_code/code.py | 5 +++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index 378bac5a491..6de0a112b7d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -99,6 +99,7 @@ mbyt Michael Aquilina Michael Birtwell Michael Droettboom +Michael Seifert Mike Lundy Nicolas Delaby Oleg Pidsadnyi diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5dada862a57..fb5395731e1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,9 @@ * When loading plugins, import errors which contain non-ascii messages are now properly handled in Python 2 (`#1998`_). Thanks `@nicoddemus`_ for the PR. +* Fixed memory leak in the RaisesContext (pytest.raises) (`#1965`_). + Thanks `@MSeifert04`_ for the report 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`_). diff --git a/_pytest/_code/code.py b/_pytest/_code/code.py index 416ee0b1b3b..5b423793926 100644 --- a/_pytest/_code/code.py +++ b/_pytest/_code/code.py @@ -1,6 +1,7 @@ import sys from inspect import CO_VARARGS, CO_VARKEYWORDS import re +from weakref import ref import py builtin_repr = repr @@ -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 @@ -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 "" % (self.typename, len(self.traceback)) From 552c7d4286f582255752730d2c512d0aff4a44c4 Mon Sep 17 00:00:00 2001 From: Michael Seifert Date: Tue, 18 Oct 2016 01:22:53 +0200 Subject: [PATCH 2/3] added test (thanks @nicoddemus) and added links in Changelog --- CHANGELOG.rst | 10 ++++++---- testing/python/raises.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fb5395731e1..35a54de4b97 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,7 +13,7 @@ Thanks `@nicoddemus`_ for the PR. * Fixed memory leak in the RaisesContext (pytest.raises) (`#1965`_). - Thanks `@MSeifert04`_ for the report the PR. + 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`` @@ -39,12 +39,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 diff --git a/testing/python/raises.py b/testing/python/raises.py index 59fd622fd19..80c3df2abe3 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -96,3 +96,21 @@ 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_memoryleak(self, method): + import weakref + + class T: + def __call__(self): + raise ValueError + + t = T() + if method == 'function': + pytest.raises(ValueError, t) + else: + with pytest.raises(ValueError): + t() + + t = weakref.ref(t) + assert t() is None From 1130b9f742d053a429149e56b8a0c8aec72a9968 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 8 Nov 2016 21:34:45 -0200 Subject: [PATCH 3/3] Fix the stubborn test about cyclic references left by pytest.raises In Python 2, a context manager's __exit__() leaves sys.exc_info with the exception values even when it was supposed to suppress the exception, so we explicitly call sys.exc_clear() which removes the traceback and allow the object to be released. Also updated the test to not depend on the immediate destruction of the object but instead to ensure it is not being tracked as a cyclic reference. Fix #1965 --- CHANGELOG.rst | 5 ++++- _pytest/python.py | 6 +++++- testing/python/raises.py | 22 +++++++++++++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 35a54de4b97..e88eb5b09e2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,7 +12,10 @@ * When loading plugins, import errors which contain non-ascii messages are now properly handled in Python 2 (`#1998`_). Thanks `@nicoddemus`_ for the PR. -* Fixed memory leak in the RaisesContext (pytest.raises) (`#1965`_). +* 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 diff --git a/_pytest/python.py b/_pytest/python.py index a42e7185e6e..18432c1e7f8 100644 --- a/_pytest/python.py +++ b/_pytest/python.py @@ -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 diff --git a/testing/python/raises.py b/testing/python/raises.py index 80c3df2abe3..8f141cfa1a9 100644 --- a/testing/python/raises.py +++ b/testing/python/raises.py @@ -1,4 +1,6 @@ import pytest +import sys + class TestRaises: def test_raises(self): @@ -98,10 +100,13 @@ def test_custom_raise_message(self): assert False, "Expected pytest.raises.Exception" @pytest.mark.parametrize('method', ['function', 'with']) - def test_raises_memoryleak(self, method): - import weakref + def test_raises_cyclic_reference(self, method): + """ + Ensure pytest.raises does not leave a reference cycle (#1965). + """ + import gc - class T: + class T(object): def __call__(self): raise ValueError @@ -112,5 +117,12 @@ def __call__(self): with pytest.raises(ValueError): t() - t = weakref.ref(t) - assert t() is None + # 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 +