From 75f11f0b6576a7bd60927e3425a679c20c40f261 Mon Sep 17 00:00:00 2001 From: Allan Feldman Date: Wed, 21 Feb 2018 19:51:33 -0800 Subject: [PATCH 1/5] Fix reference cycle caused by PseudoFixtureDef. Python types have reference cycles to themselves when they are created. This is partially caused by descriptors which get / set values from the __dict__ attribute for getattr / setattr on classes. This is not normally an issue since types tend to remain referenced for the lifetime of the Python process (and thus never become garbage). However, in the case of PseudoFixtureDef, the class is generated in _get_active_fixturedef and later discarded when pytest_fixture_setup returns. As a result, the generated PseudoFixtureDef type becomes garbage. This is not really a performance issue but it can lead to some problems when making tests and assertions about garbage when using pytest. This garbage creation problem can be rectified by returning a namedtuple instance which is functionally the same. In the modified code, the namedtuple is allocated / deallocated using reference counting rather than having to use the garbage collector. --- _pytest/fixtures.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index a8445767c..21fc32846 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -4,7 +4,7 @@ import functools import inspect import sys import warnings -from collections import OrderedDict, deque, defaultdict +from collections import OrderedDict, deque, defaultdict, namedtuple import attr import py @@ -23,6 +23,8 @@ from _pytest.compat import ( ) from _pytest.outcomes import fail, TEST_OUTCOME +PseudoFixtureDef = namedtuple('PseudoFixtureDef', ('cached_result', 'scope')) + def pytest_sessionstart(session): import _pytest.python @@ -440,10 +442,9 @@ class FixtureRequest(FuncargnamesCompatAttr): fixturedef = self._getnextfixturedef(argname) except FixtureLookupError: if argname == "request": - class PseudoFixtureDef(object): - cached_result = (self, [0], None) - scope = "function" - return PseudoFixtureDef + cached_result = (self, [0], None) + scope = "function" + return PseudoFixtureDef(cached_result, scope) raise # remove indent to prevent the python3 exception # from leaking into the call From aa53e37fa255248c101f32dda876ad589bc68bb2 Mon Sep 17 00:00:00 2001 From: Allan Feldman Date: Wed, 21 Feb 2018 21:35:35 -0800 Subject: [PATCH 2/5] Add a test to expose leaked PseudoFixtureDef types. --- testing/python/fixture.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/testing/python/fixture.py b/testing/python/fixture.py index 6bcb1ab00..8638e361a 100644 --- a/testing/python/fixture.py +++ b/testing/python/fixture.py @@ -519,6 +519,41 @@ class TestRequestBasic(object): assert len(arg2fixturedefs) == 1 assert arg2fixturedefs['something'][0].argname == "something" + def test_request_garbage(self, testdir): + testdir.makepyfile(""" + import sys + import pytest + import gc + + @pytest.fixture(autouse=True) + def something(request): + # this method of test doesn't work on pypy + if hasattr(sys, "pypy_version_info"): + yield + else: + original = gc.get_debug() + gc.set_debug(gc.DEBUG_SAVEALL) + gc.collect() + + yield + + gc.collect() + leaked_types = sum(1 for _ in gc.garbage + if 'PseudoFixtureDef' in str(_)) + + gc.garbage[:] = [] + + try: + assert leaked_types == 0 + finally: + gc.set_debug(original) + + def test_func(): + pass + """) + reprec = testdir.inline_run() + reprec.assertoutcome(passed=1) + def test_getfixturevalue_recursive(self, testdir): testdir.makeconftest(""" import pytest From 287c003cfd28cb2fc0bd4cba92b76333a8543ab7 Mon Sep 17 00:00:00 2001 From: Allan Feldman Date: Wed, 21 Feb 2018 21:56:17 -0800 Subject: [PATCH 3/5] Add myself to AUTHORS. --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index ab646f406..3a564606c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -197,3 +197,4 @@ Xuan Luong Xuecong Liao Zoltán Máté Roland Puntaier +Allan Feldman From 7536e949b18aa6ab5c3594ce9c200aed9dcf8dcb Mon Sep 17 00:00:00 2001 From: Allan Feldman Date: Wed, 21 Feb 2018 22:00:39 -0800 Subject: [PATCH 4/5] Add changelog entry. --- changelog/3249.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/3249.bugfix.rst diff --git a/changelog/3249.bugfix.rst b/changelog/3249.bugfix.rst new file mode 100644 index 000000000..38fa9179f --- /dev/null +++ b/changelog/3249.bugfix.rst @@ -0,0 +1 @@ +Fix reference cycle generated when using the ``request`` fixture. From 48548767fcc1c65134309b649cf4d63f5b9cec65 Mon Sep 17 00:00:00 2001 From: Allan Feldman Date: Wed, 21 Feb 2018 22:59:33 -0800 Subject: [PATCH 5/5] Use a frozen attr class for PseudoFixtureDef. --- _pytest/fixtures.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 21fc32846..f9d0ef572 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -4,7 +4,7 @@ import functools import inspect import sys import warnings -from collections import OrderedDict, deque, defaultdict, namedtuple +from collections import OrderedDict, deque, defaultdict import attr import py @@ -23,7 +23,11 @@ from _pytest.compat import ( ) from _pytest.outcomes import fail, TEST_OUTCOME -PseudoFixtureDef = namedtuple('PseudoFixtureDef', ('cached_result', 'scope')) + +@attr.s(frozen=True) +class PseudoFixtureDef(object): + cached_result = attr.ib() + scope = attr.ib() def pytest_sessionstart(session):