Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#1445 closed defect (fixed)

Response middleware can leak db connection

Reported by: Maniac <Maniac@…> Owned by: Adrian Holovaty
Component: contrib.admin Version:
Severity: normal Keywords:
Cc: Maniac@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Both modpython.py and wsgi.py do this:

    try:
        request = ModPythonRequest(req)
        response = self.get_response(req.uri, request)
    finally:
        db.db.close()

    # Apply response middleware
    for middleware_method in self._response_middleware:
        response = middleware_method(request, response)

... meaning that if any response middleware uses db connection it will not be closed. The most common middleware doing it is Session.

The right thing to do would be stick the middleware part up under try block.

However even more right thing (I think) would be to refactor both request and response middleware application along with finally: part into BaseHandler._get_response to avoid maintaining identical code in modpython.py and wsgi.py.

Change History (6)

comment:1 by hugo, 18 years ago

yep, I wrote myself a response middleware that just closes the current db connection to make sure that connections are allways closed, regardless of any problems. The leaked connections can pose problems, as they are "idle in transaction" under PostgreSQL and therefore might block records they read in the transaction (psycopg does an implicit transaction that is only closed on db.close() or one of .save() or .delete() in Django) due to the multi-version-nature of PostgreSQL.

comment:2 by junzhang.jn@…, 18 years ago

Resolution: fixed
Status: newclosed

this is fixed at rev:2358

comment:3 by Adrian Holovaty, 18 years ago

(In [2474]) magic-removal: Fixed #1445 -- Moved response middleware-handling code inside try/finally that closes DB connection

comment:4 by Maniac <Maniac@…>, 18 years ago

That was fast :-). Thanks!

comment:5 by bryan@…, 18 years ago

Resolution: fixed
Status: closedreopened

This needs to be applied to trunk as well. I'm running a django server using trunk and the postgres idle connections keep piling up. After applying this fix to trunk, the idle connections don't appear.

comment:6 by Adrian Holovaty, 18 years ago

Resolution: fixed
Status: reopenedclosed

Closing this because trunk is magic-removal.

Note: See TracTickets for help on using tickets.
Back to Top