Test regressions on PyPy3.10: `tests/test_adapter.py::TestSessionActions::test_do_not_leak_response`

[mgorny]

With 0.14.2:

$ tox -e pypy3
.pkg: _optional_hooks> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True flit_core.buildapi
.pkg: get_requires_for_build_sdist> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True flit_core.buildapi
.pkg: build_sdist> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True flit_core.buildapi
pypy3: install_package> python -I -m pip install --force-reinstall --no-deps /tmp/cachecontrol/.tox/.tmp/package/4/cachecontrol-0.14.2.tar.gz
pypy3: commands[0]> py.test tests/
========================================================= test session starts =========================================================
platform linux -- Python 3.10.14[pypy-7.3.17-final], pytest-8.3.4, pluggy-1.5.0
cachedir: .tox/pypy3/.pytest_cache
rootdir: /tmp/cachecontrol
configfile: pyproject.toml
collected 101 items                                                                                                                   

tests/test_adapter.py ...........FF                                                                                             [ 12%]
tests/test_cache_control.py .......................                                                                             [ 35%]
tests/test_chunked_response.py ...                                                                                              [ 38%]
tests/test_etag.py .....                                                                                                        [ 43%]
tests/test_expires_heuristics.py ................                                                                               [ 59%]
tests/test_max_age.py ..                                                                                                        [ 61%]
tests/test_redirects.py ....                                                                                                    [ 65%]
tests/test_regressions.py ..                                                                                                    [ 67%]
tests/test_serialization.py ..........                                                                                          [ 77%]
tests/test_server_http_version.py .                                                                                             [ 78%]
tests/test_storage_filecache.py ..................                                                                              [ 96%]
tests/test_storage_redis.py ...                                                                                                 [ 99%]
tests/test_vary.py .                                                                                                            [100%]

============================================================== FAILURES ===============================================================
______________________________________ TestSessionActions.test_do_not_leak_response[use_adapter] ______________________________________

self = <tests.test_adapter.TestSessionActions object at 0x00007f50d61cf6a8>, url = 'http://127.0.0.1:44109/'
sess = <requests.sessions.Session object at 0x00007f50d5f51168>

    def test_do_not_leak_response(self, url, sess):
        resp = sess.get(url + "stream", stream=True)
        resp.raise_for_status()
        r1_weak = weakref.ref(resp.raw)
    
        # This is a mis-use of requests, becase we should either consume
        # the body, or call .close().
        # But requests without cachecontrol handle this anyway, because
        # urllib3.response.HTTPResponse has a __del__ finalizer on it that closes it
        # once there are no more references to it.
        # We should not break this.
    
        resp = None
        # Below this point, it should be closed because there are no more references
        # to the session response.
    
        r1 = r1_weak()
>       assert r1 is None or r1.closed
E       assert (<urllib3.response.HTTPResponse object at 0x00007f50d5f63910> is None or False)
E        +  where False = <urllib3.response.HTTPResponse object at 0x00007f50d5f63910>.closed

tests/test_adapter.py:89: AssertionError
-------------------------------------------------------- Captured stdout setup --------------------------------------------------------
Using adapter
______________________________________ TestSessionActions.test_do_not_leak_response[use_wrapper] ______________________________________

self = <tests.test_adapter.TestSessionActions object at 0x00007f50d61cf1d8>, url = 'http://127.0.0.1:44109/'
sess = <requests.sessions.Session object at 0x00007f50d5e77910>

    def test_do_not_leak_response(self, url, sess):
        resp = sess.get(url + "stream", stream=True)
        resp.raise_for_status()
        r1_weak = weakref.ref(resp.raw)
    
        # This is a mis-use of requests, becase we should either consume
        # the body, or call .close().
        # But requests without cachecontrol handle this anyway, because
        # urllib3.response.HTTPResponse has a __del__ finalizer on it that closes it
        # once there are no more references to it.
        # We should not break this.
    
        resp = None
        # Below this point, it should be closed because there are no more references
        # to the session response.
    
        r1 = r1_weak()
>       assert r1 is None or r1.closed
E       assert (<urllib3.response.HTTPResponse object at 0x00007f50d5e76598> is None or False)
E        +  where False = <urllib3.response.HTTPResponse object at 0x00007f50d5e76598>.closed

tests/test_adapter.py:89: AssertionError
-------------------------------------------------------- Captured stdout setup --------------------------------------------------------
Using helper
======================================================= short test summary info =======================================================
FAILED tests/test_adapter.py::TestSessionActions::test_do_not_leak_response[use_adapter] - assert (<urllib3.response.HTTPResponse object at 0x00007f50d5f63910> is None or False)
FAILED tests/test_adapter.py::TestSessionActions::test_do_not_leak_response[use_wrapper] - assert (<urllib3.response.HTTPResponse object at 0x00007f50d5e76598> is None or False)
==================================================== 2 failed, 99 passed in 0.97s =====================================================
[08/Jan/2025:10:12:56] ENGINE HTTP Server cherrypy._cpwsgi_server.CPWSGIServer(('127.0.0.1', 44109)) shut down
pypy3: exit 1 (2.03 seconds) /tmp/cachecontrol> py.test tests/ pid=330914
.pkg: _exit> python /usr/lib/python3.12/site-packages/pyproject_api/_backend.py True flit_core.buildapi
  pypy3: FAIL code 1 (4.31=setup[2.28]+cmd[2.03] seconds)
  evaluation failed :( (4.38 seconds)

They pass on CPython 3.10. All tests used to pass in 0.14.1.

[mgorny]

Adding a gc.collect() after resp = None fixes this, but I'm not sure if this is guaranteed to work.

[woodruffw]

Thanks for the report @mgorny!

I suspect this is caused by #344, which both added that test and also added the weakref behavior.

CC @Tasssadar for thoughts on how to address this -- I suppose we might need a different patch (or no patch at all) in the PyPy case.

[Tasssadar]

Yes, PyPy behaves differently here - I think adding gc.collect() there is a correct fix, since it does not use refcounting we need to force the GC to simulate the runtime behavior, and docs look like it should be reliable.

[woodruffw]

OK, makes sense. Would you be interested in sending a patch for that? Otherwise I can do in a bit.

[Tasssadar]

Unless I'm saving you some real work, then please do it, it will probably be faster for you since you have the dev environment and everything set up, thanks :)

[woodruffw]

#352 has a potential fix. @mgorny could you give that a try?