Skip to content

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

Closed
Vacbo wants to merge 1 commit into
masterfrom
demo/get-db-connection-leak
Closed

fix: use try/finally in get_db to prevent PostgreSQL connection leaks#3
Vacbo wants to merge 1 commit into
masterfrom
demo/get-db-connection-leak

Conversation

@Vacbo
Copy link
Copy Markdown
Owner

@Vacbo Vacbo commented May 30, 2026

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.


Summary by cubic

Prevents PostgreSQL connection leaks by changing get_db to a try/finally pattern that always closes the session. Stops idle in transaction buildup and pool exhaustion under load.

  • Bug Fixes
    • Replaced with Session(engine) as session: yield session with session = Session(engine) and finally: session.close() to ensure cleanup on success and errors.
    • Eliminates leaked sessions that caused "too many clients already" during concurrent exceptions.

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

Review in cubic

Copilot AI review requested due to automatic review settings May 30, 2026 23:07
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the FastAPI DB-session dependency to ensure the SQLModel Session is always closed via an explicit try/finally, aiming to prevent pooled PostgreSQL connections from remaining checked out under exceptional request flows.

Changes:

  • Replaced the with Session(engine) as session: yield session pattern with session = Session(engine) + try: yield + finally: session.close() in the get_db() dependency.

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

@Vacbo Vacbo closed this May 30, 2026
@Vacbo Vacbo deleted the demo/get-db-connection-leak branch May 30, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants