[Ops] Celery tasks using the pooled AsyncSessionLocal can bind asyncpg futures to a dead event loop #129

New issue

Closed

opened 2026-07-14 20:52:30 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-07-14 20:52:30 +00:00

Contributor

Context

Found while implementing #109 (recap email escaping), not fixed there — it is out of that issue's scope and deserves its own change.

app/database.py provides task_session() specifically so Celery tasks get a NullPool, throw-away engine per call. Celery tasks spin up a fresh event loop each invocation (asyncio.run(...) / new_event_loop()), and a pooled connection created under one loop must never be reused under another.

Most tasks in app/tasks/reminder_tasks.py correctly use task_session(). At least these do not, and instead use the pooled global AsyncSessionLocal:

send_recap_email
auto_close_voting / _auto_close_voting_async

Current behavior

In a long-lived Celery prefork worker, a pooled connection opened under a previous task's (now-closed) event loop can be handed to a later task running under a new loop. asyncpg then raises:

Future ... attached to a different loop

This reproduces readily: driving send_recap_email under test hit it immediately, and the test only passed once a NullPool sessionmaker was injected. In production this presents as intermittent, hard-to-reproduce recap-email and vote-auto-close failures that clear up after a worker restart — exactly the kind of flake that gets misattributed to SMTP or Discord.

Fix / Spec

Sweep app/tasks/ for every use of AsyncSessionLocal inside a Celery task body and switch each to task_session().
Confirm no remaining task-side import of AsyncSessionLocal. (_cleanup_trashed_audio_async should be checked too — it opens AsyncSessionLocal() directly.)
Consider a guard that makes the wrong thing hard to do: e.g. have the task decorator/base class provide the session, or add a lint/test that fails if AsyncSessionLocal is referenced anywhere under app/tasks/.

Acceptance criteria

No Celery task constructs a session from the pooled AsyncSessionLocal.
A regression test that would catch a reintroduction (grep-style assertion, or a task-level test exercising two sequential invocations across distinct event loops).
Backend suite green.

References

webapp/backend/app/database.py (task_session, NullPool rationale)
webapp/backend/app/tasks/reminder_tasks.py — send_recap_email, auto_close_voting / _auto_close_voting_async, _cleanup_trashed_audio_async

Found during the v3.3.0 hardening pass (#109).

## Context Found while implementing #109 (recap email escaping), not fixed there — it is out of that issue's scope and deserves its own change. `app/database.py` provides `task_session()` specifically so Celery tasks get a **NullPool, throw-away engine per call**. Celery tasks spin up a fresh event loop each invocation (`asyncio.run(...)` / `new_event_loop()`), and a pooled connection created under one loop must never be reused under another. Most tasks in `app/tasks/reminder_tasks.py` correctly use `task_session()`. At least these do not, and instead use the **pooled global `AsyncSessionLocal`**: - `send_recap_email` - `auto_close_voting` / `_auto_close_voting_async` ## Current behavior In a long-lived Celery prefork worker, a pooled connection opened under a previous task's (now-closed) event loop can be handed to a later task running under a new loop. asyncpg then raises: ``` Future ... attached to a different loop ``` This reproduces readily: driving `send_recap_email` under test hit it immediately, and the test only passed once a NullPool sessionmaker was injected. In production this presents as intermittent, hard-to-reproduce recap-email and vote-auto-close failures that clear up after a worker restart — exactly the kind of flake that gets misattributed to SMTP or Discord. ## Fix / Spec 1. Sweep `app/tasks/` for every use of `AsyncSessionLocal` inside a Celery task body and switch each to `task_session()`. 2. Confirm no remaining task-side import of `AsyncSessionLocal`. (`_cleanup_trashed_audio_async` should be checked too — it opens `AsyncSessionLocal()` directly.) 3. Consider a guard that makes the wrong thing hard to do: e.g. have the task decorator/base class provide the session, or add a lint/test that fails if `AsyncSessionLocal` is referenced anywhere under `app/tasks/`. ## Acceptance criteria - No Celery task constructs a session from the pooled `AsyncSessionLocal`. - A regression test that would catch a reintroduction (grep-style assertion, or a task-level test exercising two sequential invocations across distinct event loops). - Backend suite green. ## References - `webapp/backend/app/database.py` (`task_session`, NullPool rationale) - `webapp/backend/app/tasks/reminder_tasks.py` — `send_recap_email`, `auto_close_voting` / `_auto_close_voting_async`, `_cleanup_trashed_audio_async` _Found during the v3.3.0 hardening pass (#109)._

claude-bot added this to the v3.4.0 - Reliability, Operations & Self-Hosting milestone

2026-07-14 20:52:38 +00:00

claude-bot added the

backend

bug

labels

2026-07-14 20:52:46 +00:00

claude-bot referenced this issue

2026-07-14 21:33:58 +00:00

[Hardening] Escape HTML in recap emails; stop echoing raw LLM errors from /ask #109