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

Memory Leak Due to Cyclic References in Traceback #10535

Open
1 task done
availov opened this issue Mar 10, 2025 · 3 comments
Open
1 task done

Memory Leak Due to Cyclic References in Traceback #10535

availov opened this issue Mar 10, 2025 · 3 comments
Labels

Comments

@availov
Copy link

availov commented Mar 10, 2025

Describe the bug

A potential memory leak has been observed in aiohttp when handling streaming responses. The following script reproduces the issue by continuously reading a response stream from an aiohttp server. After the client disconnects due to ServerDisconnectedError, garbage collection (gc) still reports lingering ClientResponse objects, suggesting a resource leak due to cyclic references in traceback objects.

To Reproduce

  1. Run the following script.
  2. Observe the output for lingering ClientResponse objects, which are visualized using objgraph.
import asyncio
import aiohttp
from aiohttp import web
import gc
from time import time
import objgraph

gc.set_debug(gc.DEBUG_LEAK)


def get_garbage():
    result = []
    gc.collect()
    for obj in gc.garbage:
        obj_name = type(obj).__name__
        result.append(f'{obj_name}')
        if obj_name in ('ClientResponse',):
            print('ClientResponse not collected!')
            objgraph.show_backrefs(
                obj,
                max_depth=30,
                too_many=50,
                filename=f"/tmp/{int(time() * 1000)}err_referrers.png",
            )

    return result


class Client:
    def __init__(self):
        self.session = aiohttp.ClientSession()
        self.response = None

    async def fetch_stream(self, url):
        try:
            self.response = await self.session.get(url)
            if self.response.status == 200:
                while True:
                    chunk = await self.response.content.readexactly(6)
                    print(f'received: {chunk.decode().strip()}')
            else:
                print(f'response status code: {self.response.status}')
        except (
            aiohttp.ClientConnectorError,
            aiohttp.ServerDisconnectedError,
            aiohttp.ClientPayloadError,
            asyncio.IncompleteReadError
        ) as e:
            print(f'connection error ({type(e).__name__})')
        except Exception as e:
            print(f'unexpected error: {e}')
        finally:
            self.response = None  # Explicitly clear response
            self.session = None  # This should close the session, but memory leak persists due to traceback references.


async def stream_handler(request):
    writer = request.transport
    if writer:
        writer.close()  # Forcefully closing connection

    return web.Response()


async def main():
    app = web.Application()
    app.router.add_get('/stream', stream_handler)

    runner = web.AppRunner(app)
    await runner.setup()
    site = web.TCPSite(runner, 'localhost', 8080)
    await site.start()

    client = Client()
    client_task = asyncio.create_task(client.fetch_stream('http://localhost:8080/stream'))

    await client_task
    await asyncio.sleep(0.5)  # Allow time for cleanup

    print(f'Garbage objects: {get_garbage()}')

    await runner.cleanup()


if __name__ == '__main__':
    asyncio.run(main())

Expected behavior

  • ClientResponse should be properly garbage collected after the session is closed.
  • No lingering references should prevent cleanup.

Logs/tracebacks

Observe the output for lingering ClientResponse objects, which are visualized using objgraph.

Python Version

3.12.9

aiohttp Version

3.11.11

multidict Version

6.1.0

propcache Version

0.2.0

yarl Version

1.17.1

OS

Ubuntu 22.04.5 LTS

Related component

Client

Additional context

Fixed Version FixClientResponse (Memory Leak Resolved)

import asyncio
import aiohttp
from aiohttp import web
import gc
from time import time
import objgraph

gc.set_debug(gc.DEBUG_LEAK)

def get_garbage():
    result = []
    gc.collect()
    for obj in gc.garbage:
        obj_name = type(obj).__name__
        result.append(f'{obj_name}')
        if obj_name in ('ClientResponse',):
            objgraph.show_backrefs(
                obj,
                max_depth=30,
                too_many=50,
                filename=f"/tmp/{int(time() * 1000)}err_referrers.png",
            )

    return result


class FixClientResponse(aiohttp.ClientResponse):
    def close(self):
        if self._connection is None:
            return

        if self._connection.protocol is None:
            return

        self._connection.protocol._exception = None  # Break cyclic references

        super().close()


class Client:
    def __init__(self):
        self.session = aiohttp.ClientSession(response_class=FixClientResponse)
        self.response = None

    async def fetch_stream(self, url):
        try:
            self.response = await self.session.get(url)
            if self.response.status == 200:
                while True:
                    chunk = await self.response.content.readexactly(6)
                    print(f'received: {chunk.decode().strip()}')
            else:
                print(f'response status code: {self.response.status}')
        except (
            aiohttp.ClientConnectorError,
            aiohttp.ServerDisconnectedError,
            aiohttp.ClientPayloadError,
            asyncio.IncompleteReadError
        ) as e:
            print(f'connection error ({type(e).__name__})')
        except Exception as e:
            print(f'unexpected error: {e}')
        finally:
            self.response = None
            self.session = None


async def stream_handler(request):
    writer = request.transport
    if writer:
        writer.close()  # Forcefully closing connection

    return web.Response()


async def main():
    app = web.Application()
    app.router.add_get('/stream', stream_handler)

    runner = web.AppRunner(app)
    await runner.setup()
    site = web.TCPSite(runner, 'localhost', 8080)
    await site.start()

    client = Client()
    client_task = asyncio.create_task(client.fetch_stream('http://localhost:8080/stream'))

    await client_task
    await asyncio.sleep(0.5)  # Allow time for cleanup

    print(f'Garbage objects: {get_garbage()}')

    await runner.cleanup()


if __name__ == '__main__':
    asyncio.run(main())

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@availov availov added the bug label Mar 10, 2025
@availov
Copy link
Author

availov commented Mar 10, 2025

Image

@bdraco
Copy link
Member

bdraco commented Mar 10, 2025

It certainly wouldn't be the first time I've seen problems with exception traces holding on to references.

It looks like the fix is self._connection.protocol._exception = None # Break cyclic references

That seems reasonable to me. Would you like to open a PR with that change?

@availov
Copy link
Author

availov commented Mar 11, 2025

Sure, I'll open a PR with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants