Skip to content
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

Pytest holds on to fixtures too long causing a memory leak #5642

Closed
matham opened this issue Jul 22, 2019 · 31 comments
Closed

Pytest holds on to fixtures too long causing a memory leak #5642

matham opened this issue Jul 22, 2019 · 31 comments
Labels
status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity topic: fixtures anything involving fixtures directly or indirectly type: performance performance or memory problem/improvement

Comments

@matham
Copy link

matham commented Jul 22, 2019

Pytest seems to hold on to the object yielded, even after the test is done, sometimes, even until all the tests are done.

The following code:

import pytest
import gc
import weakref


class SomethingInteresting(object):

	def check_app(self):
		assert True
		print('app checked')


@pytest.fixture
def my_app():
	app = SomethingInteresting()
	yield app

	app = weakref.ref(app)
	gc.collect()
	print('app is', app())
	if app() is not None:
		assert False


def test_my_app(my_app):
	my_app.check_app()

Results in this:

pytest /g/Python/libs/Playground/src/playground15.py
============================================================================================================= test session starts ============================================================================================================= platform win32 -- Python 3.7.3, pytest-4.6.3, py-1.8.0, pluggy-0.12.0
rootdir: G:\Python\libs\Playground
collected 1 item

..\Playground\src\playground15.py .E                                                                                                                                                                                                     [100%]

=================================================================================================================== ERRORS ==================================================================================================================== ______________________________________________________________________________________________________ ERROR at teardown of test_my_app _______________________________________________________________________________________________________

    @pytest.fixture
    def my_app():
        app = SomethingInteresting()
        yield app

        app = weakref.ref(app)
        gc.collect()
        print('app is', app())
        if app() is not None:
>               assert False
E     assert False

..\Playground\src\playground15.py:22: AssertionError
------------------------------------------------------------------------------------------------------------ Captured stdout call ------------------------------------------------------------------------------------------------------------- app checked
---------------------------------------------------------------------------------------------------------- Captured stdout teardown ----------------------------------------------------------------------------------------------------------- app is <src.playground15.SomethingInteresting object at 0x0000021F5F295908>
====================================================================================================== 1 passed, 1 error in 0.09 seconds ======================================================================================================

I tried to investigate using objgraph, and various pytest modules (e.g. warning) were blamed depending on the exact code. When I tested the app after the fixture has fully returned at the next test, then the app seemed to have been released.

However, in a more complex app that I have, pytest seemed to hold on to the app fixture instances forever, untill all the test has finished. I'm attaching what objgraph shows for this case that Terminal reporter is holding on to it.
sample-graph

Tested on Windows10, py3.7.

pytest                        4.6.3
pytest-cov                    2.7.1
pytest-trio                   0.5.2
@blueyed
Copy link
Contributor

blueyed commented Jul 22, 2019

Just some quick ideas: is this a regression maybe? In that case git-bisect could be useful.
In either case trying the latest pytest might be good - but maybe you are stuck on Python 2?

@matham
Copy link
Author

matham commented Jul 22, 2019

I updated to pytest 5.0.1 and the result was the same (assert False was hit)

@matham
Copy link
Author

matham commented Jul 22, 2019

Is there a pytest version I should give a try to see if this is a regression? I can downgrade and test to see if a bisect would help at all.

@blueyed
Copy link
Contributor

blueyed commented Jul 22, 2019

bisect

Likely not then - just thought that you've might have noticed this yourself as a regression.
But might be worth a try still, if you can easily reproduce it (and as long as older pytest versions work for you / your code, of course :))

@asottile
Copy link
Member

I'm not seeing a leak here, or at least the memory for that is freed once the fixture is used up

The way fixtures work is they are (potentially) reused so a fixture may be called once and used multiple times. What pytest does is store the fixture's yielded / returned value in the FixtureDef object which usually outlives the fixture's function frame itself

when that fixture is no longer needed, the FixtureDef's finalization is called which includes tearing down the cached result

if hasattr(self, "cached_result"):
del self.cached_result

so I think this is not a bug

@asottile
Copy link
Member

here is, for example, a test demoing that the object in fact does not leak:

import pytest
import gc
import weakref


class C(object):

    def check_app(self):
        assert True
        print('app checked')


@pytest.fixture
def my_app():
    app = C()
    yield app

    app = weakref.ref(app)


def test_my_app(my_app):
    my_app.check_app()


def test2():
    obj = next(iter(o for o in gc.get_objects() if isinstance(o, C)), None)
    assert obj is None
$ pytest t.py -vs
============================= test session starts ==============================
platform linux -- Python 3.7.4, pytest-5.0.1, py-1.8.0, pluggy-0.12.0 -- /tmp/y/venv/bin/python3.7
cachedir: .pytest_cache
rootdir: /tmp/y
collected 2 items                                                              

t.py::test_my_app app checked
PASSED
t.py::test2 PASSED

=========================== 2 passed in 0.01 seconds ===========================

@matham
Copy link
Author

matham commented Jul 22, 2019

But what about the objgraph image I uploaded? Admittedly, it I don't have example code as I saw it in a larger app. That image was generated in a test subsequent to the test that used the fixture. I.e. the fixture was already done, but terminal reporter seems to have held on to that until all the tests were sone. I saw similar behavior when there are warnings in the logs

I'll try to replicate this.

@matham
Copy link
Author

matham commented Jul 22, 2019

Also, for the example code I posted, is there some post fixture hook I can use to make sure the fixture has been garbage collected? Rather than having to wait for the next test. E.g. if I wanted to make sure the fixture is not being leaked.

@asottile
Copy link
Member

I tried reading the objgraph but have no idea what I'm looking at and can't ^F -- could you post an svg instead?

@matham
Copy link
Author

matham commented Jul 23, 2019

Sorry I'm on a phone now so I cannot crop it, and anyway, the only relevant thing afaict in the giant graph is that the object at the root that ultimately keeps all these other objects alive, which ultimately keeps the app object alive, is a TerminalReporter object.

In other words it seems like TerminalReporter keeps the app fixture alive until all the tests are all done. Is it possible those logging object are keeping a ref to the fixtures?

I'll see if I can replicate this with a minimal example.

@matham
Copy link
Author

matham commented Jul 23, 2019

Ok, I have reproducible example:

import pytest
import gc
import weakref


class SomethingInteresting(object):

    def check_app(self):
        assert True
        print('app checked')

    async def sleep(self):
        pass


apps = []


@pytest.fixture
async def my_app():
    app = SomethingInteresting()
    app.sleep()
    yield app

    app = weakref.ref(app)
    apps.append(app)
    gc.collect()

    print('app is', app())
    if len(apps) >= 2 and apps[0]() is not None:
        assert False


async def test_my_app(my_app):
    my_app.check_app()


async def test_my_app2(my_app):
    my_app.check_app()

As you can see, I save a ref to the current app fixture in apps, and then check in the subsequent test if the last fixture is still alive. If I remove the line app.sleep(), everything passes, otherwise, I get the warning it (as I should) was never awaited on and the assert False is triggered:

============================================================================================================= test session starts ============================================================================================================= platform win32 -- Python 3.7.3, pytest-5.0.1, py-1.8.0, pluggy-0.12.0
rootdir: G:\Python\libs\ceed, inifile: pytest.ini
plugins: asyncio-0.10.0, cov-2.7.1, trio-0.5.2
collected 2 items

ceed\tests\playground15.py ..E                                                                                                                                                                                                           [100%]

=================================================================================================================== ERRORS ==================================================================================================================== ______________________________________________________________________________________________________ ERROR at teardown of test_my_app2 ______________________________________________________________________________________________________

    def finalizer():
        """Yield again, to finalize."""
        async def async_finalizer():
            try:
                await gen_obj.__anext__()
            except StopAsyncIteration:
                pass
            else:
                msg = "Async generator fixture didn't stop."
                msg += "Yield only once."
                raise ValueError(msg)

>       loop.run_until_complete(async_finalizer())

e:\python\python37\lib\site-packages\pytest_asyncio\plugin.py:93:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ C:\Users\Matthew Einhorn\AppData\Local\Programs\Python\Python37\Lib\asyncio\base_events.py:584: in run_until_complete
    return future.result()
e:\python\python37\lib\site-packages\pytest_asyncio\plugin.py:85: in async_finalizer
    await gen_obj.__anext__()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    @pytest.fixture
    async def my_app():
        app = SomethingInteresting()
        app.sleep()
        yield app

        app = weakref.ref(app)
        apps.append(app)
        gc.collect()

        print('app is', app())
        if len(apps) >= 2 and apps[0]() is not None:
>           assert False
E           assert False

ceed\tests\playground15.py:31: AssertionError
------------------------------------------------------------------------------------------------------------ Captured stdout call ------------------------------------------------------------------------------------------------------------- app checked
---------------------------------------------------------------------------------------------------------- Captured stdout teardown ----------------------------------------------------------------------------------------------------------- app is <playground15.SomethingInteresting object at 0x000001E33C6D6588>
============================================================================================================== warnings summary =============================================================================================================== ceed/tests/playground15.py::test_my_app
ceed/tests/playground15.py::test_my_app2
  G:\Python\libs\ceed\ceed\tests\playground15.py:22: RuntimeWarning: coroutine 'SomethingInteresting.sleep' was never awaited
    app.sleep()

-- Docs: https://docs.pytest.org/en/latest/warnings.html
================================================================================================ 2 passed, 2 warnings, 1 error in 0.21 seconds ================================================================================================

So essentially, when there's a warning, the fixture never dies. The same seemed to happen if app.sleep() was dropped in test_my_app instead.

Using pytest-trio instead of pytest-asyncio had the same issue. Here's the objgraph for the app at the time the assertion failed for whatever it's worth showing that pytest (tree root) is keeping the app object alive (final leaf at the bottom):
leak_asyncio

@asottile
Copy link
Member

at least in that case:

not sure there's anything to do here 🤷‍♂

the only ~slighty improvement would be to tear down the PytestPDB global at the end of the run, but even then I don't think that helps you since the cpython warning system is the one keeping your frame alive so long

@blueyed
Copy link
Contributor

blueyed commented Jul 23, 2019

As for the warnings: could be collect/stringify them earlier in pytest?
Or could/should Python not keep the ref in the first place, but format it already (earlier)?

It might also be that that's the case already, and __pytestPDB / pytestPDB is the problem after all? (this is via the import for set_trace)

@asottile
Copy link
Member

it's not just pytest internally, they're captured and exposed through this hook: https://pytest.readthedocs.io/en/latest/reference.html#_pytest.hookspec.pytest_warning_captured

since this hook is historic=True (presumably because warnings can occur during startup before plugins are registered) it records all calls indefinitely

@Zac-HD Zac-HD added status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity topic: fixtures anything involving fixtures directly or indirectly type: performance performance or memory problem/improvement labels Jul 25, 2019
@nicoddemus
Copy link
Member

Is there any action to be done by pytest here?

@asottile
Copy link
Member

afaik no -- since we need to be able to pass the original warning along to hook implementers (unless we were to deprecate and remove that hook -- but I don't see that happening)

@nicoddemus
Copy link
Member

Yeah, that's what I was thinking too.

So I'm closing this as "wont/cant fix"

@matham thanks again for the detailed report, but I'm afraid there's not anything that can be done to solve this.

@haroldrandom
Copy link

will -p no:warnings solve this?

@ax3l
Copy link

ax3l commented Oct 15, 2022

@asottile Re: #5642 (comment)

Is here a way to disable the caching of a yield-ed object of a fixture though?
I have a test setup where my autouse fixture manages its own heap (on an Nvidia GPU) and I need to be sure the object I create is really destructed:

@pytest.fixture(autouse=True, scope="function")
def init():
    mymod.init_heap()  # init GPU context
    yield
    mymod.free_heap()  # free GPU context; double-free once obj fixture is later trying to free

@pytest.fixture(scope="function")  # can I disable pytest caching for this yield?
def obj():
    v = mymod.Obj()
    yield v
    del v  # still refs remaining

@asottile
Copy link
Member

@ax3l not sure what you're asking -- those fixtures will be collected and destroyed per-test (since you have scope='function')

@ax3l
Copy link

ax3l commented Oct 15, 2022

What I am observing is the following:

  • 1 ref on v before yield v
  • 7 refs on v inside an empty test that uses it
  • 3 refs on v after the empty test, during obj() fixture teardown

For that reason, the following runtime logic occurs:

  • init() startup
  • obj() startup
  • Obj allocation
  • test run
  • obj() teardown
  • init() teardown
  • Obj free

The last line, free, is too late.

@ax3l
Copy link

ax3l commented Oct 15, 2022

Complete reproducer (please assume v = np.array([1, 2, 3]) lives on a memory space that is gone after init() teardown):

# conftest.py
import sys
import numpy as np

import pytest


@pytest.fixture(autouse=True, scope="function")
def init():
    yield

@pytest.fixture(scope="function")  # can I disable pytest caching for this fixture?
def obj():
    v = np.array([1, 2, 3])
    print("startup: ", sys.getrefcount(v) - 1)
    yield v  # can I disable pytest caching for this yield?
    print("\nteardown: ", sys.getrefcount(v) - 1)
    del v  # still refs remaining
# test_foo.py
import sys

def test_foo(obj):
    print("++ test_foo ++")
    print("in test:", sys.getrefcount(obj) - 1)
$ python3 -m pytest . -s -vvv
======================================== test session starts ========================================
platform linux -- Python 3.8.10, pytest-7.1.3, pluggy-0.13.1 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/axel/tmp/pytest-refs
collected 1 item                                                                                    

test_foo.py::test_foo startup:  1
++ test_foo ++
in test: 6
PASSED
teardown:  3


========================================= 1 passed in 0.01s =========================================

@asottile
Copy link
Member

if you're depending on the garbage collector collecting your objects in order you've probably got bigger problems. python's __del__ isn't really guaranteed to run in any specific order already

if you want specific and consistent memory management that's what a context manager is for

@ax3l
Copy link

ax3l commented Oct 15, 2022

python's __del__ isn't really guaranteed to run in any specific order already

Good point, demonstrator is CPython only where ref counting releases immediately after last ref is freed.

if you want specific and consistent memory management that's what a context manager is for

How can one use a context manager with pytest fixtures?
Use an plugin like pytest-contextfixture for my context manager in init()?

@asottile
Copy link
Member

asottile commented Oct 15, 2022

the easiest is to just:

@pytest.fixture
def whatever():
    with whatever_context() as obj:
        yield obj

@ax3l
Copy link

ax3l commented Oct 15, 2022

Let's assume this is CPython for a second, where last refs immediately frees. I think pytest still holds on to the yield, even inside a context:

# conftest.py
import contextlib
import sys
import numpy as np

import pytest


@contextlib.contextmanager
def init():
    v = np.array([1, 2, 3])
    print("\nenter: ", sys.getrefcount(v) - 1)
    yield v
    print("\nexit: ", sys.getrefcount(v) - 1)
    del v

@pytest.fixture(scope="function")
def obj():
    with init() as v:
        print("startup: ", sys.getrefcount(v) - 1)
        yield v   # can I disable pytest caching for this yield?
        print("\nteardown: ", sys.getrefcount(v) - 1)
enter:  1
startup:  2
++ test_foo ++
in test: 7
PASSED
teardown:  4

exit:  4

@ax3l
Copy link

ax3l commented Oct 15, 2022

Hm, I guess you want to yield the context manager itself as a fixture, not the objects...
and then pytest will at most cache the context manager, but not its managed objects? But then I don't use fixtures at all besides a single one.

@asottile
Copy link
Member

my point is more that you'd use a context manager to do the allocation and deallocation explicitly -- rather than relying on the garbage collector

@ax3l
Copy link

ax3l commented Oct 17, 2022

Thanks, got ya.

Just to loop back to the original question, the moment we actually hand out objects to pytest: is there a way to deactivate caching in a fixture in pytest? Or is there a recommended way to write generators of objects that are uncached in pytest but still use decorators like parametrize?

@asottile
Copy link
Member

that sounds like a completely different question -- I'd make a new issue / discussion -- this one has gotten pretty off topic already

@ax3l
Copy link

ax3l commented Oct 17, 2022

Thanks for your help. I'll move this to #10387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity topic: fixtures anything involving fixtures directly or indirectly type: performance performance or memory problem/improvement
Projects
None yet
Development

No branches or pull requests

7 participants