Django

Code

Ticket #10472 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

RegexURLResolver.reverse() is not threadsafe

Reported by: tdterry Assigned to: tdterry
Milestone: Component: Core framework
Version: 1.0 Keywords: urlresolvers, reverse, threading
Cc: Triage Stage: Unreviewed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

I have found an incomplete-initialization bug in RegexURLResolver.reverse() when running under Apache mpm_worker with mod_wsgi.

RegexURLResolver objects are stored in process local memory using @memoize. The _reverse_dict is built on demand, so two threads trying to build it for the first time can run into an incomplete initialization. The attached patch changes RegexURLResolver._get_reverse_dict() to do an atomic set of self._reverse_dict, preventing multiple threads from seeing an inconsistent _reverse_dict.

Testing requires a multi-threaded setup and a good load test. These are not available in the framework, so I have not included a specific test with the patch.

Attachments

thread-safe-urlresolvers.diff (1.7 kB) - added by tdterry on 03/11/09 11:17:22.
Patch against r10030 to solve threadsafe issues with the urlresolvers.RegexURLResolver

Change History

03/11/09 11:17:22 changed by tdterry

  • attachment thread-safe-urlresolvers.diff added.

Patch against r10030 to solve threadsafe issues with the urlresolvers.RegexURLResolver

03/11/09 11:18:01 changed by tdterry

  • owner changed from nobody to tdterry.
  • needs_better_patch changed.
  • status changed from new to assigned.
  • needs_tests changed.
  • needs_docs changed.

03/11/09 12:00:26 changed by mrts

  • keywords changed from urlresolvers, reverse, threadsafe to urlresolvers, reverse, threading.
  • component changed from Uncategorized to Core framework.
  • stage changed from Unreviewed to Accepted.

Pushing to accepted, the bug is real (strangely I haven't been hit by this running under the same setup) and the patch looks OK.

03/11/09 13:38:43 changed by mrts

  • stage changed from Accepted to Unreviewed.

Looking around a bit more thoroughly, I didn't find no places where RegexURLResolver instances are shared between threads. Main use is via BaseHandler.get_response() where the BaseHandler instance creates the resolver instance anew for each request. The ones created within urls.py (conf/urls/defaults.py) are initialized before requests come in and not modified after (correct me in the latter).

Pushing back to unreviewed, IMHO can be closed as invalid.

03/11/09 13:40:11 changed by mrts

tdterry, I assumed you were actually hit by a bug, not doing code review. If you indeed were, please provide more context.

03/12/09 02:28:23 changed by mtredinnick

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [10037]) Fixed #10472 -- Fixed a race condition in reverse URL resolving.

This only shows up in for reverse() (not forwards resolving), since that path uses a globally shared resolver object. Based on a patch from Travis Terry.

03/12/09 02:37:38 changed by mtredinnick

(In [10039]) [1.0.X] Fixed #10472 -- Fixed a race condition in reverse URL resolving.

This only shows up in for reverse() (not forwards resolving), since that path uses a globally shared resolver object. Based on a patch from Travis Terry.

Backport of r10037 from trunk.

03/12/09 03:10:58 changed by mrts

tdterry, sorry, I missed the memoize on line 73, should have read the ticket description more carefully.

Add/Change #10472 (RegexURLResolver.reverse() is not threadsafe)




Change Properties
Action