MagicStack / asyncpg

A fast PostgreSQL Database Client Library for Python/asyncio.
Apache License 2.0
6.88k stars 399 forks source link

Continuous memory increase in Sanic web server after calling `loop.start_tls(...)` in `connect_utils._create_ssl_connection` #1004

Open LCBHSStudent opened 1 year ago

LCBHSStudent commented 1 year ago

Background: I built a simple web server using sqlalchemy and sanic, then created a request handler function that can query my local PostgreSQL server running in docker. However, I saw continuous memory increase while benching my handler function.

After running tracemalloc for help, I got the result below:

Top 10 lines
#1: /home/tusimple/Workspace/projects/map-workdata-manager/venv/lib/python3.8/site-packages/asyncpg/connect_utils.py:700: 5670.1 KiB
    new_tr = await loop.start_tls(

The code for creating the async_engine is:

from sqlalchemy.ext.asyncio import create_async_engine
engine = create_async_engine("postgresql+asyncpg://{}:{}@{}/{}".format(...))

At first, I tried adding ?ssl=disable after the previous connect string in order to prevent connect_utils._create_ssl_connection calls, and it works. Then, I tried to make some stupid changes to the source code of asyncpg, because I think this SSL transport is not closed after creating new transport. (I'm not sure is that correct)

# line 697 
    if hasattr(loop, 'start_tls'):
        if do_ssl_upgrade:
            try:
                new_tr = await loop.start_tls(
                    tr, pr, ssl_context, server_hostname=host)        # <--- here
            except (Exception, asyncio.CancelledError):
                tr.close()
                raise
        else:
            new_tr = tr
# line 706

Finally, I modified the code starts from line 702 to:

            except (Exception, asyncio.CancelledError):
                raise
            finally:
                tr.close()

After this modification, my web server keeps working fine, but I don't know whether this is a proper modification.

If you have any questions or ideas, please let me know, thank you so much!

stalkerg commented 8 months ago

Same issue with Tornado. Basically, even if connection doesn't expect SSL the asyncpg try to use it. As we know, if SSL connection was cancel it provide leak - https://github.com/python/cpython/issues/109534#issuecomment-1865257133

stalkerg commented 8 months ago

Also, seems like this line https://github.com/MagicStack/asyncpg/blob/master/asyncpg/connection.py#L471 is leak. (sorry, it's different issue I believe)