Skip to content

Commit

Permalink
Fix memory leak caused by fixture values never been garbage collected
Browse files Browse the repository at this point in the history
The leak was caused by the (unused) `FixtureRequest._fixture_values`
cache.

This was introduced because the `partial` object (created to call
FixtureDef.finish() bound with the Request) is kept alive
through the entire session when a function-scoped fixture depends
on a session-scoped (or higher) fixture because of the nested
`addfinalizer` calls.

FixtureDef.finish() started receiving a request object in order to
obtain the proper hook proxy object (#2127), but this does not seem
useful at all in practice because `pytest_fixture_post_finalizer`
will be called with the `request` object of the moment the fixture value
was *created*, not the request object active when the fixture value
is being destroyed. We should probably deprecate/remove the request
parameter from `pytest_fixture_post_finalizer`.

Fix #2981
  • Loading branch information
nicoddemus committed Dec 13, 2017
1 parent 476d4df commit c3f63ac
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
17 changes: 10 additions & 7 deletions _pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ def __init__(self, pyfuncitem):
self.fixturename = None
#: Scope string, one of "function", "class", "module", "session"
self.scope = "function"
self._fixture_values = {} # argname -> fixture value
self._fixture_defs = {} # argname -> FixtureDef
fixtureinfo = pyfuncitem._fixtureinfo
self._arg2fixturedefs = fixtureinfo.name2fixturedefs.copy()
Expand Down Expand Up @@ -450,8 +449,7 @@ class PseudoFixtureDef:
raise
# remove indent to prevent the python3 exception
# from leaking into the call
result = self._getfixturevalue(fixturedef)
self._fixture_values[argname] = result
self._compute_fixture_value(fixturedef)
self._fixture_defs[argname] = fixturedef
return fixturedef

Expand All @@ -466,7 +464,14 @@ def _get_fixturestack(self):
values.append(fixturedef)
current = current._parent_request

def _getfixturevalue(self, fixturedef):
def _compute_fixture_value(self, fixturedef):
"""
Creates a SubRequest based on "self" and calls the execute method of the given fixturedef object. This will
force the FixtureDef object to throw away any previous results and compute a new fixture value, which
will be stored into the FixtureDef object itself.
:param FixtureDef fixturedef:
"""
# prepare a subrequest object before calling fixture function
# (latter managed by fixturedef)
argname = fixturedef.argname
Expand Down Expand Up @@ -515,12 +520,11 @@ def _getfixturevalue(self, fixturedef):
exc_clear()
try:
# call the fixture function
val = fixturedef.execute(request=subrequest)
fixturedef.execute(request=subrequest)
finally:
# if fixture function failed it might have registered finalizers
self.session._setupstate.addfinalizer(functools.partial(fixturedef.finish, request=subrequest),
subrequest.node)
return val

def _check_scope(self, argname, invoking_scope, requested_scope):
if argname == "request":
Expand Down Expand Up @@ -573,7 +577,6 @@ def __init__(self, request, scope, param, param_index, fixturedef):
self.scope = scope
self._fixturedef = fixturedef
self._pyfuncitem = request._pyfuncitem
self._fixture_values = request._fixture_values
self._fixture_defs = request._fixture_defs
self._arg2fixturedefs = request._arg2fixturedefs
self._arg2index = request._arg2index
Expand Down
1 change: 1 addition & 0 deletions changelog/2981.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix **memory leak** where objects returned by fixtures were never destructed by the garbage collector.
43 changes: 43 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -913,3 +913,46 @@ def test(request):
})
result = testdir.runpytest()
result.stdout.fnmatch_lines(['* 1 passed *'])


def test_fixture_values_leak(testdir):
"""Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected
life-times (#2981).
"""
testdir.makepyfile("""
import attr
import gc
import pytest
import weakref
@attr.s
class SomeObj(object):
name = attr.ib()
fix_of_test1_ref = None
session_ref = None
@pytest.fixture(scope='session')
def session_fix():
global session_ref
obj = SomeObj(name='session-fixture')
session_ref = weakref.ref(obj)
return obj
@pytest.fixture
def fix(session_fix):
global fix_of_test1_ref
obj = SomeObj(name='local-fixture')
fix_of_test1_ref = weakref.ref(obj)
return obj
def test1(fix):
assert fix_of_test1_ref() is fix
def test2():
gc.collect()
# fixture "fix" created during test1 must have been destroyed by now
assert fix_of_test1_ref() is None
""")
result = testdir.runpytest()
result.stdout.fnmatch_lines(['* 2 passed *'])

0 comments on commit c3f63ac

Please sign in to comment.