Skip to content

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

Description

@deeprave

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions