fix: use try/finally in get_db to prevent PostgreSQL connection leaks

[Vacbo]

Problem

The current get_db() implementation uses with Session(engine) as session: yield session. Under high load with concurrent requests that raise exceptions, this pattern causes PostgreSQL connections to leak into an idle in transaction state, eventually exhausting the connection pool and resulting in:

FATAL: sorry, too many clients already

This happens because:

1. The generator pattern with yield pauses execution in the route handler
2. When an exception is raised in the route, the with context manager's cleanup depends on garbage collection timing
3. Under high concurrency (many simultaneous exceptions), cleanup is delayed or incomplete
4. PostgreSQL accumulates connections in idle in transaction state
5. The connection pool (pool_size + max_overflow) fills up, blocking new requests

Root Cause

The with Session(engine) as session context manager relies on __exit__ being called when control flow exits the with block. However, with generator-based dependency injection:

def get_db() -> Generator[Session, None, None]:
    with Session(engine) as session:
        yield session  # execution pauses here, context manager __exit__ not yet called

When the route handler raises an exception after yield, the __exit__ cleanup is deferred until Python's garbage collector runs. Under load, this deferral causes connections to accumulate faster than they can be cleaned up.

Verification

Load testing with bombardier (300 concurrent connections, 30 seconds):

Pattern	Endpoint	idle_in_transaction	Total Connections	Result
with Session(engine) as session: yield session	/error (raises)	5	53	LEAKED ❌
with Session(engine) as session: yield session	/ok (success)	2	88	LEAKED ❌
try/finally + session.close()	/error (raises)	0	8	CLEAN ✅
try/finally + session.close()	/ok (success)	0	8	CLEAN ✅

Solution

# OLD - leaks connections under load
def get_db() -> Generator[Session, None, None]:
    with Session(engine) as session:
        yield session
# NEW - guaranteed cleanup
def get_db() -> Generator[Session, None, None]:
    session = Session(engine)
    try:
        yield session
    finally:
        session.close()

The try/finally pattern ensures session.close() is always called immediately when the generator finishes, regardless of whether the route raised an exception. This guarantees connections are returned to the pool promptly.

Example Routes Used in Testing

@app.post("/error")
def error_endpoint(db: Session = Depends(get_db)):
    """Raises exception - exposes the connection leak with 'with Session' pattern."""
    db.exec(text("INSERT INTO company (company_name, timezone) VALUES ('test', 'UTC')"))
    db.commit()
    raise ValueError("boom")
@app.post("/ok")
def ok_endpoint(db: Session = Depends(get_db)):
    """Successful endpoint - can also leak connections under high concurrency."""
    db.exec(text("INSERT INTO company (company_name, timezone) VALUES ('test', 'UTC')"))
    db.commit()
    return {"ok": True}
@app.post("/explicit-rollback")
def explicit_rollback(db: Session = Depends(get_db)):
    """Explicit transaction control - try/finally still guarantees cleanup."""
    db.begin()
    try:
        db.exec(text("INSERT INTO company (company_name, timezone) VALUES ('test', 'UTC')"))
        db.commit()
    except Exception:
        db.rollback()
        raise

Load Test Command

I used bombardier to make a simple load test and reproduce the scenario:

bombardier -c 300 -d 30s -m POST -H "Content-Type: application/json" \
  -b '{"name":"test"}' "http://localhost:8000/error"

Post-Test Connection Inspection

SELECT 
    count(*) FILTER (WHERE state = 'idle in transaction') as idle_in_transaction,
    count(*) FILTER (WHERE state = 'active') as active,
    count(*) as total
FROM pg_stat_activity
WHERE datname = current_database()
AND pid != pg_backend_pid();

• idle_in_transaction > 0: Connection leak - sessions not properly closed
• idle_in_transaction = 0: All connections properly returned to pool ✅

Additional Context

This issue was reproduced consistently with pool_size=2, max_overflow=10. With smaller pools or higher concurrency, the too many clients already error appears faster. The try/finally pattern eliminates the leak entirely.

---

Recreated from upstream fastapi/full-stack-fastapi-template#2320 by @andersou, for a MAS-Ops Pipeline 2 (PR review) demonstration.

---

Note

Medium Risk
Touches shared DB dependency injection used by all API routes; behavior is a safer lifecycle fix with no API contract change, but mis-handling could still affect every request.

Overview
Replaces the with Session(engine) pattern in get_db with an explicit try / finally that always calls session.close() after the dependency generator finishes.

Under concurrent load—especially when route handlers raise—deferred context-manager cleanup was leaving PostgreSQL sessions in idle in transaction and exhausting the pool. This change guarantees the SQLModel session is closed as soon as FastAPI tears down the dependency, for every route that uses SessionDep.

^{Reviewed by Cursor Bugbot for commit 02a36a9. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

Summary by cubic

Prevented PostgreSQL connection leaks by switching get_db() to a try/finally pattern that always closes sessions. This avoids idle-in-transaction buildup and “too many clients” errors under exceptions and high load.

• Bug Fixes
  • Replaced with Session(engine): yield session with session = Session(engine); try: yield session; finally: session.close().
  • Ensures immediate cleanup when handlers raise, reliably returning connections to the pool.

^{Written for commit 02a36a9. Summary will update on new commits.}

[Copilot]

Pull request overview

This PR updates the FastAPI DB-session dependency (get_db) to guarantee SQLModel Session cleanup by explicitly closing the session in a finally block, reducing the risk of PostgreSQL connections being held longer than intended under exception-heavy / high-concurrency request patterns.

Changes:

• Replaced with Session(engine) as session: yield session with an explicit try/finally around yield.
• Ensures session.close() is always executed when the dependency scope ends (success or exception).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

📝 Info: Functional equivalence of context manager vs try/finally for Session

The old code used with Session(engine) as session: yield session, which relies on Session.__enter__ (returns self) and Session.__exit__ (calls self.close()). The new code manually creates the session, yields it, and calls session.close() in a finally block. These are functionally equivalent because SQLAlchemy's Session.__exit__ does nothing beyond calling close() — it does not auto-commit or auto-rollback based on exception status (unlike Session.begin() which does). However, the with statement pattern is more idiomatic and slightly more concise. The refactor doesn't appear to have a clear motivation — both patterns behave identically for this use case.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[cubic-dev-ai]

No issues found across 1 file

_{Re-trigger cubic}

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This is a well-executed fix for a critical PostgreSQL connection leak issue. The change from context manager (with Session(engine) as session) to explicit try/finally with session.close() is the correct approach for generator-based FastAPI dependencies.

Why this fix works:

• When using with context managers in generator-based dependencies, the __exit__ method is called when control exits the with block. However, yield pauses execution inside the context, so __exit__ is only called when the generator is cleaned up (deferred until garbage collection in some cases).
• Under high concurrency, especially when exceptions occur, this deferral causes connections to pile up in idle in transaction state, eventually exhausting the connection pool.
• The try/finally pattern ensures session.close() is called immediately when the generator completes, regardless of whether the consuming code raised an exception.

Safety considerations verified:

• session.close() in SQLAlchemy automatically rolls back uncommitted transactions before closing, so no data integrity concerns.
• No API contract changes - SessionDep still returns a Session object with the same behavior.
• All existing route handlers that use SessionDep will benefit from this fix without requiring code changes.

Files Reviewed (1 file)

• backend/app/api/deps.py - Connection leak fix for get_db()

---

_{Reviewed by laguna-m.1-20260312:free · 909,506 tokens}