Skip to content

Commit 2db72d6

Browse files
committed
fix: engine too many connections
Fix create_engine() connection leaks per request/call
1 parent 513ff6d commit 2db72d6

5 files changed

Lines changed: 71 additions & 43 deletions

File tree

tests/browser/db_helpers.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
from datetime import timedelta
99

1010
from dotenv import load_dotenv
11-
from sqlmodel import Session, create_engine, select
11+
from sqlmodel import Session, select
1212

1313
from utils.core.auth import get_password_hash
14-
from utils.core.db import create_default_roles, get_connection_url
14+
from utils.core.db import create_default_roles, get_engine
1515
from utils.core.models import (
1616
Account,
1717
Invitation,
@@ -39,8 +39,7 @@ def browser_db_session():
3939
saved = {key: os.environ.get(key) for key in env}
4040
os.environ.update(env)
4141
try:
42-
engine = create_engine(get_connection_url())
43-
with Session(engine) as session:
42+
with Session(get_engine()) as session:
4443
yield session
4544
finally:
4645
for key, value in saved.items():
@@ -168,8 +167,7 @@ def browser_csrf_db_session():
168167
saved = {key: os.environ.get(key) for key in env}
169168
os.environ.update(env)
170169
try:
171-
engine = create_engine(get_connection_url())
172-
with Session(engine) as session:
170+
with Session(get_engine()) as session:
173171
yield session
174172
finally:
175173
for key, value in saved.items():

tests/conftest.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
from typing import Generator, cast
44

55
pytest_plugins = ["tests.frontend.fixtures"]
6-
from sqlmodel import create_engine, Session, select
6+
from sqlmodel import Session, select
77
from fastapi.testclient import TestClient
88
from dotenv import load_dotenv
99
from utils.core.db import (
1010
get_connection_url,
11+
get_engine,
1112
tear_down_db,
1213
set_up_db,
1314
create_default_roles,
@@ -74,9 +75,12 @@ def engine(env_vars):
7475
Create a new SQLModel engine for the test database.
7576
Use PostgreSQL for testing to match production environment.
7677
"""
77-
# Use PostgreSQL for testing to match production environment
78+
# Use PostgreSQL for testing to match production environment. get_engine()
79+
# returns the same cached pool the app itself uses via get_session(), so
80+
# TestClient requests and direct test queries share one pool instead of
81+
# opening a second one for the same database.
7882
ensure_database_exists(get_connection_url())
79-
engine = create_engine(get_connection_url())
83+
engine = get_engine()
8084
set_up_db(drop=True)
8185

8286
yield engine

utils/core/auth.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from fastapi.templating import Jinja2Templates
1414
from fastapi import Cookie
1515
from starlette.responses import Response
16-
from utils.core.db import create_engine, get_connection_url
16+
from utils.core.db import get_engine
1717
from utils.core.models import (
1818
AccountRecoveryToken,
1919
EmailVerificationToken,
@@ -339,8 +339,7 @@ def send_reset_email_task(email: str) -> None:
339339
FastAPI background tasks should not reuse request-scoped resources from
340340
`yield` dependencies, because cleanup may run before the task executes.
341341
"""
342-
engine = create_engine(get_connection_url())
343-
with Session(engine) as session:
342+
with Session(get_engine()) as session:
344343
send_reset_email(email, session)
345344

346345

utils/core/db.py

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import logging
33
from itertools import chain
44
from typing import Union, Sequence
5-
from sqlalchemy.engine import URL
5+
from sqlalchemy.engine import Engine, URL
66
from sqlmodel import create_engine, Session, SQLModel, select, text
77
from utils.core.models import (
88
Account,
@@ -28,17 +28,42 @@
2828
# --- Database connection functions ---
2929

3030

31+
_engine_cache: dict[str, Engine] = {}
32+
33+
34+
def get_engine() -> Engine:
35+
"""Returns a process-wide cached engine for the current connection URL.
36+
37+
Calling create_engine() per request/call leaks pooled connections until
38+
Python's GC reclaims them (engines aren't freed by simple refcounting),
39+
which can exhaust Postgres's max_connections under sustained load. Cache
40+
by URL rather than as a single global so tests that swap DB_NAME within
41+
the same process (e.g. browser vs. browser-csrf DBs) still get one
42+
engine per database instead of sharing a mismatched pool.
43+
"""
44+
url = get_connection_url()
45+
key = str(url)
46+
engine = _engine_cache.get(key)
47+
if engine is None:
48+
engine = create_engine(url)
49+
_engine_cache[key] = engine
50+
return engine
51+
52+
3153
def ensure_database_exists(url: URL) -> None:
3254
dbname = url.database
3355
server_url = url.set(database="postgres")
3456
engine = create_engine(server_url, isolation_level="AUTOCOMMIT")
35-
with engine.connect() as conn:
36-
exists = conn.execute(
37-
text("SELECT 1 FROM pg_database WHERE datname = :n"),
38-
{"n": dbname},
39-
).scalar()
40-
if not exists:
41-
conn.execute(text(f'CREATE DATABASE "{dbname}"'))
57+
try:
58+
with engine.connect() as conn:
59+
exists = conn.execute(
60+
text("SELECT 1 FROM pg_database WHERE datname = :n"),
61+
{"n": dbname},
62+
).scalar()
63+
if not exists:
64+
conn.execute(text(f'CREATE DATABASE "{dbname}"'))
65+
finally:
66+
engine.dispose()
4267

4368

4469
def get_connection_url() -> URL:
@@ -259,28 +284,32 @@ def set_up_db(drop: bool = False) -> None:
259284
drop (bool): If True, drops all existing tables before creating new ones.
260285
"""
261286
engine = create_engine(get_connection_url())
262-
if drop:
263-
SQLModel.metadata.drop_all(engine)
264-
# Ensure the private schema exists before creating tables
265-
with engine.connect() as conn:
266-
conn.execute(text("CREATE SCHEMA IF NOT EXISTS private"))
267-
conn.commit()
268-
SQLModel.metadata.create_all(engine)
269-
# Create default permissions and seed account emails
270-
with Session(engine) as session:
271-
create_permissions(session)
272-
session.commit()
273-
seed_account_emails(session)
274-
engine.dispose()
287+
try:
288+
if drop:
289+
SQLModel.metadata.drop_all(engine)
290+
# Ensure the private schema exists before creating tables
291+
with engine.connect() as conn:
292+
conn.execute(text("CREATE SCHEMA IF NOT EXISTS private"))
293+
conn.commit()
294+
SQLModel.metadata.create_all(engine)
295+
# Create default permissions and seed account emails
296+
with Session(engine) as session:
297+
create_permissions(session)
298+
session.commit()
299+
seed_account_emails(session)
300+
finally:
301+
engine.dispose()
275302

276303

277304
def tear_down_db() -> None:
278305
"""
279306
Tears down the database by dropping all tables and the private schema.
280307
"""
281308
engine = create_engine(get_connection_url())
282-
SQLModel.metadata.drop_all(engine)
283-
with engine.connect() as conn:
284-
conn.execute(text("DROP SCHEMA IF EXISTS private CASCADE"))
285-
conn.commit()
286-
engine.dispose()
309+
try:
310+
SQLModel.metadata.drop_all(engine)
311+
with engine.connect() as conn:
312+
conn.execute(text("DROP SCHEMA IF EXISTS private CASCADE"))
313+
conn.commit()
314+
finally:
315+
engine.dispose()

utils/core/dependencies.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
oauth2_scheme_cookie,
1414
verify_password,
1515
)
16-
from utils.core.db import create_engine, get_connection_url
16+
from utils.core.db import get_engine
1717
from utils.core.models import (
1818
User,
1919
Role,
@@ -43,8 +43,7 @@ def get_session() -> Generator[Session, None, None]:
4343
Yields:
4444
Session: A SQLModel session object for database operations.
4545
"""
46-
engine = create_engine(get_connection_url())
47-
with Session(engine) as session:
46+
with Session(get_engine()) as session:
4847
yield session
4948

5049

@@ -434,8 +433,7 @@ async def get_user_from_request(request: Request) -> Optional[User]:
434433
tokens = (access_token, refresh_token)
435434

436435
# Get a database session
437-
engine = create_engine(get_connection_url())
438-
with Session(engine) as session:
436+
with Session(get_engine()) as session:
439437
user, new_access_token, new_refresh_token = get_user_from_tokens(
440438
tokens, session
441439
)

0 commit comments

Comments
 (0)