New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session fixture keeps frames of other fixtures alive #2981

Closed
nicoddemus opened this Issue Nov 29, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@nicoddemus
Member

nicoddemus commented Nov 29, 2017

Consider this test file:

import pytest
import weakref

class SomeObj:
    def __init__(self, name):
        self.name = name
    def __repr__(self):
        return '<SomeObj "{}">'.format(self.name)

ref = None
session_ref = None

@pytest.fixture(scope='session')
def session_fix():
    global session_ref
    session_obj = SomeObj('session')
    session_ref = weakref.ref(session_obj)
    return session_obj

@pytest.fixture
def fix(session_fix):
    global ref
    obj = SomeObj('local')
    ref = weakref.ref(obj)
    return obj

def test1(fix):
    assert ref() is fix

def test2():
    assert ref() is None

This fails with:

============================= test session starts =============================
platform win32 -- Python 3.6.3, pytest-3.3.1.dev26+gfdfc194, py-1.5.2, pluggy-0.6.0 -- c:\sybil\.env36\scripts\python.exe
cachedir: .cache
rootdir: C:\pytest\.tmp, inifile: pytest.ini
collected 2 items

test_leak.py::test1 PASSED                                               [ 50%]
test_leak.py::test2 FAILED                                               [100%]

================================== FAILURES ===================================
____________________________________ test2 ____________________________________

    def test2():
>       assert ref() is None
E       assert <SomeObj "local"> is None
E        +  where <SomeObj "local"> = ref()

test_leak.py:31: AssertionError
===================== 1 failed, 1 passed in 0.04 seconds ======================

So for some reason the object created in fix is being kept alive after test1 has finished. This is caused by session_fix and can be verified by changing test1 to no longer require session_fix in which case both tests pass.

This seems to reproduce the errors reported in #2968 and #2970 because both tests pass in 3.2.5. 😓

@nicoddemus nicoddemus self-assigned this Nov 29, 2017

@nicoddemus nicoddemus referenced this issue Nov 29, 2017

Closed

Memory usage in 3.3.0 #2968

4 of 4 tasks complete

nicoddemus added a commit to nicoddemus/pytest that referenced this issue Dec 13, 2017

Fix memory leak caused by fixture values never been garbage collected
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 (pytest-dev#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 pytest-dev#2981
@JacobSMoller

This comment has been minimized.

JacobSMoller commented Dec 14, 2017

Just ran the fix from #3030 locally, this also fixes an issue we encountered where our connections to postgresql were not closed, resulting in:
(psycopg2.OperationalError) FATAL: remaining connection slots are reserved for non-replication superuser connections and
(psycopg2.OperationalError) FATAL: sorry, too many clients already
If the test role was a superuser.

Just for anyone else coming across this issue.

Thanks @nicoddemus and the rest of the pytest team for all the work you do. 🎊

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Dec 14, 2017

Thanks @JacobSMoller for testing the PR! 👍 😁

Glad this fixed your problem, but I would recommend regardless to explicitly close your connections and do not rely on the garbage collector for that.

@mplanchard

This comment has been minimized.

mplanchard commented Jan 5, 2018

@nicoddemus just wanted to say thanks for your work on this! We'll be upgrading immediately and will let you know if we run into any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment