Opened 18 years ago

Closed 18 years ago

Last modified 17 years ago

#2770 closed defect (fixed)

[patch] django.contrib.auth.handlers.modpython doesn't send signals.request_*, leaks db connections

Reported by: lec9@… Owned by: Malcolm Tredinnick
Component: Contrib apps Version: dev
Severity: normal Keywords:
Cc: gary.wilson@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The module django.contrib.auth.handlers.modpython does not send signals.request_started before it begins processing, and more importantly, doesn't send signals.request_finished once it's completed, which means that the callback established by the line """dispatcher.connect(connection.close, signal=signals.request_finished)""" in django.db's init.py is never called.

As such, the database connection is not closed (until the python interpreter is culled, eg when apache2 restarts) and is left to fester. I spotted this as I was using it to authenticate all access to an SVN repository, and "svn diff" was making so many requests that eventually postgres gave an error of "connection limit exceeded for non-superusers".

Patch to follow.

Attachments (1)

pythonauthenhandler.patch (2.2 KB ) - added by lec9@… 18 years ago.
Patch to django/contrib/auth/handlers/modpython.py

Download all attachments as: .zip

Change History (7)

by lec9@…, 18 years ago

Attachment: pythonauthenhandler.patch added

Patch to django/contrib/auth/handlers/modpython.py

comment:1 by lec9@…, 18 years ago

Summary: django.contrib.auth.handlers.modpython doesn't send signals.request_*, leaks db connections[patch] django.contrib.auth.handlers.modpython doesn't send signals.request_*, leaks db connections

Added "[patch]" to beginning of summary, and comment here due to Akismet lameness filter. Note that patch probably needs paths tidying up slightly, as it is the output of svndiff on my own import of django.

comment:2 by Gary Wilson <gary.wilson@…>, 18 years ago

Cc: gary.wilson@… added

comment:3 by Malcolm Tredinnick, 18 years ago

Owner: changed from Adrian Holovaty to Malcolm Tredinnick

Looks generally correct. I've got a query into Joseph Kocherhans just to check I understand all the subtleties of this request path, but as soon as he confirms I'm understanding things properly, I'll apply this.

comment:4 by anonymous, 18 years ago

After discussion with Joseph, I've decided not to use this patch as it stands. We aren't processing a real Django request during the call to authenhandler() and the client code may have hooked other handlers into the request_started and request_finished signals that implicitly assume they are wrapped around a full-blown Django request.

I'll commit something that fixes the database leak in the same way those signals would.

comment:5 by Malcolm Tredinnick, 18 years ago

Resolution: fixed
Status: newclosed

(In [3789]) Fixed #2770 -- Fixed a database connection leak in
django.contrib.auth.handlers.modpython.

comment:6 by lec9@…, 17 years ago

Thanks, sorry for the misunderstanding. I asked around on IRC but they weren't sure what counted as a request. I'll apply your patch to my tree when I get a moment.

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