Opened 9 years ago

Closed 9 years ago

Last modified 9 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: mtredinnick
Component: Contrib apps Version: master
Severity: normal Keywords:
Cc: gary.wilson@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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@… 9 years ago.
Patch to django/contrib/auth/handlers/modpython.py

Download all attachments as: .zip

Change History (7)

Changed 9 years ago by lec9@…

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

comment:1 Changed 9 years ago by lec9@…

  • Summary changed from django.contrib.auth.handlers.modpython doesn't send signals.request_*, leaks db connections to [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 Changed 9 years ago by Gary Wilson <gary.wilson@…>

  • Cc gary.wilson@… added

comment:3 Changed 9 years ago by mtredinnick

  • Owner changed from adrian to mtredinnick

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 Changed 9 years ago by anonymous

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 Changed 9 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:6 Changed 9 years ago by lec9@…

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