Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#26888 closed Bug (fixed)

RegexURLResolver doesn't work in threaded environments

Reported by: Markus Holtermann Owned by: Marten Kenbeek
Component: Core (URLs) Version: 1.10
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As reported on django-developers:

I'm using django (1, 10, 0, u'beta', 1).

When I try to reverse url in shell everything goes fine.

When under nginx/uwsgi with many concurrent request I get

... /local/lib/python2.7/site-packages/django/urls/resolvers.py", line 241,
in reverse_dict
    return self._reverse_dict[language_code]
KeyError: 'it'

After a wile I figured out that RegexURLResolver is memoized by get_resolver and so it acts like a singleton for a certain number of requests.

Analyzing the code of RegexURLResolver I found that the method _poupulate will return directly if it has been called before and not yet finished.

...
def _populate(self):
    if self._populating:
        return
    self._populating = True
...

if used for recursive call in a single thread this will not hurt, but in my case in uwsgi multi thread mode I got the error.

here is my quick and dirty fix:

class RegexURLResolver(LocaleRegexProvider):
    def __init__(self, regex, urlconf_name, default_kwargs=None, app_name=None, namespace=None):

        ...

        self._populating = False
        self.RLock = threading.RLock()

        ...

    def _populate(self):
        if self._populating:
            self.RLock.acquire()
            self.RLock.release()
            return
        self._populating = True
        self.RLock.acquire()

        ...

        self._populating = False
        self.RLock.release()

Change History (4)

comment:1 by Marten Kenbeek, 8 years ago

Has patch: set
Owner: changed from Markus Holtermann to Marten Kenbeek
Status: newassigned

PR: https://github.com/django/django/pull/6914

I don't think it's possible to write tests for this, other than simply calling _populate() on a large number of concurrent threads and hoping for the best. I'm open to suggestions.

Version 0, edited 8 years ago by Marten Kenbeek (next)

comment:2 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin
Type: UncategorizedBug

comment:3 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 389a5318:

Fixed #26888 -- Fixed concurrency issue in URL resolver.

Fixed a regression in 625b8e9295d79650208bfb3fca8bf9e6aaf578e4:
improper short-circuiting could lead to a KeyError when threads
concurrently call RegexURLResolver._populate().

comment:4 by Tim Graham <timograham@…>, 8 years ago

In 06323da:

[1.10.x] Fixed #26888 -- Fixed concurrency issue in URL resolver.

Fixed a regression in 625b8e9295d79650208bfb3fca8bf9e6aaf578e4:
improper short-circuiting could lead to a KeyError when threads
concurrently call RegexURLResolver._populate().

Backport of 389a5318a06e7e4d8f8aba14af88c4cc4ea0db47 from master

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