tortoise / tortoise/tortoise-orm

ConnectionHandler.close_all can leak cross-loop connections after loop-switch reconnect

Open
#2,240 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
5.6k
Forks
516
Avg merge
2d 21h
Merged PRs (30d)
9

Description

Summary

ConnectionHandler.close_all(discard=True) can fail to close an existing
connection if it is called from a different event loop than the one that opened
the connection.

The current close path calls ConnectionHandler.all(), and all() calls
ConnectionHandler.get(alias). Since #2098, get() is no longer a pure lookup:
when it sees that the stored connection was bound to another event loop, it
creates a replacement connection and stores that replacement under the same
alias.

That is useful during normal connection access, but it is unsafe during
shutdown. close_all() closes the replacement, discards the alias, and loses
the original connection without closing it.

For SQLite this leaves the original aiosqlite worker thread alive. In pytest
or ASGI test-client usage this can later surface as:

PytestUnhandledThreadExceptionWarning: Exception in thread ... (_connection_worker_thread)
RuntimeError: Event loop is closed

Environment

Observed with:

  • tortoise-orm==1.1.7
  • aiosqlite==0.22.1
  • Python 3.14.6

Minimal reproducer

import asyncio
import tempfile
from pathlib import Path

from tortoise import Tortoise, fields
from tortoise.models import Model


class ReproThing(Model):
    id = fields.IntField(primary_key=True)


async def create_and_use_connection(database_path: Path):
    config = {
        "connections": {"default": f"sqlite://{database_path.as_posix()}"},
        "apps": {
            "models": {
                "models": ["__main__"],
                "default_connection": "default",
            }
        },
    }
    context = await Tortoise.init(config=config)
    with context:
        original = context.connections.get("default")
        await original.execute_query("select 1")
        print(f"loop A: {id(asyncio.get_running_loop())}")
        print(f"original client: {id(original)}")
        print(f"original bound loop: {id(original._bound_loop)}")
        return context, original


async def close_on_second_loop(context, original):
    with context:
        print(f"loop B: {id(asyncio.get_running_loop())}")
        print(
            "stored before close_all:",
            [id(client) for client in context.connections._copy_storage().values()],
        )
        try:
            await context.connections.close_all(discard=True)
            print(
                "stored after close_all:",
                [id(client) for client in context.connections._copy_storage().values()],
            )
            print(f"original closed by close_all: {original._connection is None}")
            assert original._connection is None, (
                "close_all discarded but did not close original"
            )
        finally:
            # Without this manual cleanup, the leaked aiosqlite worker can keep
            # the process alive or report back to a closed event loop.
            if original._connection is not None:
                await original.close()


with tempfile.TemporaryDirectory() as temp_dir:
    path = Path(temp_dir) / "repro.sqlite3"
    ctx, original_connection = asyncio.run(create_and_use_connection(path))
    asyncio.run(close_on_second_loop(ctx, original_connection))

Representative output:

.../tortoise/connection.py:262: TortoiseLoopSwitchWarning:
Tortoise connection 'default' was created on a different event loop and will be reconnected.
  return [self.get(alias) for alias in self.db_config]
loop A: 4483643584
original client: 4490190128
original bound loop: 4483643584
loop B: 4490527760
stored before close_all: [4490190128]
stored after close_all: []
original closed by close_all: False
AssertionError: close_all discarded but did not close original

Expected behaviour

close_all(discard=True) should close already-created connections in the
current connection context and then discard them.

It should not create, replace, or reconnect connections during shutdown. It also
should not discard a stored alias while leaving the original connection open.

This preserves current-context semantics: close_all() does not need to close
connections in all contexts or all event loops. It only needs to avoid
replacing the stored clients in the current context while closing that current
context.

Actual behaviour

close_all(discard=True):

  1. Calls ConnectionHandler.all().
  2. all() calls ConnectionHandler.get(alias) for each configured alias.
  3. get(alias) detects the loop switch and creates a new client.
  4. The new client replaces the original in storage.
  5. close_all() closes the new client.
  6. close_all() discards the alias.
  7. The original aiosqlite connection remains open and is no longer reachable
    through the connection handler.

Suggested fix

close_all() should not call get() during shutdown. It should iterate over
already-stored connection objects in the current context without acquiring,
creating, or replacing anything.

Conceptually:

async def close_all(self, discard: bool = True) -> None:
    if self._db_config is None:
        return

    storage = self._copy_storage()
    connections = [
        storage[alias]
        for alias in self.db_config
        if alias in storage
    ]
    await asyncio.gather(*(connection.close() for connection in connections))

    if discard:
        for alias in self.db_config:
            if alias in storage:
                self.discard(alias)

The important property is that shutdown uses a non-acquiring view of current
context storage.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in tortoise/connection.py around ConnectionHandler.close_all, then inspect all(), get(), _copy_storage(), and discard(). Use the supplied two-event-loop reproducer to observe the current behavior. Done means shutdown closes the already-stored current-context connections without creating replacements, then discards their aliases without leaving the original connection open.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, sqlite
Domain
database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.