mirror of
				https://github.com/django/django.git
				synced 2025-10-31 01:25:32 +00:00 
			
		
		
		
	[1.10.x] Fixed #26888 -- Fixed concurrency issue in URL resolver.
Fixed a regression in625b8e9295: improper short-circuiting could lead to a KeyError when threads concurrently call RegexURLResolver._populate(). Backport of389a5318a0from master
This commit is contained in:
		
				
					committed by
					
						 Tim Graham
						Tim Graham
					
				
			
			
				
	
			
			
			
						parent
						
							96a37a0266
						
					
				
				
					commit
					06323dafc7
				
			| @@ -9,6 +9,7 @@ from __future__ import unicode_literals | |||||||
|  |  | ||||||
| import functools | import functools | ||||||
| import re | import re | ||||||
|  | import threading | ||||||
| from importlib import import_module | from importlib import import_module | ||||||
|  |  | ||||||
| from django.conf import settings | from django.conf import settings | ||||||
| @@ -164,7 +165,7 @@ class RegexURLResolver(LocaleRegexProvider): | |||||||
|         # urlpatterns |         # urlpatterns | ||||||
|         self._callback_strs = set() |         self._callback_strs = set() | ||||||
|         self._populated = False |         self._populated = False | ||||||
|         self._populating = False |         self._local = threading.local() | ||||||
|  |  | ||||||
|     def __repr__(self): |     def __repr__(self): | ||||||
|         if isinstance(self.urlconf_name, list) and len(self.urlconf_name): |         if isinstance(self.urlconf_name, list) and len(self.urlconf_name): | ||||||
| @@ -178,9 +179,13 @@ class RegexURLResolver(LocaleRegexProvider): | |||||||
|         ) |         ) | ||||||
|  |  | ||||||
|     def _populate(self): |     def _populate(self): | ||||||
|         if self._populating: |         # Short-circuit if called recursively in this thread to prevent | ||||||
|  |         # infinite recursion. Concurrent threads may call this at the same | ||||||
|  |         # time and will need to continue, so set 'populating' on a | ||||||
|  |         # thread-local variable. | ||||||
|  |         if getattr(self._local, 'populating', False): | ||||||
|             return |             return | ||||||
|         self._populating = True |         self._local.populating = True | ||||||
|         lookups = MultiValueDict() |         lookups = MultiValueDict() | ||||||
|         namespaces = {} |         namespaces = {} | ||||||
|         apps = {} |         apps = {} | ||||||
| @@ -213,7 +218,7 @@ class RegexURLResolver(LocaleRegexProvider): | |||||||
|                         namespaces[namespace] = (p_pattern + prefix, sub_pattern) |                         namespaces[namespace] = (p_pattern + prefix, sub_pattern) | ||||||
|                     for app_name, namespace_list in pattern.app_dict.items(): |                     for app_name, namespace_list in pattern.app_dict.items(): | ||||||
|                         apps.setdefault(app_name, []).extend(namespace_list) |                         apps.setdefault(app_name, []).extend(namespace_list) | ||||||
|                 if not pattern._populating: |                 if not getattr(pattern._local, 'populating', False): | ||||||
|                     pattern._populate() |                     pattern._populate() | ||||||
|                 self._callback_strs.update(pattern._callback_strs) |                 self._callback_strs.update(pattern._callback_strs) | ||||||
|             else: |             else: | ||||||
| @@ -225,7 +230,7 @@ class RegexURLResolver(LocaleRegexProvider): | |||||||
|         self._namespace_dict[language_code] = namespaces |         self._namespace_dict[language_code] = namespaces | ||||||
|         self._app_dict[language_code] = apps |         self._app_dict[language_code] = apps | ||||||
|         self._populated = True |         self._populated = True | ||||||
|         self._populating = False |         self._local.populating = False | ||||||
|  |  | ||||||
|     @property |     @property | ||||||
|     def reverse_dict(self): |     def reverse_dict(self): | ||||||
|   | |||||||
| @@ -5,6 +5,7 @@ Unit tests for reverse URL lookups. | |||||||
| from __future__ import unicode_literals | from __future__ import unicode_literals | ||||||
|  |  | ||||||
| import sys | import sys | ||||||
|  | import threading | ||||||
|  |  | ||||||
| from admin_scripts.tests import AdminScriptTestCase | from admin_scripts.tests import AdminScriptTestCase | ||||||
|  |  | ||||||
| @@ -429,6 +430,18 @@ class ResolverTests(SimpleTestCase): | |||||||
|         self.assertTrue(resolver._is_callback('urlpatterns_reverse.nested_urls.View3')) |         self.assertTrue(resolver._is_callback('urlpatterns_reverse.nested_urls.View3')) | ||||||
|         self.assertFalse(resolver._is_callback('urlpatterns_reverse.nested_urls.blub')) |         self.assertFalse(resolver._is_callback('urlpatterns_reverse.nested_urls.blub')) | ||||||
|  |  | ||||||
|  |     def test_populate_concurrency(self): | ||||||
|  |         """ | ||||||
|  |         RegexURLResolver._populate() can be called concurrently, but not more | ||||||
|  |         than once per thread (#26888). | ||||||
|  |         """ | ||||||
|  |         resolver = RegexURLResolver(r'^/', 'urlpatterns_reverse.urls') | ||||||
|  |         resolver._local.populating = True | ||||||
|  |         thread = threading.Thread(target=resolver._populate) | ||||||
|  |         thread.start() | ||||||
|  |         thread.join() | ||||||
|  |         self.assertNotEqual(resolver._reverse_dict, {}) | ||||||
|  |  | ||||||
|  |  | ||||||
| @override_settings(ROOT_URLCONF='urlpatterns_reverse.reverse_lazy_urls') | @override_settings(ROOT_URLCONF='urlpatterns_reverse.reverse_lazy_urls') | ||||||
| class ReverseLazyTest(TestCase): | class ReverseLazyTest(TestCase): | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user