Skip to content

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

Open
andersou wants to merge 1 commit into
fastapi:masterfrom
andersou:fix/get-db-try-finally
Open

fix: use try/finally in get_db to prevent PostgreSQL connection leaks#2320
andersou wants to merge 1 commit into
fastapi:masterfrom
andersou:fix/get-db-try-finally

Conversation

@andersou
Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants