Skip to content

Commit 2986173

Browse files
fix: keep Session off the thread pool in update_profile
Offload only CPU-bound image processing, convert remaining falsely-async routes, and type the async test helper. Follow-up: #237.
1 parent 750dbe0 commit 2986173

4 files changed

Lines changed: 31 additions & 46 deletions

File tree

main.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,7 @@ async def general_exception_handler(request: Request, exc: Exception):
393393

394394

395395
@app.get("/")
396-
async def read_home(
397-
request: Request, _: None = Depends(require_unauthenticated_client)
398-
):
396+
def read_home(request: Request, _: None = Depends(require_unauthenticated_client)):
399397
return templates.TemplateResponse(request, "index.html", {"user": None})
400398

401399

routers/core/static_pages.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717

1818
@router.get("/{page_name}", name="read_static_page")
19-
async def read_static_page(
19+
def read_static_page(
2020
page_name: str, request: Request, user: Optional[User] = Depends(get_optional_user)
2121
):
2222
"""

routers/core/user.py

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -131,26 +131,33 @@ async def update_profile(
131131
session: Session = Depends(get_session),
132132
):
133133
avatar_changed = bool(avatar_file and avatar_file.filename)
134-
avatar_data: Optional[bytes] = None
135-
avatar_content_type: Optional[str] = None
136134

137-
# Async chunked read must stay on the event loop; sync image/DB work is offloaded.
135+
# Async upload read stays on the event loop. CPU-bound image work is
136+
# offloaded; Session/ORM mutations stay here (do not pass Session into the
137+
# thread pool — see issue #237).
138138
if avatar_changed:
139139
assert avatar_file is not None
140140
reject_oversized_content_length(
141141
request.headers.get("content-length"), MAX_AVATAR_UPLOAD_BYTES
142142
)
143143
avatar_data = await read_upload_with_size_limit(avatar_file, MAX_FILE_SIZE)
144-
avatar_content_type = avatar_file.content_type
144+
processed_image, content_type = await run_in_threadpool(
145+
validate_and_process_image, avatar_data, avatar_file.content_type
146+
)
147+
if user.avatar:
148+
user.avatar.avatar_data = processed_image
149+
user.avatar.avatar_content_type = content_type
150+
else:
151+
assert user.id is not None
152+
user.avatar = UserAvatar(
153+
user_id=user.id,
154+
avatar_data=processed_image,
155+
avatar_content_type=content_type,
156+
)
145157

146-
await run_in_threadpool(
147-
_apply_profile_update,
148-
session,
149-
user,
150-
name,
151-
avatar_data,
152-
avatar_content_type,
153-
)
158+
user.name = name
159+
session.commit()
160+
session.refresh(user)
154161

155162
if is_htmx_request(request):
156163
response = templates.TemplateResponse(
@@ -176,34 +183,6 @@ async def update_profile(
176183
return RedirectResponse(url=router.url_path_for("read_profile"), status_code=303)
177184

178185

179-
def _apply_profile_update(
180-
session: Session,
181-
user: User,
182-
name: Optional[str],
183-
avatar_data: Optional[bytes],
184-
avatar_content_type: Optional[str],
185-
) -> None:
186-
"""Sync image processing and DB persistence for update_profile."""
187-
if avatar_data is not None:
188-
processed_image, content_type = validate_and_process_image(
189-
avatar_data, avatar_content_type
190-
)
191-
if user.avatar:
192-
user.avatar.avatar_data = processed_image
193-
user.avatar.avatar_content_type = content_type
194-
else:
195-
assert user.id is not None
196-
user.avatar = UserAvatar(
197-
user_id=user.id,
198-
avatar_data=processed_image,
199-
avatar_content_type=content_type,
200-
)
201-
202-
user.name = name
203-
session.commit()
204-
session.refresh(user)
205-
206-
207186
@router.post("/communication-preferences", response_class=RedirectResponse)
208187
def update_communication_preferences(
209188
request: Request,

tests/utils/test_dependencies.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from concurrent.futures import ThreadPoolExecutor
33
from unittest.mock import MagicMock, patch
44
from datetime import datetime, timedelta, UTC
5+
from typing import Any, Coroutine, TypeVar
56
from starlette.concurrency import run_in_threadpool
67
from starlette.requests import Request
78
from utils.core.models import (
@@ -715,14 +716,21 @@ def _request_with_auth_cookies(
715716
)
716717

717718

718-
def _run_async(coro): # type: ignore[no-untyped-def]
719+
T = TypeVar("T")
720+
721+
722+
def _run_async(coro: Coroutine[Any, Any, T]) -> T:
719723
"""Drive an async helper from sync tests.
720724
721725
The full suite may already have an event loop (e.g. after Playwright), so
722726
asyncio.run() on the main thread can fail. Always run in a fresh thread.
723727
"""
728+
729+
def _runner() -> T:
730+
return asyncio.run(coro)
731+
724732
with ThreadPoolExecutor(max_workers=1) as pool:
725-
return pool.submit(asyncio.run, coro).result()
733+
return pool.submit(_runner).result()
726734

727735

728736
def test_get_user_from_request_resolves_user_via_threadpool() -> None:

0 commit comments

Comments
 (0)