tortoise / tortoise/tortoise-orm
ConnectionHandler.close_all can leak cross-loop connections after loop-switch reconnect
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.7aiosqlite==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):
- Calls
ConnectionHandler.all(). all()callsConnectionHandler.get(alias)for each configured alias.get(alias)detects the loop switch and creates a new client.- The new client replaces the original in storage.
close_all()closes the new client.close_all()discards the alias.- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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