Skip to content

Commit db4df58

Browse files
authored
Merge pull request #3030 from nicoddemus/leak
Fix memory leak caused by fixture values never been garbage collected
2 parents b17c6e5 + c3f63ac commit db4df58

File tree

3 files changed

+54
-7
lines changed

3 files changed

+54
-7
lines changed

_pytest/fixtures.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ def __init__(self, pyfuncitem):
267267
self.fixturename = None
268268
#: Scope string, one of "function", "class", "module", "session"
269269
self.scope = "function"
270-
self._fixture_values = {} # argname -> fixture value
271270
self._fixture_defs = {} # argname -> FixtureDef
272271
fixtureinfo = pyfuncitem._fixtureinfo
273272
self._arg2fixturedefs = fixtureinfo.name2fixturedefs.copy()
@@ -450,8 +449,7 @@ class PseudoFixtureDef:
450449
raise
451450
# remove indent to prevent the python3 exception
452451
# from leaking into the call
453-
result = self._getfixturevalue(fixturedef)
454-
self._fixture_values[argname] = result
452+
self._compute_fixture_value(fixturedef)
455453
self._fixture_defs[argname] = fixturedef
456454
return fixturedef
457455

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

469-
def _getfixturevalue(self, fixturedef):
467+
def _compute_fixture_value(self, fixturedef):
468+
"""
469+
Creates a SubRequest based on "self" and calls the execute method of the given fixturedef object. This will
470+
force the FixtureDef object to throw away any previous results and compute a new fixture value, which
471+
will be stored into the FixtureDef object itself.
472+
473+
:param FixtureDef fixturedef:
474+
"""
470475
# prepare a subrequest object before calling fixture function
471476
# (latter managed by fixturedef)
472477
argname = fixturedef.argname
@@ -515,12 +520,11 @@ def _getfixturevalue(self, fixturedef):
515520
exc_clear()
516521
try:
517522
# call the fixture function
518-
val = fixturedef.execute(request=subrequest)
523+
fixturedef.execute(request=subrequest)
519524
finally:
520525
# if fixture function failed it might have registered finalizers
521526
self.session._setupstate.addfinalizer(functools.partial(fixturedef.finish, request=subrequest),
522527
subrequest.node)
523-
return val
524528

525529
def _check_scope(self, argname, invoking_scope, requested_scope):
526530
if argname == "request":
@@ -573,7 +577,6 @@ def __init__(self, request, scope, param, param_index, fixturedef):
573577
self.scope = scope
574578
self._fixturedef = fixturedef
575579
self._pyfuncitem = request._pyfuncitem
576-
self._fixture_values = request._fixture_values
577580
self._fixture_defs = request._fixture_defs
578581
self._arg2fixturedefs = request._arg2fixturedefs
579582
self._arg2index = request._arg2index

changelog/2981.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix **memory leak** where objects returned by fixtures were never destructed by the garbage collector.

testing/acceptance_test.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,3 +913,46 @@ def test(request):
913913
})
914914
result = testdir.runpytest()
915915
result.stdout.fnmatch_lines(['* 1 passed *'])
916+
917+
918+
def test_fixture_values_leak(testdir):
919+
"""Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected
920+
life-times (#2981).
921+
"""
922+
testdir.makepyfile("""
923+
import attr
924+
import gc
925+
import pytest
926+
import weakref
927+
928+
@attr.s
929+
class SomeObj(object):
930+
name = attr.ib()
931+
932+
fix_of_test1_ref = None
933+
session_ref = None
934+
935+
@pytest.fixture(scope='session')
936+
def session_fix():
937+
global session_ref
938+
obj = SomeObj(name='session-fixture')
939+
session_ref = weakref.ref(obj)
940+
return obj
941+
942+
@pytest.fixture
943+
def fix(session_fix):
944+
global fix_of_test1_ref
945+
obj = SomeObj(name='local-fixture')
946+
fix_of_test1_ref = weakref.ref(obj)
947+
return obj
948+
949+
def test1(fix):
950+
assert fix_of_test1_ref() is fix
951+
952+
def test2():
953+
gc.collect()
954+
# fixture "fix" created during test1 must have been destroyed by now
955+
assert fix_of_test1_ref() is None
956+
""")
957+
result = testdir.runpytest()
958+
result.stdout.fnmatch_lines(['* 2 passed *'])

0 commit comments

Comments
 (0)