omnilib / aiosqlite

asyncio bridge to the standard sqlite3 module
https://aiosqlite.omnilib.dev
MIT License
1.22k stars 96 forks source link

Connections not closing when async tasks are cancelled #259

Open PartlyAtomic opened 1 year ago

PartlyAtomic commented 1 year ago

Description

When a task is cancelled while an aiosqlite.Connection object is connecting, the sqlite connection is never closed and is left hanging. Additionally the aiosqlite.Connection threads don't get shut down.

Details

Repro code:

import asyncio

import aiosqlite

async def connect_to_db():
    async with aiosqlite.connect("test.db") as db:
        await db.execute("SELECT value FROM generate_series(0,1000000,1)")

async def main_loop():
    tasks = []

    for i in range(5):
        tasks.append(asyncio.create_task(connect_to_db()))
    print("Tasks started")

    await asyncio.sleep(0)
    print("Async tick")

    for task in tasks:
        task.cancel()
    print("Tasks cancelled")

    for task in tasks:
        try:
            await task
        except asyncio.CancelledError:
            continue

    # This was a debug helper to track open connections
    # from aiosqlite.core import print_connections_info
    # print_connections_info()

    await asyncio.sleep(5)
    print("Complete")

if __name__ == "__main__":
    asyncio.run(main_loop())

Expected behavior: File handles for test.db would be released as tasks were successfully cancelled.

Actual behavior: 5 file handles are kept indefinitely for test.db (can be verified with a program such as OpenedFilesView or keeping track of the sqlite connections). Additionally after the event loop stops, the script hangs.

image

PartlyAtomic commented 1 year ago

I suspect there may be other problems when tasks are cancelled, but haven't figured out minimal test cases for them yet. My program was often getting into a state where a series of cancelled write tasks would eventually lock the database indefinitely.

iamthebull commented 1 month ago

You may not be handling the CancelledError properly. See here.

This exception can be caught to perform custom operations when asyncio Tasks are cancelled. In almost all situations the exception must be re-raised.