RazerM/sqlalchemy_aio

Thread workers are not deleted after AsyncEngine becomes garbage collected

MrP4p3r opened this issue · 4 comments

...if I intentionally leave AsyncConnection un-closed.

Also this leads to connection pool overflow for some reason.

Is it possible to handle garbage collection for AsyncConnection to close underlying SQLAlchemy connection and call thread_worker.quit()?

My advice with sqlalchemy_aio is to close connections explicitly, which is most easily done using the async context managers.

I don't think we can do anything async to clean up like happens with normal SQLAlchemy during garbage collection, but I could be wrong.

Using context manager is not really suitable for my case unfortunately. :/
I think this can be implemented using weakref.finalize and asyncio.create_task.
I also tried to define a AsyncConnection.__del__ method. This is not a good solution but it worked for me.

@MrP4p3r were you able to come up with a code diff to achieve this or a pull request? I'd be interested in such a fix.

@skewty Yesterday I tried again to play with sqlalchemy_aio code and decided that the solution I suggested applied to sqlalchemy_aio code is a dirty hack.

I used this hotfix in my project.

engine: AsyncEngine = ...

async def connect() -> AsyncConnection:
    connection = await engine.connect()
    sync_connection = connection.sync_connection
    thread_worker = connection._worker
    async def _auto_close():
        if not sync_connection.closed:
            await thread_worker.run(sync_connection.close)
            await thread_worker.quit()
    weakref.finalize(connection, lambda: asyncio.create_task(_auto_close()))
    return connection

This is not a good solution since _auto_close may fail for some reason, created task should be awaited, and so on...

It seems the problem can be solved gently only if __adel__() method is introduced.